Skip to content

fix: get the final address by installing package through links#8842

Merged
zkochan merged 7 commits intopnpm:mainfrom
btea:fix/add-packages-redirect-new-url
Dec 11, 2024
Merged

fix: get the final address by installing package through links#8842
zkochan merged 7 commits intopnpm:mainfrom
btea:fix/add-packages-redirect-new-url

Conversation

@btea
Copy link
Member

@btea btea commented Dec 7, 2024

fix #8833

@btea btea requested a review from zkochan as a code owner December 7, 2024 15:02
@zkochan
Copy link
Member

zkochan commented Dec 8, 2024

I'd assume the change needs to be done here: https://github.com/pnpm/pnpm/blob/main/resolving/tarball-resolver/src/index.ts

But I don't really know what part and where gets cached. So, it would be good to have a test that reproduces the issue.

@btea
Copy link
Member Author

btea commented Dec 10, 2024

But I don't really know what part and where gets cached.

In the current use case, since https://pkg.pr.new/vite@main is a redirected link, if the latest link after the redirection is not obtained, then when the installation is repeated, since pkgResponse.body.id gets the same result each time, isNew will be false in the following code, and multiple installations will get the same result.

After the modification, the latest result with the commit hash is obtained each time to obtain the newest modification.

const isNew = !ctx.resolvedPkgsById[pkgResponse.body.id]

@btea
Copy link
Member Author

btea commented Dec 10, 2024

To verify this result, I created a pr for the test.

I repeatedly submitted modified code, then executed pnpm i https://pkg.pr.new/btea/utils/@btea/utils@5 many times, and found that the code downloaded for the first time was always installed.

When I used pd i https://pkg.pr.new/btea/utils/@btea/utils@5 to install, I could get the latest commit every time.

@btea
Copy link
Member Author

btea commented Dec 11, 2024

It looks like the direct request to https://registry.npmjs.org/is-positive/download/is-positive-3.1.0.tgz resulted in a 404.

We failed to capture the error and threw an error directly.

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.

avoid caching original tarball urls with redirections

2 participants