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

lockfile v9 support #62

Closed
NamesMT opened this issue Apr 26, 2024 · 19 comments · Fixed by #64
Closed

lockfile v9 support #62

NamesMT opened this issue Apr 26, 2024 · 19 comments · Fixed by #64

Comments

@NamesMT
Copy link
Contributor

NamesMT commented Apr 26, 2024

After updating to pnpm@9, nolyfill (when ran with npx/bunx/pnpx/pnpm dlx) exits with error: e.startsWith is not a function.

Error does not happens with pnpm@8. is caused by lockfile v9 by pnpm@9
Reproducible with any fresh package, require the package.json to have at least one installed package and run pnpm i with pnpm@9 for v9 lockfile

Reproduce one-liner: docker run -it --rm namesmt/images-alpine:node-dev zsh -c "cd home && na init && ni unocss && nlx nolyfill -d"
pnpm@8 working one-liner: docker run -it --rm namesmt/images-alpine:node-dev_pnpm8.15.7 zsh -c "cd home && na init && ni unocss && nlx nolyfill -d"

@SukkaW
Copy link
Owner

SukkaW commented Apr 26, 2024

I will look into this.

@NamesMT
Copy link
Contributor Author

NamesMT commented Apr 26, 2024

Hi @SukkaW, maybe ignore this, just decided spinned up a new docker instance with pnpm@9 but it doesn't have this error.

For some reason nolyfill doesn't work in my main environment anymore after upgrading to pnpm@9 a while ago, will investigate more on this.

@NamesMT NamesMT closed this as completed Apr 26, 2024
@NamesMT
Copy link
Contributor Author

NamesMT commented Apr 26, 2024

Here's the full debug log for now:

I'll try to test what caused it if possible after a few days.

/home/eprm/my-hono-app …
➜ nlx nolyfill -d
Trace: TypeError: e.startsWith is not a function
    at sV.refToRelative (/root/.cache/pnpm/dlx/fiazrcs3p4mgptwq6srgws2tue/18f19c5b71d-3bab/node_modules/.pnpm/[email protected]/node_modules/nolyfill/dist/package-manager-6inIkRRx.js:6:34251)
    at yT.getPkgInfo (/root/.cache/pnpm/dlx/fiazrcs3p4mgptwq6srgws2tue/18f19c5b71d-3bab/node_modules/.pnpm/[email protected]/node_modules/nolyfill/dist/package-manager-6inIkRRx.js:7:108296)
    at /root/.cache/pnpm/dlx/fiazrcs3p4mgptwq6srgws2tue/18f19c5b71d-3bab/node_modules/.pnpm/[email protected]/node_modules/nolyfill/dist/package-manager-6inIkRRx.js:7:115340
    at Array.forEach (<anonymous>)
    at DL (/root/.cache/pnpm/dlx/fiazrcs3p4mgptwq6srgws2tue/18f19c5b71d-3bab/node_modules/.pnpm/[email protected]/node_modules/nolyfill/dist/package-manager-6inIkRRx.js:7:115299)
    at async /root/.cache/pnpm/dlx/fiazrcs3p4mgptwq6srgws2tue/18f19c5b71d-3bab/node_modules/.pnpm/[email protected]/node_modules/nolyfill/dist/package-manager-6inIkRRx.js:7:114377
    at async Promise.all (index 0)
    at async Dx (/root/.cache/pnpm/dlx/fiazrcs3p4mgptwq6srgws2tue/18f19c5b71d-3bab/node_modules/.pnpm/[email protected]/node_modules/nolyfill/dist/package-manager-6inIkRRx.js:7:114341)
    at async Object.n [as searchForPackages] (/root/.cache/pnpm/dlx/fiazrcs3p4mgptwq6srgws2tue/18f19c5b71d-3bab/node_modules/.pnpm/[email protected]/node_modules/nolyfill/dist/package-manager-6inIkRRx.js:12:2542)
    at async /root/.cache/pnpm/dlx/fiazrcs3p4mgptwq6srgws2tue/18f19c5b71d-3bab/node_modules/.pnpm/[email protected]/node_modules/nolyfill/dist/package-manager-6inIkRRx.js:20:71505
    at /root/.cache/pnpm/dlx/fiazrcs3p4mgptwq6srgws2tue/18f19c5b71d-3bab/node_modules/.pnpm/[email protected]/node_modules/nolyfill/dist/cli.js:28:292

