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

use overrides in package.json instead of force-resolutions #588

Merged
merged 1 commit into from
Nov 2, 2022

Conversation

glharper
Copy link
Member

@glharper glharper commented Nov 2, 2022

This should remove all vulnerabilities listed on npm install/npm run build and all build warnings related thereto.
Internal Ref ID 4404953

@@ -116,10 +116,16 @@
"uuid": "^8.3.0",
"ws": "^7.5.6"
},
"resolutions": {
"overrides": {
"extend": "3.0.2",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this just manually what "-force-resolution" use to do?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overrides allow a bit more granular control, where we can force a version when a dependency is a sub-dependency of a specific library. This link describes the override capability better than I can.

@glharper glharper merged commit 0ee23f6 into master Nov 2, 2022
@@ -94,7 +94,7 @@
"lint": "eslint -c .eslintrc.js --ext .ts src",
"civersion": "node ci/version.js",
"prepare": "npm run build",
"preinstall": "npm install --package-lock-only --ignore-scripts --no-audit && npx npm-force-resolutions"
"preinstall": "npm install --package-lock-only --ignore-scripts --no-audit"
Copy link

@anantoghosh anantoghosh Nov 2, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please read #549 (comment)
Running npm install in the preinstall script here will run it when a user installs this package messing up their node_modules. This also does not work with yarn or pnpm.
Use an alternate script name like setup to allow your developers to run this script.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants