-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
chore(dev-deps): Bump nestjs deps & add nestjs v8 E2E test #14148
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
Conversation
@@ -50,8 +50,8 @@ | |||
"@sentry/utils": "8.36.0" | |||
}, | |||
"devDependencies": { | |||
"@nestjs/common": "^8.0.0 || ^9.0.0 || ^10.0.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMHO no need to define this like this, this should define the actual version we want to use locally.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea!
8ef902e
to
b9a70a5
Compare
For whatever reason, 10.4.6 is failing in node-integration-tests. there is also no changelog there, so maybe there was some problem ??
8d16fa9
to
28858df
Compare
Note for the future: There seems to be some problem with the node-integration-tests and module resolution. As soon as the nestjs version there matches the one from the nestjs package - which means it will hoist the dependency to the workspace-level node_modules folder - the tests start failing weirdly. For now, I "fixed" this by using a different version of nestjs in node-integration-tests from nestjs. Since we want to get rid of the node-specific part there anyhow in v9, I figured it's not worth it to put more work into fixing this properly... cc @chargome |
We support v8 but only test v10. By adding a (very basic) e2e test app for v8 of nestjs we can ensure this continues to work (as long as we want to support it).
Also bumps nestjs dev deps to hopefully fix some security warnings for outdates deps (e.g. https://github.com/getsentry/sentry-javascript/security/dependabot/376).
Note for the future: There seems to be some problem with the node-integration-tests and module resolution. As soon as the nestjs version there matches the one from the nestjs package - which means it will hoist the dependency to the workspace-level node_modules folder - the tests start failing weirdly. For now, I "fixed" this by using a different version of nestjs in node-integration-tests from nestjs. Since we want to get rid of the node-specific part there anyhow in v9, I figured it's not worth it to put more work into fixing this properly...