-
Notifications
You must be signed in to change notification settings - Fork 2.6k
feat(core): add packageManager field when setting up new workspaces #33702
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
base: master
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
✅ 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 9826694
☁️ Nx Cloud last updated this comment at |
JamesHenry
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@juristr I think we'll want a custom snapshot serializer for these otherwise they will be subject to change every time our CI environment changes by even a patch version of npm
|
@JamesHenry oh yeah good catch |
4f41b09 to
9826694
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nx Cloud is proposing a fix for your failed CI:
These changes fix the e2e test failures by ensuring the generated test specs use npm commands that match the package manager configured in the created workspaces. Since the create-package template hardcodes packageManager: 'npm' in the generated workspace, the e2e tests now correctly use npm ls and npx instead of dynamically resolved commands from the generating workspace (which uses pnpm).
We could not verify this fix.
diff --git a/packages/plugin/src/generators/create-package/files/e2e/__cliName__.spec.ts__tmpl__ b/packages/plugin/src/generators/create-package/files/e2e/__cliName__.spec.ts__tmpl__
index 11c89d9f74..192e057641 100644
--- a/packages/plugin/src/generators/create-package/files/e2e/__cliName__.spec.ts__tmpl__
+++ b/packages/plugin/src/generators/create-package/files/e2e/__cliName__.spec.ts__tmpl__
@@ -20,7 +20,7 @@ describe('<%= cliName %>', () => {
projectDirectory = createTestProject();
// npm ls will fail if the package is not installed properly
- execSync('<%= packageManagerCommands.list %> <%= pluginPackageName %>', {
+ execSync('npm ls --depth 100 <%= pluginPackageName %>', {
cwd: projectDirectory,
stdio: 'inherit',
});
@@ -45,7 +45,7 @@ function createTestProject(extraArgs = '') {
});
execSync(
- `<%= packageManagerCommands.dlx %> <%= cliName %>@e2e ${projectName} ${extraArgs}`,
+ `npx <%= cliName %>@e2e ${projectName} ${extraArgs}`,
{
cwd: dirname(projectDirectory),
stdio: 'inherit',
Or Apply changes locally with:
npx nx-cloud apply-locally Mu6j-N9dD
Apply fix locally with your editor ↗ View interactive diff ↗
🎓 Learn more about Self-Healing CI on nx.dev
Current Behavior
Currently we don't add the
packageManagerproperty when generating a new workspace.Expected Behavior
We should have it as more and more tooling relies on it to make decisions. (e.g. on CI etc..)
Related Issue(s)
Fixes #