-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
fixing dependency bot alert #12267
fixing dependency bot alert #12267
Conversation
Signed-off-by: Rameez Sajwani <rameezwazirali@hotmail.com>
Review ChecklistHello reviewers! 👋 Please follow this checklist when reviewing this Pull Request. General
If a new flag is being introduced:
If a workflow is added or modified:
Bug fixes
Non-trivial changes
New/Existing features
Backward compatibility
|
Signed-off-by: Rameez Sajwani <rameezwazirali@hotmail.com>
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.
I don't think this is the right way to fix it. It looks like the dependency resolving was manually modified which isn't correct.
We either need to explicitly force this dependency to a later version using something like https://docs.npmjs.com/cli/v8/configuring-npm/package-json#overrides, or move away from create-react-app
.
This specific nth-check
issue is not really a pressing security issue (see facebook/create-react-app#11174), but the bigger concern is that the project seems to be entirely unmaintained. So we should probably switch away from that for that reason and use something like Vite instead.
Also cc @notfelineit since in another PR we also talked about upgrading things in general, so maybe we should bite the bullet and move to Vite as well then all in 1 go.
#12270 uses the override to get rid of the broken dependency. We need to either do that, or move away from
I don't see how this is possible. The changes here show that for an existing package, the dependency was overwritten which is only possible if you manually change the |
@dbussink , should I close this PR then given you mention this is not the right way to resolve this dependency |
closing this PR in favor of #12270 |
Signed-off-by: Rameez Sajwani rameezwazirali@hotmail.com
Fix vulnerability define here
Seeing this warning when we push any PR
I did npm install to fix this issue.
Related Issue(s)
alert
Checklist
Deployment Notes