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

fix: use hosted-git-info to parse registry urls #5758

Merged
merged 1 commit into from Nov 1, 2022
Merged

Conversation

lukekarrys
Copy link
Member

@lukekarrys lukekarrys commented Oct 26, 2022

Previously this was using new URL which would fail on some urls that
hosted-git-info is able to parse. But if we still get a url that can't
be parsed, we now set it to be removed from the tree instead of
erroring.

Fixes: #5278
Closes: #5487

@lukekarrys lukekarrys requested a review from a team as a code owner October 26, 2022 22:38
@lukekarrys lukekarrys changed the title lk/git ssh urls fix: use hosted-git-info to parse registry urls Oct 26, 2022
@lukekarrys lukekarrys marked this pull request as draft October 26, 2022 22:55
@lukekarrys
Copy link
Member Author

This change is located entirely within @npmcli/arborist and those tests pass. However, the commands/link.js tests fail due to paths such as file:../../../global-prefix/lib/node_modules/@myscope/bar which DO parse with new URL but not with hosted-git-info.parseUrl.

Converting this PR to a draft while I investigate further.

@lukekarrys
Copy link
Member Author

Falling back to new URL() if it can't be parsed with hosted-git-info works as a fix. I think landing that in hosted-git-info first is ideal, so I've made that PR here npm/hosted-git-info#176 and will update this branch when that lands.

Previously this was using `new URL` which would fail on some urls that
`hosted-git-info` is able to parse. But if we still get a url that can't
be parsed, we now set it to be removed from the tree instead of
erroring.

Fixes: #5278
@lukekarrys lukekarrys marked this pull request as ready for review October 27, 2022 22:45
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.

[BUG] git+ssh does not work with version 8.16
2 participants