From 20d01a28ed4f192042ff43e2a8dcb3ea98892ab3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= <mbajtoss@gmail.com>
Date: Fri, 29 May 2020 08:37:54 +0200
Subject: [PATCH] fix(repository): fix DynamicModelCtor type to correctly
 preserve Props
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Before this change, the following code would create an instance
with the type of the base model class, ignoring custom model properties:

```ts
const Product = defineModelClass<typeof Model, {name: string}>(
  Model,
  definition,
);
const p = new Product();
// p has type Model now
```

This patch fixes the problem by changing the order in which constructor
functions are defined in `DynamicModelCtor`, so that the compiler always
picks the constructor returning `Base & Props`.

Signed-off-by: Miroslav Bajtoš <mbajtoss@gmail.com>
---
 .../unit/model/define-model-class.unit.ts      | 18 ++++++++++++++++++
 packages/repository/src/define-model-class.ts  |  4 ++--
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/packages/repository/src/__tests__/unit/model/define-model-class.unit.ts b/packages/repository/src/__tests__/unit/model/define-model-class.unit.ts
index 1285e9aa03bb..e406bb7d4f40 100644
--- a/packages/repository/src/__tests__/unit/model/define-model-class.unit.ts
+++ b/packages/repository/src/__tests__/unit/model/define-model-class.unit.ts
@@ -31,6 +31,24 @@ describe('defineModelClass', () => {
     expect(instance.title).to.equal('a title');
   });
 
+  it('creates a correctly typed constructor', () => {
+    const definition = new ModelDefinition('Product').addProperty('name', {
+      type: 'string',
+    });
+    const Product = defineModelClass<typeof Model, {name: string}>(
+      Model,
+      definition,
+    );
+
+    // When the `data` argument is not provided, then TypeScript may pick
+    // Model's constructor signature instead of Product constructor signature.
+    // When that happens, `p` has type `Model` without Product properties.
+    // This test verifies that such situation does not happen.
+    const p = new Product();
+    p.name = 'Pen';
+    // The test passed when the line above is accepted by the compiler.
+  });
+
   it('creates an Entity class', () => {
     const definition = new ModelDefinition('Product')
       .addProperty('id', {type: 'number', id: true})
diff --git a/packages/repository/src/define-model-class.ts b/packages/repository/src/define-model-class.ts
index cc42c10d1f7e..f529fc7736d2 100644
--- a/packages/repository/src/define-model-class.ts
+++ b/packages/repository/src/define-model-class.ts
@@ -79,10 +79,10 @@ export function defineModelClass<
 export type DynamicModelCtor<
   BaseCtor extends typeof Model,
   Props extends object
-> = BaseCtor & {
+> = {
   /** Model constructor accepting partial model data. */
   new (data?: DataObject<PrototypeOf<BaseCtor> & Props>): PrototypeOf<
     BaseCtor
   > &
     Props;
-};
+} & BaseCtor;