nestjs / docs.nestjs.com

The official documentation https://docs.nestjs.com đŸ“•
MIT License
1.2k stars 1.74k forks source link

Cannot get HMR working with nestjs + typeorm + graphql #1299

Closed brandon-leapyear closed 4 years ago

brandon-leapyear commented 4 years ago

Bug Report

Current behavior

I have a NestJS project with TypeORM + Graphql configured

@Module({
  imports: [
    GraphQLFederationModule.forRoot({
      autoSchemaFile: path.join(process.cwd(), 'src/schema.gql'),
      context: ({ req }) => ({ req }),
    }),
    TypeOrmModule.forRootAsync({
      useClass: TypeOrmConfigService,
      imports: [ConfigModule],
    }),
    ...
  ],
  ...
})
export class AppModule {}

I followed the instructions in the guide, but when making a change to one of my .resolver.ts files, I see one of two errors:

AlreadyHasActiveConnectionError: Cannot create a new connection named "default", 
because connection with such name already exist and it now has an active connection session.

or

(node:45219) UnhandledPromiseRejectionWarning: Error: Schema must contain 
uniquely named types but contains multiple types named "User".

For the first case, I found this StackOverflow post, but adding keepConnectionAlive didn't help for me. (I added it to the object returned by TypeOrmConfigService.createTypeOrmOptions()). It seems to be better by doing

  if (module.hot) {
    module.hot.accept()
    module.hot.dispose(async () => {
      await getConnection().close()
      await app.close()
    })
  }

but I still get the graphql error, plus a new error:

(node:46024) UnhandledPromiseRejectionWarning: CannotExecuteNotConnectedError: 
Cannot execute operation on "default" connection because connection is not yet established.

Any ideas?

Input Code

Expected behavior

Possible Solution

Environment


Nest versions:
    "@nestjs/common": "^7.0.9",
    "@nestjs/core": "^7.0.3",
    "@nestjs/graphql": "^7.3.7",
    "@nestjs/platform-express": "^7.0.11",
    "@nestjs/typeorm": "^7.0.0",

For Tooling issues:
- Node version: v12.14.1
- Platform: Mac

Others:

ceopaludetto commented 4 years ago

I think this is a problem about @nestjs/graphql and both-side relations, the same error occurs using @nestjs/sequelize package.

For knowledge, my overview about: When using webpack you're forced to write your entities/models with not directly importing. This occurs because webpack harmony modules doens't work like expected in cycle dependencies, which work's like a charm in commonjs(require).

Imagine two models(Book and Author): If in Author we use HasMany decorator and in Book we use BelongsTo, the both files need the exported class of each other. To avoid the undefined in typescript compilant and require, we use () => Model acessor, but this doesn't work in webpack, to avoid this behavior we are forced to move the dependency in a third file which export the two Models. When you do that, the dependency of Model (Author or Book) is duplicated, the first is to Sequelize Module or TypeormModule entity resolver and the other is to relations, then when we put HotModuleReplacementPlugin in server side, the new accepted file will run the metadata registry twice, duplicating the metadata and writing in schema.

I don't know if this is the real behavior, but when I use a one-sided relation the error doens't occur. I've read some comments on the type-graphql repo and, some users suggest deduplicating metadata in hmr enviroments, I think this issue was solved there.

Something like deduplication is difficult to achieve, I think.

ceopaludetto commented 4 years ago

References: https://github.com/MichalLytek/type-graphql/issues/396#issuecomment-564665244 https://github.com/MichalLytek/type-graphql/issues/289

kamilmysliwiec commented 4 years ago

Can you please provide a minimum reproduction repository? Also, let me move it to the Docs repo since HMR isn't specifically related to NestJS.

ceopaludetto commented 4 years ago

I figure out. Changing either tsconfig or babel.config fix the issue for me. You can just modify tsconfig module compiler options to CommonJS or @babel/preset-env module to cjs.

Note: Apply this modification only in server side, this will break your side effect free modules in client side(if you use).

Example: babel.config.js or .babelrc

module.exports = {
  presets: [
    ["@babel/preset-env", {
        modules: "cjs" // or isServer ? "cjs" : false
    }]
  ]
}

tsconfig.json

{
  "compilerOptions": {
     "module": "CommonJS"
  }
}

Doens't apply the modification in both, choose one, if you choose tsconfig, set modules in babel to false, if you choose babel, set modules in tsconfig to ESNext(avoid double modification in your modules).

Verify too if your externals in webpack is setted.

Can you try @brandon-leapyear ?

brandon-leapyear commented 4 years ago

:sparkles: This is an old work account. Please reference @brandonchinn178 for all future communication :sparkles:


tsconfig.json already has module set to commonjs. We don't have a babel.config.js or .babelrc file; we're using the nest CLI

brandon-leapyear commented 4 years ago

:sparkles: This is an old work account. Please reference @brandonchinn178 for all future communication :sparkles:


https://github.com/brandon-leapyear/nest-graphql-typeorm-hmr

Minimal repro here. Running HMR currently gets the typeorm error:

AlreadyHasActiveConnectionError: Cannot create a new connection named "default", 
because connection with such name already exist and it now has an active connection 
session.

feel free to play around on there

brandon-leapyear commented 4 years ago

:sparkles: This is an old work account. Please reference @brandonchinn178 for all future communication :sparkles:


Never mind, got it working with keepConnectionAlive. The reason it wasn't working was because I was setting it to process.env.NODE_ENV === 'development', but it turns out that NODE_ENV wasn't actually defined. Setting keepConnectionAlive: !!module.hot works