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

pnpm-lock.yaml is no longer updated #234

Closed
5 tasks done
StarpTech opened this issue Jun 29, 2022 · 8 comments
Closed
5 tasks done

pnpm-lock.yaml is no longer updated #234

StarpTech opened this issue Jun 29, 2022 · 8 comments
Labels
bug Something isn't working external lib

Comments

@StarpTech
Copy link
Contributor

StarpTech commented Jun 29, 2022

Describe the bug

See CI Job
The command is executed but the updated lock file was not committed wundergraph/wundergraph@b9dea13

Expectation

I miss the line

 git add -- packages/sdk/package.json packages/sdk/CHANGELOG.md packages/nextjs/package.json packages/nextjs/CHANGELOG.md pnpm-lock.yaml

Reproduction

No idea

Lerna config and logs

lerna.json

<!-- Please paste your `lerna.json` here -->

lerna-debug.log

<!-- If you have a `lerna-debug.log` available, please paste it here -->
<!-- Otherwise, feel free to delete this <details> block -->

Environment Info

| Executable        | Version |
| ----------------- | ------- |
| `lerna --version` | 1.5.1 |
| `npm --version`   | VERSION |
| `yarn --version`  | VERSION |
| `node --version`  | VERSION |

---
OR simply run `npx lerna info` command and copy+paste the result here

Used Package Manager

pnpm

Validations

@ghiscoding
Copy link
Member

ghiscoding commented Jun 29, 2022

yeah that is actually pnpm itself which seem to have broke it with their last release v7.4.0 on yesterday (specifically this PR to exit early on --lockfile-only). I found that out yesterday when pushing a small readme update yesterday and my unit tests related to pnpm started failing, see PR #233 where I tested couple things and this pnpm comment I left for pnpm's maintainer. I only identified it yesterday through my unit tests but in some ways I'm glad you actually confirmed the issue. I have a possible quick fix for this in my PR #233 by calling --fix-lockfile but I'm not crazy about having this patch to make it work the way it should. Perhaps I could release it as a quick fix and rollback whenever pnpm addresses the regression issue in a better way, what do you think?

EDIT

Opened a regression issue on pnpm project

@StarpTech
Copy link
Contributor Author

@ghiscoding thank you for the fast feedback and fix. What's the purpose of --lockfile-only when the command does not update the lock file? 😕 Maybe, I don't understand the difference between --lockfile-only and --fix-lockfile. I like your approach to calling both. Nothing could go wrong.

@ghiscoding
Copy link
Member

ghiscoding commented Jun 29, 2022

The --fix-lockfile works because oh how the pnpm change was applied in the PR that I referenced, if you look at the changes here you see that he removed the --lockfile-only to make it exist early if no changes is detected (that is the breaking change for us) and if you look at the remaining of that if condition that he modified then you can see that --fix-lockfile is still part of the condition (previous behavior) and so I decided to give it a try and saw that it does work to get my unit tests working again in my PR #233, basically it's a bypass that I found for the new regression bug in pnpm to get their previous behavior, I don't like it but it's a quick fix until the regression is addressed (hopefully).

I'll look at releasing a new version later tonight including this patch fix (PR #233), but I still hope that pnpm revert their change or fix their regression eventually in a better way. Waiting for their reply on the matter

@StarpTech
Copy link
Contributor Author

Nice 👍

@ghiscoding
Copy link
Member

@StarpTech by the way, my unit tests only failed in the CI but they seem to pass locally even with latest pnpm version. Was your new release done via CI or from your local computer? I'd like to know so that I can update the pnpm issue with relevant info. Thanks

@StarpTech
Copy link
Contributor Author

From the CI configured to the latest version.

@ghiscoding
Copy link
Member

ok so it seems to fail only in CI mode so far, by the way to make it clear the quick fix is to use pnpm install --lockfile-only --fix-lockfile so it's now 2 flags instead of just 1, I think I didn't explain well earlier.

@ghiscoding
Copy link
Member

ghiscoding commented Jun 30, 2022

release quick fix under v1.6.0 even though pnpm is expected to release a fix for the regression within the next day. Also note that pnpm maintainer gave me another alternative instead of the fix lock which I'm now using pnpm install --lockfile-only --no-frozen-lockfile. I might revert/remove this no frozen flag in the short future after pnpm release their new version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working external lib
Projects
None yet
Development

No branches or pull requests

2 participants