chore: address review feedback

_SPIKE_.md

@@ -84,10 +84,16 @@ class Product extends Entity {
   categoryId: number;
 }
 
+/**
+ * Navigation properties of the Product model.
+ */
 interface ProductLinks {
-  category?: Category & CategoryLinks;
+  category?: CategoryWithLinks;
 }
 
+/**
+ * Product's own properties and navigation properties.
+ */
 type ProductWithLinks = Product & ProductLinks;
 ```
 
@@ -100,10 +106,16 @@ class Category extends Entity {
   products?: Product[];
 }
 
+/**
+ * Navigation properties of the Category model.
+ */
 interface CategoryLinks {
-  products?: Product & ProductLinks;
+  products?: ProductWithLinks[];
 }
 
+/**
+ * Category's own properties and navigation properties.
+ */
 type CategoryWithLinks = Category & CategoryLinks;
 ```
 
@@ -120,6 +132,10 @@ This solution has few important properties I'd like to explicitly point out:
 - It makes it easy to define a type where all navigational properties are
   optional. For example: `Product & Partial<ProductLinks>`
 
+  UPDATE: As it turns out, it's not enough to mark all navigational properties
+  as optional. See the discussion in
+  https://github.com/strongloop/loopback-next/pull/2592#discussion_r267600322
+
 ### Integration with CrudRepository APIs
 
 The CRUD-related Repository interfaces and classes are accepting a new generic
@@ -213,6 +229,12 @@ schema. Here is an example as produced by `getJsonSchemaRef`:
 }
 ```
 
+The first schema defines `CategoryWithLinks` as the top-level schema,
+`definitions` contain only `ProductWithLinks` schema.
+
+The second schema contains only `$ref` entry at the top-level, the actual schema
+for `CategoryWithLinks` is defined in `definitions`.
+
 ### Controller spec
 
 The last missing piece is integration with controller spec builder.
@@ -241,7 +263,7 @@ class CategoryController {
   })
   async find(
     @param.query.object('filter', getFilterSchemaFor(Category)) filter?: Filter,
-  ): Promise<TodoList[]> {
+  ): Promise<CategoryWithLinks[]> {
     return await this.categoryRepository.find(filter);
   }
 }
@@ -295,8 +317,9 @@ via OpenAPI spec extensions. For example:
 
 - Add a new generic parameter `Links` to CRUD-related Repository interfaces and
   implementations.
-- Modify the signature `find` and `findById` to return `T & Partial<Links>`
-  instead of `T`.
+- Modify the signature `find` and `findById` to return `T & Links` instead of
+  `T`. If this requires too many explicit casts, then consider using
+  `T & Partial<Links>` instead, assuming it improves the situation.
 
 6. Update `examples/todo-list` to leverage these new features:
 
@@ -305,3 +328,7 @@ via OpenAPI spec extensions. For example:
 - Update repositories to include related models: overwrite `find` and `findById`
   methods, add a hard-coded retrieval of related models.
 - Update response schemas for controller methods `find` and `findById`
+
+7. Replace our temporary poor-man's relation resolver with a real one, as
+   described in https://github.com/strongloop/loopback-next/pull/2124. Update
+   the example app as part of this work.

packages/repository/src/repositories/legacy-juggler-bridge.ts

@@ -337,10 +337,7 @@ export class DefaultCrudRepository<
     }
   }
 
-  async find(
-    filter?: Filter<T>,
-    options?: Options,
-  ): Promise<(T & Partial<Links>)[]> {
+  async find(filter?: Filter<T>, options?: Options): Promise<(T & Links)[]> {
     const models = await ensurePromise(
       this.modelClass.find(filter as legacy.Filter, options),
     );
@@ -359,14 +356,14 @@ export class DefaultCrudRepository<
     id: ID,
     filter?: Filter<T>,
     options?: Options,
-  ): Promise<T & Partial<Links>> {
+  ): Promise<T & Links> {
     const model = await ensurePromise(
       this.modelClass.findById(id, filter as legacy.Filter, options),
     );
     if (!model) {
       throw new EntityNotFoundError(this.entityClass, id);
     }
-    return this.toEntity<T & Partial<Links>>(model);
+    return this.toEntity<T & Links>(model);
   }
 
   update(entity: T, options?: Options): Promise<void> {
@@ -456,6 +453,6 @@ export class DefaultCrudRepository<
   }
 
   protected toEntities<R extends T>(models: juggler.PersistedModel[]): R[] {
-    return models.map(m => this.toEntity(m));
+    return models.map(m => this.toEntity<R>(m));
   }
 }

packages/repository/src/repositories/repository.ts

@@ -3,20 +3,20 @@
 // This file is licensed under the MIT License.
 // License text available at https://opensource.org/licenses/MIT
 
-import {Entity, ValueObject, Model} from '../model';
 import {
-  DataObject,
-  Options,
   AnyObject,
   Command,
+  Count,
+  DataObject,
   NamedParameters,
+  Options,
   PositionalParameters,
-  Count,
 } from '../common-types';
-import {DataSource} from '../datasource';
 import {CrudConnector} from '../connectors';
-import {Filter, Where} from '../query';
+import {DataSource} from '../datasource';
 import {EntityNotFoundError} from '../errors';
+import {Entity, Model, ValueObject} from '../model';
+import {Filter, Where} from '../query';
 
 // tslint:disable:no-unused
 
@@ -66,7 +66,7 @@ export interface CrudRepository<
    * @param options Options for the operations
    * @returns A promise of an array of records found
    */
-  find(filter?: Filter<T>, options?: Options): Promise<(T & Partial<Links>)[]>;
+  find(filter?: Filter<T>, options?: Options): Promise<(T & Links)[]>;
 
   /**
    * Updating matching records with attributes from the data object
@@ -150,11 +150,7 @@ export interface EntityCrudRepository<
    * @param options Options for the operations
    * @returns A promise of an entity found for the id
    */
-  findById(
-    id: ID,
-    filter?: Filter<T>,
-    options?: Options,
-  ): Promise<T & Partial<Links>>;
+  findById(id: ID, filter?: Filter<T>, options?: Options): Promise<T & Links>;
 
   /**
    * Update an entity by id with property/value pairs in the data object