-
Notifications
You must be signed in to change notification settings - Fork 994
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
Fix PNPM issues with private registries #8330
Fix PNPM issues with private registries #8330
Conversation
Thank you for tagging me! I am not too familiar with the ecosystem but I understand the problem. Any chance you can add a spec? |
Actually I meant to ping @mburumaxwell 🤦♂️, sorry about that. Thanks for chiming in though! Yes, this is a draft because I plan to add some specs 👍. |
0228de9
to
1eef40f
Compare
@dancallaghan This is now ready, I'll deploy it next week! |
npm_and_yarn/lib/dependabot/npm_and_yarn/update_checker/registry_finder.rb
Show resolved
Hide resolved
npm_and_yarn/lib/dependabot/npm_and_yarn/update_checker/registry_finder.rb
Outdated
Show resolved
Hide resolved
npm_and_yarn/lib/dependabot/npm_and_yarn/update_checker/registry_finder.rb
Show resolved
Hide resolved
1eef40f
to
e7bffce
Compare
Thanks for reviewing @yeikel, hopefully I addressed your comments. @dancallaghan If you're able to try this again, I appreciate. I introduced a few refactoring so it'd be good to know I did not break anything! |
Thanks @deivid-rodriguez I've tested locally with the latest changes on our use case and it's working as expected 👍 |
…ecessary If registries & scopes have been explicitly configured, trust that.
e7bffce
to
7c0354a
Compare
@deivid-rodriguez, It seems like the changes to the lockfile in pnpm v9 mean these improvements no longer apply. The performance issues caused by dependabot checking non-standard repositories for all packages rather than the ones within configured namespaces have resurfaced when using pnpm v9 Dependabot seems to want to check all packages against my private repository rather than only once in the given namespace - the same as #8242 (comment) |
I no longer actively contribute to this repo, so I won't be able to help right now, but thanks for letting me know! |
This PR fixes two issues:
This is handy because developers don't need to commit a
.npmrc
file and matches what we already support for NPM and Yarn..npmrc
.This is a good last resort fallback, but it should not be used if registries & scopes have been explicitly configured.
Fixes #8242.
Fixes performance issues introduced by #8094.
Likely to fix #8008.