@SukkaW
Copy link
Owner

SukkaW commented Apr 26, 2024

I am able to reproduce the error with pnpx nolyfill, but I can't reproduce the error with npx nolyfill though.

@NamesMT
Copy link
Contributor Author

NamesMT commented Apr 26, 2024

Ahhhh just tried again with a fresh instance, previously I tested without any package inside of package.json, after installing any package, the issue could be reproduced with pnpm@9
Guess we could re-open this issue.
pnpx (pnpm dlx) worked with pnpm@8.

Updated the OG post with more detail.

@NamesMT NamesMT reopened this Apr 26, 2024
@huijiewei
Copy link

Same problem

@Akkuma
Copy link

Akkuma commented May 1, 2024

Also seeing this error with npx and bunx

@NamesMT NamesMT changed the title pnpm 9 support lockfile v9 support May 9, 2024
@NamesMT
Copy link
Contributor Author

NamesMT commented May 9, 2024

@SukkaW just wanna ping you to clarify, seems to be caused by lockfile v9, and is happenning in all package manager execute, including npx/bunx

@nonzzz
Copy link
Contributor

nonzzz commented May 14, 2024

I think this fix break the previous behavior. When i'm using v1.0.30

image

v1.0.31

image

I print the parser result. Notice is not right.

My local env is wsl2 , NodeJs v16.14.0 and my pnpm-lock version is 5.3

@nonzzz
Copy link
Contributor

nonzzz commented May 14, 2024

This question should be reopen. This fix don't meet the document minimum support.

@SukkaW SukkaW reopened this May 14, 2024
@SukkaW
Copy link
Owner

SukkaW commented May 14, 2024

@NamesMT Is it possible to add old pnpm 8 support back?

@NamesMT
Copy link
Contributor Author

NamesMT commented May 14, 2024

I'll try if it's possible to support both,

@nonzzz What lockfile version are you using?, is it v5?

@nonzzz
Copy link
Contributor

nonzzz commented May 14, 2024

@NamesMT Right. my lockfile version is 5.3

@SukkaW
Copy link
Owner

SukkaW commented May 14, 2024

I'll try if it's possible to support both,

Maybe we can install both versions of pnpm utils, and manually choose the one bassd on lockfile version.

@NamesMT
Copy link
Contributor Author

NamesMT commented May 14, 2024

I'll try if it's possible to support both,

Maybe we can install both versions of pnpm utils, and manually choose the one bassd on lockfile version.

I'm thought about it but was afraid that it would increase the CLI bundle size too much.
I'll try to do it and report back with the precise number of the increase.

@SukkaW
Copy link
Owner

SukkaW commented May 14, 2024

I'm thought about it but was afraid that it would increase the CLI bundle size too much.

Let's give it a shot first. Correct behavior matters a lot.

@NamesMT
Copy link
Contributor Author

NamesMT commented May 14, 2024

@SukkaW PR is live: #65,

I've tested it with a fresh project of pnpm@7 (lockfile v5.4) and pnpm@9

New bundle size is 1.27MB after adding patches for @pnpm/list, 1.0.31 bundle is 1.19MB, about a ~6.7% increase.

@nonzzz
Copy link
Contributor

nonzzz commented May 14, 2024

I think this size is not matter, But if pnpm inadvertently breaks this behavior we also need to report it to pnpm.

@NamesMT
Copy link
Contributor Author

NamesMT commented May 14, 2024

@nonzzz it was my bad, pnpm did said they dropped support for lockfile v5 (pnpm@7 <=) but I didn't notice it and only tested pnpm@9 & pnpm@8 for #64.
Though IIRC, pnpm said they only support and maintains 2 latest major versions, so pnpm@7 and lower is now technically deprecated, unsafe and is no longer maintained.

@NamesMT NamesMT closed this as completed Jun 23, 2024
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 a pull request may close this issue.

5 participants