nestjs / nest

A progressive Node.js framework for building efficient, scalable, and enterprise-grade server-side applications with TypeScript/JavaScript 🚀
https://nestjs.com
MIT License
66.9k stars 7.55k forks source link

Sequelize-Typescript issues in Starter and Example07 #285

Closed sebastianwahn closed 6 years ago

sebastianwahn commented 6 years ago

I'm submitting a...


[ ] Regression 
[x] Bug report
[ ] Feature request
[ ] Documentation issue or request
[ ] Support request => Please do not submit support request here, instead post your question on Stack Overflow.

Current behavior

When using the nest starter boilerplate code, the version 0.6.1 for package "sequelize-typescript":"^0.6.1" is used.

In example 07 regarding the integration of sequelize, following code is provided as an example to use the repository:

@Component()
export class CatsService {
  constructor(
    @Inject('CatsRepository') private readonly catsRepository: typeof Model) {}

  async findAll(): Promise<Cat[]> {
    return await this.catsRepository.findAll<Cat>();
  }
}

Although, when checking the code of the example, it uses version 0.5.0 of "sequelize-typescript": "^0.5.0",

In the 0.5.0 the code example works fine, when using the starter code (which uses 0.6.1) following TypeScript compiler error show up:

Error:(12, 22) TS2684: The 'this' context of type 'typeof Model' is not assignable to method's 'this' of type 'new () => Cat'. Cannot assign an abstract constructor type to a non-abstract constructor type.

Expected behavior

The code example shows how to work with a current version of sequelize-typescript, which is used in the starter.

Minimal reproduction of the problem with instructions

  1. check out your repository
  2. go to example07
  3. run example07 --> should not produce an TypeScript error
  4. update sequelize-typescript to version 0.6.1
  5. run example07 again --> should throw the TypeScript error TS2684

What is the motivation / use case for changing the behavior?

Clear and foolproof example on how to work with nest and sequelize. Furthermore, reduction of debugging time for other developers.

Workaround

Use version 0.5.0 of sequelize-typescript instead of 0.6.1 initially set by the nest starter.

Environment


Nest version: X.Y.Z
```
"dependencies": {
    "@nestjs/common": "^4.4.0",
    "@nestjs/core": "^4.4.0",
    "@nestjs/microservices": "^4.4.0",
    "@nestjs/swagger": "^1.1.2",
    "@nestjs/testing": "^4.4.0",
    "@nestjs/websockets": "^4.4.0",
    "config": "^1.28.1",
    "mysql2": "^1.5.1",
    "redis": "^2.7.1",
    "reflect-metadata": "^0.1.10",
    "rxjs": "^5.4.0",
    "sequelize": "^4.23.4",
    "sequelize-typescript": "^0.6.1",
    "typescript": "^2.x"
  },
  "devDependencies": {
    "@types/config": "^0.0.33",
    "@types/node": "^8.0.28",
    "@types/sequelize": "^4.0.79",
    "nodemon": "^1.12.1",
    "ts-node": "^3.3.0",
    "tslint": "^5.8.0"
  }
```

For Tooling issues:
- Node version: v8.7.0
- Platform:  Mac / Docker (node:8)

Further information

The problematic code is export declare abstract class Model<T> extends Hooks { introduced in https://github.com/RobinBuschmann/sequelize-typescript/commit/cc95b82eb49bd1b41d844b2c37744f4b52883938#diff-1c485f95a55d5ce0466f63a966ced297R25

kamilmysliwiec commented 6 years ago

Hi @sebastianwahn, I can't reproduce this issue 🙁 Let's try to change typeof Model into typeof Cat and let me know

liquidyarn commented 6 years ago

Hi @kamilmysliwiec I had the same issue and when I change typeof Model into typeof Cat, that error disappeared. Thanks!

sebastianwahn commented 6 years ago

@kamilmysliwiec Thanks for the tip! I thought i tried this one already, but this solves it for 0.6.1

Unfortunately, this will still fail for people using the nest-starter and follow the code example on the page/repository (like I did).

So the best thing would be to update the example on the page to use typeof Cat and to update the code for example 07 to use 0.6.1 and also typeof Cat.

kamilmysliwiec commented 6 years ago

I have updated the example already, now the docs part 💪

PatrickGeyer commented 5 years ago

Hi @kamilmysliwiec - I have exactly the same issue, but I'm using a Base Model and BaseService class.

` export abstract class BaseService{

constructor(protected readonly repository: Repo...`

So the model type is passed in from each extended service. How can I set repository type to a generic Model?

I know this isn't specific to NestJS, but I can't find any issue as similar as this thread..

jberall commented 5 years ago

Did you figure out a way to setup a generic type? with possible overrides?

ie. getModel(): Model<dog,any>; getModel(): Model<cat,any>;

lock[bot] commented 5 years ago

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.