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

fix(core): ensurePackage fails on pnpm workspaces #16002

Merged
merged 7 commits into from
Apr 6, 2023

Conversation

meeroslav
Copy link
Contributor

@meeroslav meeroslav commented Mar 31, 2023

This PR:

  • Fixes broken ensurePackage on pnpm workspaces
  • Adds verbose to ensurePackage
  • Extends getPackageManagerCommand to accept root as a second parameter
  • Adds missing -w in the e2e utils
  • Fixes e2e-create-nx-workspace-npm to use selected package manager
  • Optimizes e2e-create-nx-workspace-npm to re-use the same workspace

Current Behavior

The ensurePackage fails on pnpm workspace

Expected Behavior

The ensurePackage passes on pnpm workspace and installs the package as expected.

Related Issue(s)

Fixes #

@vercel
Copy link

vercel bot commented Mar 31, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
nx-dev ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 4, 2023 7:59pm

@meeroslav meeroslav force-pushed the optimize-workspace-create-tests branch 2 times, most recently from db46661 to 7464280 Compare March 31, 2023 13:48
@@ -158,8 +160,8 @@ export function getPackageManagerCommand({
runUninstalledPackage: 'pnpm dlx',
install: 'pnpm i',
ciInstall: 'pnpm install --frozen-lockfile',
addProd: `pnpm add`,
addDev: `pnpm add -D`,
addProd: isPnpmWorkspace ? 'pnpm add -w' : 'pnpm add',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was strangely broken the whole time

});
});

afterEach(() => cleanupProject());
beforeEach(() => {
runCommand(`git reset --hard`);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We use git reset to revert all the changes done on the repo

@meeroslav meeroslav force-pushed the optimize-workspace-create-tests branch from 7464280 to c415852 Compare April 3, 2023 08:57
@meeroslav meeroslav added the scope: core core nx functionality label Apr 3, 2023
@meeroslav meeroslav force-pushed the optimize-workspace-create-tests branch 2 times, most recently from aed5f8e to 9a0db05 Compare April 3, 2023 09:26
@meeroslav meeroslav force-pushed the optimize-workspace-create-tests branch from 9a0db05 to 1aef7b9 Compare April 3, 2023 09:33
},
moduleFileExtensions: ['ts', 'tsx', 'js', 'jsx', 'html'],
maxWorkers: 1,
globals: { 'ts-jest': { tsconfig: '<rootDir>/tsconfig.spec.json' } },
globals: {},
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Usage of ts-jest in globals is deprecated

Copy link
Contributor

Choose a reason for hiding this comment

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

strange that didn't get migrated 🤔

@meeroslav meeroslav force-pushed the optimize-workspace-create-tests branch 5 times, most recently from fb0a11b to 90fd0c4 Compare April 3, 2023 11:17
@meeroslav
Copy link
Contributor Author

Converting back to draft until create-nx-workspace-npm issues are resolved

const isVerbose = process.env.NX_VERBOSE_LOGGING === 'true';
execSync(
`${
getPackageManagerCommand(detectPackageManager(), tempDir).addDev
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the case of pnpm workspace, it would try to run the pnpm add -Dw ... command, which fails since tempDir is not a workspace.

@meeroslav meeroslav changed the title fix(core): optimize e2e create-nx-workspace tests fix(core): ensurePackage fails on pnpm workspaces Apr 3, 2023
@meeroslav meeroslav force-pushed the optimize-workspace-create-tests branch 2 times, most recently from 3c56cc3 to 48b08a1 Compare April 3, 2023 17:37
afterEach(() => cleanupProject());
afterEach(() => {
runCommand(`git reset --hard HEAD`);
runCommand(`git clean -fdx`);
Copy link
Collaborator

Choose a reason for hiding this comment

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

You don't need x here. x will clean out even gitignore'd files like node_modules which will slow down the install.

@FrozenPandaz FrozenPandaz merged commit b3a3f2a into nrwl:master Apr 6, 2023
FrozenPandaz pushed a commit that referenced this pull request Apr 11, 2023
@github-actions
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 12, 2023
@meeroslav meeroslav deleted the optimize-workspace-create-tests branch April 27, 2023 09:55
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
scope: core core nx functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants