nestjs / mongoose

Mongoose module for Nest framework (node.js) 🍸
https://nestjs.com
MIT License
518 stars 116 forks source link

must provide connectionName explicitly after update from 9.0.3 to 9.1.0 #1437

Closed Tobias-Scholz closed 2 years ago

Tobias-Scholz commented 2 years ago

Did you read the migration guide?

Is there an existing issue that is already proposing this?

Potential Commit/PR that introduced the regression

231d3da

Versions

9.0.3 -> 9.1.0

Describe the regression

After updating to 9.1.0 we have to provide a connectionName in each @InjectModel. Since it is already defined elsewhere (see below), this leads to code duplication and is error-prone because the connectionName can be easily mistaken.

Minimum reproduction code

This is the module that causes the error. We set a connectionName for each Model in the forFeature call.

database.module.ts

export enum CONNECTION_TYPES {
  PRIMARY = 'PRIMARY_CONNECTION',
  SECONDARY = 'SECONDARY_CONNECTION'
}

const uris = buildConnectionStrings()

const databaseModules = [
  MongooseModule.forRoot(uris.MONGO_PRIMARY_CONNECTION_STRING, {
    connectionName: CONNECTION_TYPES.PRIMARY
  }),
  MongooseModule.forRoot(uris.MONGO_SECONDARY_CONNECTION_STRING, {
    connectionName: CONNECTION_TYPES.SECONDARY
  }),
  MongooseModule.forFeature(
    [{ name: Portfolio.name, schema: PortfolioSchema }],
    CONNECTION_TYPES.PRIMARY
  ),
  MongooseModule.forFeature(
    [{ name: Company.name, schema: CompanySchema }],
    CONNECTION_TYPES.SECONDARY
  )
]

@Module({
  imports: [...databaseModules],
  exports: [...databaseModules]
})
export class DatabaseModule {}

Because we set the connectionName in the database-module.ts, it was not needed here anymore in version 9.0.3.

account.service.ts

@Injectable()
export class AccountService {
  constructor(
    @InjectModel(Portfolio.name) private portfolioModel: Model<Portfolio>
  ) {}
}

This behaviour changed in 9.1.0. Now you have to provide the connectionName everywhere.

account.service.ts

@Injectable()
export class AccountService {
  constructor(
    @InjectModel(Portfolio.name, CONNECTION_TYPES.PRIMARY_CONNECTION) private portfolioModel: Model<Portfolio>
  ) {}
}

Expected behavior

This should continue to compile without errors.

Error message

[Nest] 3818 - 06/04/2022, 12:50:13 AM ERROR [ExceptionHandler] Nest can't resolve dependencies of the WatchlistService (?). Please make sure that the argument PortfolioModel at index [0] is available in the AppModule context.

kamilmysliwiec commented 2 years ago

Looking at the commit you shared, connectionName seems optional.

Please provide a minimum reproduction repository (Git repository/StackBlitz/CodeSandbox project).

3v3ryb0dy commented 2 years ago

Hi, thanks @kamilmysliwiec for the quick reply. I made a minimal demo. Note that @nestjs/mongoose@9.0.3 is installed. When running via npm start, the application starts. (Although it quits immediately, I think this is due to stackblitz limitations when connecting to mongodb *) However, when you install @nestjs/mongoose@9.2.0, and start the application, there is the above-mentioned error.

https://stackblitz.com/edit/nestjs-typescript-starter-d2ugnd?file=src/app.module.ts

(* Locally, the application starts fine when provided with a valid mongo uri)

kamilmysliwiec commented 2 years ago

Thanks for sharing!

So this is intentional. It turns out there was a bug in @nestjs/mongoose@9.0.3 that swallowed this error. This has been fixed in 9.2.0. If both models are registered under a specific connection, passing it as a second parameter of the @InjectModel annotation is (and should be) required.

Please, use our Discord channel (support) for such questions. We are using GitHub to track bugs, feature requests, and potential improvements.