Skip to content

fix(resolve-dependencies): not update git protocol dependency when add unrelated dependency#7054

Merged
zkochan merged 7 commits intopnpm:mainfrom
await-ovo:fix-git-protocol-update-issue
Sep 23, 2023
Merged

fix(resolve-dependencies): not update git protocol dependency when add unrelated dependency#7054
zkochan merged 7 commits intopnpm:mainfrom
await-ovo:fix-git-protocol-update-issue

Conversation

@await-ovo
Copy link
Copy Markdown
Member

fix #7008

@await-ovo await-ovo requested a review from zkochan as a code owner September 5, 2023 06:03
… unrelated dependency

docs: add changeset

chore: package.json
@await-ovo await-ovo force-pushed the fix-git-protocol-update-issue branch from 3c68847 to d76b2d0 Compare September 5, 2023 08:37
@await-ovo await-ovo marked this pull request as draft September 5, 2023 16:12
@await-ovo await-ovo marked this pull request as ready for review September 7, 2023 09:59
dev: false

github.com/kevva/is-negative/219c424611ff4a2af15f7deeff4f93c62558c43d:
resolution: {registry: http://localhost:4873/, tarball: https://codeload.github.com/kevva/is-negative/tar.gz/219c424611ff4a2af15f7deeff4f93c62558c43d}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why is there a registry field here? There should be no registry field for the git-hosted resolutions.

But even if I am not right, then I guess the registry field would have to be https://github.com

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.

Thanks for pointing out the problem here. It should be my mistake here. I can't remember where it came from.

if (
Boolean((pkgSnapshot.resolution as TarballResolution).type) ||
(pkgSnapshot.resolution as TarballResolution).tarball?.startsWith('file:')
(pkgSnapshot.resolution as TarballResolution).tarball?.startsWith('file:') || isGitHostedPkgUrl((pkgSnapshot.resolution as TarballResolution).tarball ?? '')
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This change doesn't really make a difference because git-hosted deps should not have a registry field.

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.

For git-hosted dependencies that are already installed, the resolution is now also read from the lockfile (except for pnpm update), and if we don't exclude the git-hosted dependencies here, the new resolution will come with the registry info.

The registry(http://localhost:4873) in the test fixture above should be the result of not excluding the git-hosted dependency in my local tests

@zkochan zkochan merged commit f394cfc into pnpm:main Sep 23, 2023
nachoaldamav pushed a commit that referenced this pull request Sep 26, 2023
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.

Installing packages bumps hash for unrelated git packages in lock file

2 participants