Skip to content

fix(libnpmexec): workspaces support#4643

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.

Sign up for GitHub

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

Jump to bottom
Merged
lukekarrys merged 1 commit into from
Apr 19, 2022
Merged

fix(libnpmexec): workspaces support #4643

lukekarrys merged 1 commit into from
Apr 19, 2022

Conversation

@ruyadorno
Copy link
Contributor

@ruyadornoruyadorno commented Mar 30, 2022•
edited
Loading

Fixes the proper path location to use when targetting specific workspaces.

References

Fixes: #3520
Relates to: npm/statusboard#403

@ruyadornoruyadorno requested a review from a team as a code ownerMarch 30, 2022 21:53
@ruyadornoruyadorno added semver:patch semver patch level for changes Release 8.x work is associated with a specific npm 8 release release: next These items should be addressed in the next release labels Mar 30, 2022
@ruyadornoruyadornoforce-pushed the fix-npm-exec-for-workspaces branch 2 times, most recently from 0b405af to dd6530eCompareMarch 31, 2022 03:05
nlf
nlf reviewed Mar 31, 2022
View reviewed changes
@nlfnlf mentioned this pull request Apr 11, 2022
Closed
1 task
@ruyadornoruyadornoforce-pushed the fix-npm-exec-for-workspaces branch from dd6530e to 1175520CompareApril 11, 2022 18:53
@ruyadorno
Copy link
ContributorAuthor

ruyadorno commented Apr 11, 2022

@nlf after jumping back at the code here I realized that I could get the original bugfix without explicit requiring that arborist-read-data-refactor 😊 I'm going to split these into two separate PRs so that they're more atomic, simpler to review and revert if needed be.

So here now is only the change required in lib/commands/exec.js in order to properly use localPrefix as the place to load arborist when running in the context of workspaces.

nlf
nlf approved these changes Apr 11, 2022
View reviewed changes
@ruyadornoruyadorno marked this pull request as draft April 11, 2022 19:05
@ruyadorno
fix(exec): workspaces support
7c1faf5
Fixes the proper path location to use when targetting specific workspaces. Fixes: npm#3520 Relates to: npm/statusboard#403
@ruyadornoruyadornoforce-pushed the fix-npm-exec-for-workspaces branch from 1175520 to 7c1faf5CompareApril 19, 2022 14:32
@ruyadornoruyadorno marked this pull request as ready for review April 19, 2022 14:33
ruyadorno added a commit to ruyadorno/cli that referenced this pull request Apr 19, 2022
@ruyadorno
fix(libnpmexec): fix read mixed local/registry pkg
9adb8f1
Refactor / clean up of the logic around reading installed packages. Fixes reading packages from mixed sources (one being from the local installed tree and the other from the registry using pacote.manifest). Makes it so that libnpmexec is always reading from the Arborist actual tree instead of reading `node_modules` from the file system when retrieving local package data. Fixes: npm#3668 Relates to: npm#4643 Relates to: npm#4619 Relates to: npm/statusboard#403
@ruyadornoruyadorno mentioned this pull request Apr 19, 2022
Merged
nlf
nlf approved these changes Apr 19, 2022
View reviewed changes
Copy link
Contributor

@nlfnlf left a comment

Choose a reason for hiding this comment

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

fix looks good, and these tests are great!

lukekarrys pushed a commit that referenced this pull request Apr 19, 2022
@ruyadorno@lukekarrys
fix(libnpmexec): fix read mixed local/registry pkg
4a46a27
Refactor / clean up of the logic around reading installed packages. Fixes reading packages from mixed sources (one being from the local installed tree and the other from the registry using pacote.manifest). Makes it so that libnpmexec is always reading from the Arborist actual tree instead of reading `node_modules` from the file system when retrieving local package data. Fixes: #3668 Relates to: #4643 Relates to: #4619 Relates to: npm/statusboard#403
@lukekarryslukekarrys merged commit 6253d19 into npm:latestApr 19, 2022
@ruyadornoruyadorno mentioned this pull request Apr 26, 2022
Merged
@npm-robotnpm-robot mentioned this pull request Apr 27, 2022
Merged
@gavinsharpgavinsharp mentioned this pull request Sep 14, 2022
Open
2 tasks
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@nlfnlfnlf approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

release: nextThese items should be addressed in the next releaseRelease 8.xwork is associated with a specific npm 8 releasesemver:patchsemver patch level for changes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[BUG] npx/npm exec don't work in child workspace when using --no/--no-install and --package

3 participants

@ruyadorno@nlf@lukekarrys