fix(core): include PNPM patches in externalDependencies hash computations#33551
fix(core): include PNPM patches in externalDependencies hash computations#33551leosvelperez merged 3 commits intonrwl:masterfrom
Conversation
👷 Deploy request for nx-docs pending review.Visit the deploys page to approve it
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
View your CI Pipeline Execution ↗ for commit 6bc7333
☁️ Nx Cloud last updated this comment at |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Nx Cloud has identified a flaky task in your failed CI:
Since the failure was identified as flaky, the solution is to rerun CI. Because this branch comes from a fork, it is not possible for us to push directly, but you can rerun by pushing an empty commit:
git commit --allow-empty -m "chore: trigger rerun"
git push
🎓 Learn more about Self-Healing CI on nx.dev
leosvelperez
left a comment
There was a problem hiding this comment.
Thanks for the contribution!
It's mostly fine, but we need to handle the different formats the keys in patchedDependencies can have.
a86c4bf to
fc162ba
Compare
| // find the position of the '@' | ||
| return key.slice(0, key.indexOf('@', 1)); | ||
| const atIndex = key.indexOf('@', 1); | ||
| return atIndex === -1 ? key : key.slice(0, atIndex); |
There was a problem hiding this comment.
As a note, I updated this so that it works with "packageName" specifiers to return the original. I felt that made the helper a little more predictable and cleaned up my code a bit :)
fc162ba to
3f7fcc2
Compare
3f7fcc2 to
6bc7333
Compare
leosvelperez
left a comment
There was a problem hiding this comment.
Thanks again for your contribution! LGTM!
I went ahead with some minor tweaks (refactored the patchEntries array to use a Map, reverted some unrelated lock file changes, etc.) to speed up the feedback and get this merged.
Ah thanks @leosvelperez. Sorry, I have to remove the lock file stuff every time I run pnpm and forgot. Probably a repo difference or ambiguous config. Appreciate it! |
…ions (#33551) ## Current Behavior If a workspace uses pnpm and adds a local patch to a dependency (e.g. `vitest`), this patch is not taken into account when computing that dependency's hash for purposes of determining cache changes. In practice, you could patch vitest locally, and tests would pull from the cache. ## Expected Behavior Patches can alter behavior in the same way that updating the version could, it's just that the version is not created and the patch is applied locally. This updates the pnpm lockfile parsing functionality to read the patches field into a map and then combine the patch hash with the integrity hash. The integrity hash ONLY represents the remote / tarball intergrity, so these have to be combined in order to create a proper key. ## Related Issue(s) (Could not find any, but saw this in my work today) --------- Co-authored-by: Leosvel Pérez Espinosa <leosvel.perez.espinosa@gmail.com>
|
This pull request has already been merged/closed. If you experience issues related to these changes, please open a new issue referencing this pull request. |
Current Behavior
If a workspace uses pnpm and adds a local patch to a dependency (e.g.
vitest), this patch is not taken into account when computing that dependency's hash for purposes of determining cache changes. In practice, you could patch vitest locally, and tests would pull from the cache.Expected Behavior
Patches can alter behavior in the same way that updating the version could, it's just that the version is not created and the patch is applied locally.
This updates the pnpm lockfile parsing functionality to read the patches field into a map and then combine the patch hash with the integrity hash. The integrity hash ONLY represents the remote / tarball intergrity, so these have to be combined in order to create a proper key.
Related Issue(s)
(Could not find any, but saw this in my work today)