needle-innovision / nestjs-tenancy

Multi-tenancy approach for nestjs - currently supported only for mongodb with mongoose
MIT License
186 stars 58 forks source link

Could we please upgrade the version to 8.0.0 ? #24

Closed Uter1007 closed 2 years ago

Uter1007 commented 2 years ago

https://github.com/needle-innovision/nestjs-tenancy/blob/49ce2cdb2ac87546eeb73b678f175b8509ae947c/package.json#L55

cheers, Christoph

raphaelarias commented 2 years ago

@sandeepsuvit I was exploring creating a PR to update to 8.0.0. But e2e tests are failing (maybe I did something wrong).

Do you believe the package will be updated? If not, are you aware of consequences of using it with Nest 8+?

Thanks for the great library!

image
sandeepsuvit commented 2 years ago

Hi @raphaelarias i did try an upgrade. But was getting rxjs related errors from nest node_modules in the end. So had to revert the upgrade. Will try to attempt it once again.

raphaelarias commented 2 years ago

Answering my question above, not updating to 8.0.0 will prevent the usage of @nestjs/axios. I was trying to setup Terminus for health check and NPM is complaining about upstream dependency conflict.

Is there anything I can do to help upgrade it to 8.0.0?

snps-achim commented 2 years ago

Answering my question above, not updating to 8.0.0 will prevent the usage of @nestjs/axios. I was trying to setup Terminus for health check and NPM is complaining about upstream dependency conflict.

Is there anything I can do to help upgrade it to 8.0.0?

Same for me. It is a great library, but it need to be updated to 8.0.0. I am willing to help also if needed. Please let me know.

sandeepsuvit commented 2 years ago

Hi @raphaelarias and @snps-achim thanks for your kind feedback. Here is the branch that i have tried the upgrade on feature/nest-version8-upgrade

Replace the following files with the ones in /backup folder

I have already given it a try but was facing rxjs build error when running the command npm run build. However the test cases are passing with the above files though.

snps-achim commented 2 years ago

Hi @sandeepsuvit,

I have cloned the repo, used your feature branch, copied the files and I don't see any issues with broken dependencies. Apart from changing in package.json from "eslint": "^8.17.0" to "eslint": "^7.32.0", there are no errors. No conflicts reported, no FAILs during tests.

Note:: I am using npm@latest

>npm --version
8.12.2
>npm install
npm ERR! code ERESOLVE
npm ERR! ERESOLVE unable to resolve dependency tree
npm ERR!
npm ERR! While resolving: @needle-innovision/nestjs-tenancy@1.0.21
npm ERR! Found: eslint@8.18.0
npm ERR! node_modules/eslint
npm ERR!   dev eslint@"^8.17.0" from the root project
npm ERR!
npm ERR! Could not resolve dependency:
npm ERR! peer eslint@"^5.0.0 || ^6.0.0 || ^7.0.0" from @typescript-eslint/parser@4.33.0
npm ERR! node_modules/@typescript-eslint/parser
npm ERR!   dev @typescript-eslint/parser@"^4.29.2" from the root project
npm ERR!   peer @typescript-eslint/parser@"^4.0.0" from @typescript-eslint/eslint-plugin@4.33.0
npm ERR!   node_modules/@typescript-eslint/eslint-plugin
npm ERR!     dev @typescript-eslint/eslint-plugin@"^4.29.2" from the root project
npm ERR!
npm ERR! Fix the upstream dependency conflict, or retry
npm ERR! this command with --force, or --legacy-peer-deps
npm ERR! to accept an incorrect (and potentially broken) dependency resolution.

Fixing eslint in package.json from "eslint": "^8.17.0" to "eslint": "^7.32.0"

npm install

added 1009 packages, and audited 1010 packages in 8m

161 packages are looking for funding
  run `npm fund` for details

5 moderate severity vulnerabilities

To address all issues, run:
  npm audit fix --force

Run `npm audit` for details.
npm run build

> @needle-innovision/nestjs-tenancy@1.0.21 build
> rm -rf dist && tsc -p tsconfig.json
npm run test:e2e

> @needle-innovision/nestjs-tenancy@1.0.21 test:e2e
> jest --config ./tests/jest-e2e.json --runInBand

 PASS  tests/e2e/cat-tenancy.spec.ts (37.019 s)
 PASS  tests/e2e/dog-tenancy.spec.ts (9.521 s)

Test Suites: 2 passed, 2 total
Tests:       2 passed, 2 total
Snapshots:   0 total
Time:        47.844 s
Ran all test suites.
sandeepsuvit commented 2 years ago

@snps-achim thanks for the update. Not sure why did it show the error on my end for the first time. But now it's passing.

The upgrade to version 8 is complete - Release notes.

Please verify if the changes are working for you

raphaelarias commented 2 years ago

Just saw the thread. So far it's working great! I'll keep testing it, just in case. This is amazing, thank you very much!

sandeepsuvit commented 2 years ago

I am closing this issue for now. Thanks for the support