fix(core): use workspace root for package manager detection in script targets#35550
fix(core): use workspace root for package manager detection in script targets#35550FrozenPandaz merged 6 commits intomasterfrom
Conversation
✅ Deploy Preview for nx-dev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for nx-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
View your CI Pipeline Execution ↗ for commit cbe8994
☁️ Nx Cloud last updated this comment at |
AgentEnder
left a comment
AgentEnder
left a comment
There was a problem hiding this comment.
Feel free to push back if you want, but I think this'd be cleaner if we moved where the package manager command was fetched. Previously it was once per workspace, now it'd be once per project etc and it adds an if around the for loop that wouldn't otherwise be needed.
Its definitely pretty minor, but I'd like it changed.
| const packageManagerCommand = getPackageManagerCommand( | ||
| detectPackageManager(workspaceRoot), | ||
| workspaceRoot | ||
| ); |
There was a problem hiding this comment.
This is a minor degradation, we'll be detecting the package manager / getting the command set once per project instead of once per module lifetime. I'd prefer to move this outside of here, unless you see a good reason to re-detect when reading the targets per package.json...
If we don't want to have it at module level, I'd be fine with passing it inot the readTargetsFromPackageJson fn or something
compute packageManagerCommand once per workspace at the createNodesV2 entry point and thread it through createNodeFromPackageJson and buildProjectConfigurationFromPackageJson into readTargetsFromPackageJson, instead of detecting per package.json read. addresses review feedback on #35550.
… targets readTargetsFromPackageJson received a workspaceRoot argument but never passed it to detectPackageManager / getPackageManagerCommand. Detection fell back to npm_config_user_agent, so the inferred npm run command on script targets depended on whoever invoked the process rather than on the actual workspace lockfile. The result was visible to users running nx in a workspace whose lockfile disagrees with their invoking shell's package manager. A module-level `let packageManagerCommand` cache also kept the first detection result across all subsequent calls in the same process, masking changes in workspaceRoot. Drops the module-level cache and threads workspaceRoot through to detection. The package-json create-nodes spec now seeds a package-lock.json into memfs (matching #35116's pattern for plugin specs) so the detector picks `npm` deterministically. Follow-up to #35116.
compute packageManagerCommand once per workspace at the createNodesV2 entry point and thread it through createNodeFromPackageJson and buildProjectConfigurationFromPackageJson into readTargetsFromPackageJson, instead of detecting per package.json read. addresses review feedback on #35550.
FrozenPandaz
force-pushed
the
fix/package-json-pm-detection
branch
from
d369301 to
47a0c4e
Compare
…lities getRunNxBaseCommand and preparePackageInstallation called detectPackageManager() with no argument, defaulting to CWD-based probing. Pass workspaceRoot so the lockfile probe runs in the right directory and avoid env-var fallback. Also dedupe back-to-back getPackageManagerCommand calls in preparePackageInstallation.
Inferred plugins were calling getLockFileName(detectPackageManager(...)) per project inside createNodesInternal. Hoist the detection to the createNodes callback alongside the existing pmc computation, derive both pmc and lockFileName from a single detectPackageManager call, and thread lockFileName through. Affects: angular, cypress, detox, expo, js/typescript, next, nuxt, playwright, react-native, remix, rollup, rsbuild, rspack, storybook, webpack.
![nx-cloud[bot]](https://avatars.githubusercontent.com/in/80458?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
…n entry the legacy nx-all-package-jsons-plugin entry point at packages/nx/plugins/package-json.ts was missed in the earlier hoist; update it to compute the packageManagerCommand once and pass it as the required 5th argument to createNodeFromPackageJson, matching createNodesV2. fixes the TS2554 error in nx:build-base on CI.
… [Self-Healing CI Rerun]
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, we triggered a CI rerun by adding an empty commit to this branch.
🎓 Learn more about Self-Healing CI on nx.dev
2 participants
Current Behavior
readTargetsFromPackageJson(inpackages/nx/src/utils/package-json.ts) receives aworkspaceRootargument but never passes it to package manager detection:Two consequences:
Wrong package manager —
detectPackageManager()defaultsdir = '', so the lockfile probe runs in the CWD, not the workspace. When that finds nothing it falls back tonpm_config_user_agent, so the inferredrunCommand(npm run Xvspnpm run Xvsyarn X) on script targets ends up depending on whoever invoked the nx process rather than on the workspace's actual lockfile.Module-level cache —
let packageManagerCommand(cleared with??=) memoizes the first detection result across all subsequent calls in the process. So even if the first call had the rightworkspaceRoot, every later call inherits that detection regardless of itsworkspaceRoot. This is also whypackages/nx/src/plugins/package-json/create-nodes.spec.tshad four pre-existing snapshot failures locally (pnpm run …instead of the expectednpm run …) — the first test in any process locked detection to the host's PM.This is a follow-up to #35116, which moved package manager detection into the
createNodescallback for the inferred plugins but missed this code path.Expected Behavior
workspaceRootinto bothdetectPackageManagerandgetPackageManagerCommand, so the lockfile probe runs in the right directory.The
packages/nx/src/plugins/package-json/create-nodes.spec.tsfixture now seedspackage-lock.jsoninto memfs in abeforeEach, matching the pattern #35116 established for plugin specs. Without the lockfile the detector still falls back to the env var; with it, every test deterministically picksnpm, matching the existing snapshots.Verification
Before this PR: 4 failures in
packages/nx/src/plugins/package-json/create-nodes.spec.ts:After this PR:
Related Issue(s)
Follow-up to #35116.