cli icon indicating copy to clipboard operation
cli copied to clipboard

fix(ci): rm workspace node_modules

Open reggi opened this issue 1 year ago • 1 comments

Fixes https://github.com/npm/cli/issues/7226, an issue where the npm ci needs to remove node_modules within workspace directories.

reggi avatar May 08 '24 17:05 reggi

no statistically significant performance changes detected

timing results
app-large clean lock-only cache-only modules-only no-lock no-cache no-modules no-clean show-version run-script cache-only
peer-deps
no-clean
audit
npm@latest 31.277 ±0.66 10.773 ±0.03 11.725 ±0.07 1.533 ±0.02 1.543 ±0.00 1.271 ±0.01 8.303 ±0.01 1.261 ±0.01 0.137 ±0.00 0.169 ±0.01 14.928 ±0.17 2.096 ±0.00
#7490 33.998 ±2.64 10.665 ±0.03 12.338 ±0.37 1.604 ±0.05 1.549 ±0.01 1.294 ±0.01 8.403 ±0.01 1.276 ±0.01 0.137 ±0.00 0.164 ±0.00 14.636 ±0.20 2.116 ±0.04
app-medium clean lock-only cache-only modules-only no-lock no-cache no-modules no-clean show-version run-script cache-only
peer-deps
no-clean
audit
npm@latest 24.931 ±0.50 8.021 ±0.01 8.891 ±0.04 1.493 ±0.01 1.490 ±0.01 1.395 ±0.01 5.771 ±0.05 1.295 ±0.01 0.137 ±0.00 0.164 ±0.00 9.952 ±0.02 1.993 ±0.08
#7490 25.700 ±2.44 8.172 ±0.13 8.972 ±0.06 1.524 ±0.00 1.540 ±0.03 1.403 ±0.01 5.843 ±0.02 1.291 ±0.01 0.138 ±0.00 0.164 ±0.00 9.932 ±0.05 2.053 ±0.10

npm-cli-bot avatar May 13 '24 21:05 npm-cli-bot

This seems now inconsistent behavior https://github.com/npm/cli/issues/3598 Why would ci prune workspaces when it doesn't install them either? It only prunes what it installs back - that is packages not linking. This broke our system as can't first install the workspace to cache it on docker layer and then install the parent after which we fail to run a command on the workspace since its modules are gone...

Revert please? It's unfortunate any of these commands don't really work with workspaces but this behavior shouldn't be changed IMO.

This should use the --workspaces flag?

workspaces Default: null Type: null or Boolean Set to true to run the command in the context of all configured workspaces.

Explicitly setting this to false will cause commands like install to ignore workspaces altogether. When not set explicitly:

Commands that operate on the node_modules tree (install, update, etc.) will link workspaces into the node_modules folder. - Commands that do other things (test, exec, publish, etc.) will operate on the root project, unless one or more workspaces are specified in the workspace config.

Explicitly setting this to false will cause commands like install to ignore workspaces altogether.

At least this doesn't seem to affect this at all? --workspaces=false

@reggi @wraithgar

Seems postinstall is run on ci but it can be excluded with --workspaces for the nested workspace.

You can test my issue of having let say copyfiles package on postinstall for a workspace module.

And you should get following when running npm ci for the parent module:

npm error sh: 1: copyfiles: not found

while package.json looks something like:

"scripts": {
    "postinstall": "copyfiles . .",
  },
  "dependencies": {
    "copyfiles": "^2.4.1",

this shouldn't happen since the workspace module is installed earlier... But now npm ci breaks it...

leppaott avatar Jul 29 '24 07:07 leppaott

@leppaott please do not @ notify a bunch of people like this, not everyone you just sent a notification to is even on the npm cli team.

I could not fully understand what the problem was from just your comment. If you think there is a bug in how npm ci works please open up a new issue so we can triage it properly.

wraithgar avatar Jul 29 '24 14:07 wraithgar

@wraithgar This should be reverted as it's a breaking change to delete any workspace node_modules. TS compilations need those modules. This should be considered with more thought. It's now doing the clean part yes but it's not doing the install part on clean-install. I was trying to demonstrate that you cannot run a workspace script that calls a binary after the clean-install which you surely can if you just use install here. If you do a clean-install on the workspace it has the binary, after which you do a clean-install on the root it for some reason wipes the workspace's packages.

leppaott avatar Jul 30 '24 06:07 leppaott