Skip to content

fix(arborist): only replace hostname for resolved URL #8185

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

Merged
merged 3 commits into from
Mar 26, 2025

Conversation

billy-briggs-dev
Copy link
Contributor

Related to #6110

There is an edge case where if an npm project is using a registry with a path the replace-registry-host is including the path from the registry url.

Current Behavior:

Expected Behavior:

This fix resolves this inconsistency by refactoring the logic to construct URL objects from the registry and resolved strings and maps the hostname of the new registry to the hostname of the resolved url instead of doing string parsing.

I'm welcome to any feedback on this solution!

@billy-briggs-dev billy-briggs-dev requested a review from a team as a code owner March 26, 2025 19:13
@owlstronaut
Copy link
Contributor

This is great, thanks for your contribution!

@owlstronaut owlstronaut merged commit 885accd into npm:latest Mar 26, 2025
16 checks passed
@github-actions github-actions bot mentioned this pull request Mar 26, 2025
wraithgar pushed a commit that referenced this pull request Jun 10, 2025
…ls (#8356)

When custom hosted git url are used to install dependencies it's not
installing that dependency when the url is of
`git+ssh://[email protected]:a/b/c.git` format instead of
`git+ssh://[email protected]/a/b/c.git`.

#### Cause:
` new URL(resolved)` throws an error which is getting caught and in-turn
results in removal of that node.
This behaviour was working as expected before this
#8185 which replaced
`hgi.parseUrl(resoved)` call to `new URL(resolved)`

#### Fix:
keep the `hgi.parseUrl` call to correctly parse the git url as
mentioned/explained in this PR #5758

Fixes: #8331
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