Skip to content

Conversation

nlf
Copy link
Contributor

@nlf nlf commented Jan 6, 2021

currently we're appending everything passed as an argument to npx/npm exec together into one single large string. this is rarely the correct thing to do. rather than re-adding the argument escaping here, i decided to fix it correctly and pass any additional arguments past the name of the command/script to run as an array literal to @npmcli/run-script which can handle the escaping appropriately and consistently there.

References

fixes #2410
fixes #2423
fixes #2425

@nlf nlf requested a review from a team as a code owner January 6, 2021 19:51
@npm-deploy-user
Copy link

npm-deploy-user commented Jan 6, 2021

angular-quickstart app-large app-medium ember-quickstart react-app
prev current status prev current status prev current status prev current status prev current status
initial install 41s 54.1s🛑 39.5s 58.5s🛑 34.5s 41.5s🛑 28.1s 36.4s🛑 33.5s 48.5s🛑
repeat install 9.6s 4.5s 8.6s 4.1s 8.3s 3.3s 7.7s 2.1s 9.1s 3.4s
with warm cache 32.3s 30.1s 33.8s 36.4s✅🐌 31.3s 26.7s 23.4s 21.6s 29.1s 27s
with node_modules 9.2s 7.4s 8.6s 6s 9s 4.7s 7.8s 2.7s 9.4s 7.8s
with lockfile 32.6s 39.6s🛑 31s 40s🛑 29s 28.5s 21.7s 23.9s✅🐌 27.4s 35.1s🛑
with warm cache and node_modules 9.3s 4.6s 7.9s 4.7s 8.4s 4.1s 7.6s 2.6s 9.2s 3.9s
with warm cache and lockfile 24.7s 14.6s 26.3s 18.4s 24.3s 11.5s 17.8s 8.7s 21.3s 11.1s
with node_modules and lockfile 10.1s 7s 9.8s 5.4s 8.6s 5s 7.8s 2.2s 9.7s 7.2s

@darcyclarke darcyclarke added Release 7.x work is associated with a specific npm 7 release release: next These items should be addressed in the next release semver:patch semver patch level for changes labels Jan 7, 2021
@isaacs isaacs changed the base branch from latest to release/v7.4.0 January 7, 2021 20:43
@isaacs isaacs force-pushed the nlf/fix-script-args branch from 2769fcc to 99156df Compare January 7, 2021 20:44
@isaacs isaacs merged commit 99156df into release/v7.4.0 Jan 7, 2021
@nlf nlf deleted the nlf/fix-script-args branch March 28, 2022 16:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release: next These items should be addressed in the next release Release 7.x work is associated with a specific npm 7 release semver:patch semver patch level for changes

Projects

None yet

5 participants