Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dependency resolution fails with @mikro-orm/cli and pnpm #81

Closed
duckies opened this issue Aug 12, 2022 · 9 comments
Closed

Dependency resolution fails with @mikro-orm/cli and pnpm #81

duckies opened this issue Aug 12, 2022 · 9 comments
Labels
bug Something isn't working

Comments

@duckies
Copy link

duckies commented Aug 12, 2022

Describe the bug
NestJS is unable to resolve SqlEntityManager in a pnpm monorepo only when @mikro-orm/cli is installed.

Stack trace

ERROR [ExceptionHandler] Nest can't resolve dependencies of the AppService (?). Please make sure that the argument SqlEntityManager at index [0] is available in the AppModule context.

Reproduction (duckies/mikro-orm-cli-issue)
I created a minimal reproduction that should fail on pnpm i && pnpm dev, but will work once @mikro-orm/cli is removed from the ./packages/server/package.json's dependencies and commands reran. I also included a yarn@v1 branch and NestJS is working with all dependencies installed.

Additional context
I'm unsure if this is an issue with mikro-orm, pnpm, or otherwise. I know pnpm often duplicates, and thus breaks, @nestjs/core, but I don't think this is the case here.

Versions

Dependency Version
node 16.13.1
typescript 4.7.4
mikro-orm 5.3.1
your-driver postgresql
@duckies duckies added the bug Something isn't working label Aug 12, 2022
@B4nan B4nan removed their assignment Aug 13, 2022
@B4nan
Copy link
Member

B4nan commented Aug 14, 2022

I am not sure if this is something to fix at our end, the dependency definition is looking good to me, pnpm clearly installs multiple @mikro-orm/core packages, but that itself shouldn't matter here, as you are not using the CLI at all. Note that the same issue is there for any ORM imports, e.g. the EM from core, or even the MikroORM object. Clearly the @mikro-orm/nestjs package is resolving to one @mikro-orm/core package and your app resolves to another.

I mean, we could make the core/knex packages as peer deps for CLI, we do require it to be installed locally anyway, but it feels a bit breaking (global installation of the CLI package would probably fail with some type/import errors).

➜  server git:(master) ✗ pnpm why @mikro-orm/core
Legend: production dependency, optional only, dev only

@test/server /Users/adamek/htdocs/mikro-orm/mikro-orm-cli-issue/packages/server

dependencies:
@mikro-orm/cli 5.3.1
├─┬ @mikro-orm/core 5.3.1
│ └─┬ @mikro-orm/postgresql 5.3.1 peer
│   ├── @mikro-orm/core 5.3.1 peer
│   └─┬ @mikro-orm/knex 5.3.1
│     └── @mikro-orm/core 5.3.1 peer
├─┬ @mikro-orm/knex 5.3.1
│ └─┬ @mikro-orm/core 5.3.1 peer
│   └─┬ @mikro-orm/postgresql 5.3.1 peer
│     ├── @mikro-orm/core 5.3.1 peer
│     └─┬ @mikro-orm/knex 5.3.1
│       └── @mikro-orm/core 5.3.1 peer
└─┬ @mikro-orm/postgresql 5.3.1 peer
  ├── @mikro-orm/core 5.3.1 peer
  └─┬ @mikro-orm/knex 5.3.1
    └── @mikro-orm/core 5.3.1 peer
@mikro-orm/core 5.3.1
└─┬ @mikro-orm/postgresql 5.3.1 peer
  ├── @mikro-orm/core 5.3.1 peer
  └─┬ @mikro-orm/knex 5.3.1
    └── @mikro-orm/core 5.3.1 peer
@mikro-orm/nestjs 5.1.0
└─┬ @mikro-orm/core 5.3.1 peer
  └─┬ @mikro-orm/postgresql 5.3.1 peer
    ├── @mikro-orm/core 5.3.1 peer
    └─┬ @mikro-orm/knex 5.3.1
      └── @mikro-orm/core 5.3.1 peer
@mikro-orm/postgresql 5.3.1
├── @mikro-orm/core 5.3.1 peer
└─┬ @mikro-orm/knex 5.3.1
  └── @mikro-orm/core 5.3.1 peer

@duckies
Copy link
Author

duckies commented Aug 15, 2022

I have found others online who have similar issues with a myriad of nestjs modules because pnpm can resolve even pinned dependencies to different hashes. I agree trying to resolve this within the mikro-orm ecosystem is unnecessary and I'll close this issue. I will return to using yarn, or at least a package manager with a flat node_modules folder without all of the symlinking.

@duckies duckies closed this as not planned Won't fix, can't repro, duplicate, stale Aug 15, 2022
@B4nan
Copy link
Member

B4nan commented Aug 15, 2022

It would be indeed good to support pnpm but I am having hard times to even debug this, when going thru the symlinks it looks good to me 🤷 But pnpm why kinda confirms there are two installs.

I was thinking how to get around this, but it all seems semi breaking, so would rather keep that for v6. But we could have the CLI package with only peer deps and change how we import things in that package to not fall apart in case the deps are not there (we just need nice error handling, no special functionality). The mikro-orm metapackage could work the same as e.g. jest CLI - it would depend on the @mikro-orm/cli and prefer its locally installed version (there is an npm package for this).

@B4nan
Copy link
Member

B4nan commented Sep 1, 2022

A user reported this on slack, and we found out it was caused by depending on wrong version of pg package (which in fact should not be in the user dependencies at all unless used explicitly - its a transitive dependency of the driver package).

Maybe you had the same problem, will check the repro again to verify. I will have time for this next week probably, lets reopen so we dont forget. Want to give it another try knowing this information, I would very much like to solve this.

@B4nan B4nan reopened this Sep 1, 2022
@productdevbook
Copy link

@duckies
"pg": "8.7.3", pg version temporary solution install this

@B4nan
Copy link
Member

B4nan commented Sep 1, 2022

"pg": "8.7.3", pg version temporary solution install this

Generally its not about installing this exact version, its about either not installing it at all, or dependeing on the same version as the driver does.

And in this repro I don't see such problem, its quite bare bones... https://github.com/duckies/mikro-orm-cli-issue/blob/master/packages/server/package.json

Maybe its some dependency that is shared with nest.

@B4nan
Copy link
Member

B4nan commented Sep 13, 2022

Gave this another try, and pnpm is so weird. Only way I was able to make this work was by installing @mikro-orm/knex explicitly and using that to import SqlEntityManager.

FYI this is not about the CLI package, I get the same issue if I remove it. To me this sound like a pnpm resolution bug.

B4nan added a commit that referenced this issue Sep 13, 2022
@B4nan
Copy link
Member

B4nan commented Sep 13, 2022

Looks like stating all the ORM packages as peer deps on the nestjs adapter helped - but it works only if I dont install the @mikro-orm/knex explicitly.

So if you have the @mikro-orm/knex installed, you need to use that for importing SqlEntityManager, otherwise it should work with v5.1.2 of the nestjs adapter.

@B4nan B4nan closed this as completed Sep 13, 2022
@bartvanremortele
Copy link

@B4nan I can confirm that importing from the knex package and injecting SqlEntityManager works. In our case the @mikro-orm/knex dependency was not explicitly installed but I still had this exact same issue. It was being pulled in as a dependency of @mikro-orm/postgresql

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants