Skip to content

fix calling npm.org when there's no npmrc with replaces-base#5928

Merged
jakecoffman merged 2 commits intomainfrom
jakecoffman/npm-library-no-npmrc
Oct 21, 2022
Merged

fix calling npm.org when there's no npmrc with replaces-base#5928
jakecoffman merged 2 commits intomainfrom
jakecoffman/npm-library-no-npmrc

Conversation

@jakecoffman
Copy link
Copy Markdown
Member

Context

Currently, when there is no .npmrc file, but a registry has been specified in the credentials, Dependabot generates an .npmrc file with a global registry, assuming all of the resolved URLs match, and the update goes well.

However, once it gets to the library check it calls out to the default registry.

Replaces-base

The code for checking the lockfile seems to be npm specific, and its private in another class. So it will take a bit of a refactor to get that common code out.

I thought this was a good first step to give the user a way to set a global registry with the replaces-base flag.

We should probably circle back and make that npmrc generator code apply to the library check as well. Also use replaces-base in the npmrc generator when present.

This PR also fixes a few places where we assume the registry doesn't have a protocol. I think it's important to honor the protocol as customers may have private internal registries serving on http since they aren't exposed to the internet.

Let me know if you disagree with introducing replaces-base like this!

@jakecoffman jakecoffman requested a review from a team as a code owner October 19, 2022 18:19
Copy link
Copy Markdown
Contributor

@mctofu mctofu left a comment

Choose a reason for hiding this comment

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

Makes sense to have another option to specify the registry if .npmrc isn't used!

}]
end

it { is_expected.to eq("http://example.com") }
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It makes sense to preserve the protocol but it currently will have limited use. On dotcom/ghes:

  • the protocol is stripped on the backend
  • the proxy won't attach credentials to http requests

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Ah, that's too bad. It will be there if we need it.

@jakecoffman jakecoffman merged commit a145284 into main Oct 21, 2022
@jakecoffman jakecoffman deleted the jakecoffman/npm-library-no-npmrc branch October 21, 2022 15:48
@pavera pavera mentioned this pull request Oct 31, 2022
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.

2 participants