volta-cli / volta

Volta: JS Toolchains as Code. ⚡
https://volta.sh
Other
11.09k stars 234 forks source link

npx vs node_modules vs npm run command chaining #1450

Open LegitTrevorTiernan opened 1 year ago

LegitTrevorTiernan commented 1 year ago

Hi, I've been experimenting with Volta for several typescript node repositories. Overall, it has been working very well, but I noticed a couple of oddities I'm slowly working around, and I am hoping someone can help me.

One issue I had was with serverless lambda deployments. With npm version 6, I could do a deploy with npx sls deploy -s <stageName> -r <region> and it worked fine. However, if I did ./node_modules/.bin/serverless, I would get an npm install error. I was thinking it might have something do with using aws code artifact; however, this error went away after upgrading to npm 7.

Another issue I've run into is trying to do 'npm run test:no-lint' definitions: "test:no-lint": "./node_modules/.bin/nyc npm run jasmine", "jasmine": "./node_modules/.bin/ts-node ./node_modules/.bin/jasmine \"src/*/.spec.ts\" --random=false",

nyc is 15.1 (latest) This error also happens on npm 7. I get error

test:no-lint ./node_modules/.bin/nyc npm run jasmine

//.volta/tools/image/npm/7.24.2/bin/npm:2 (set -o igncr) 2>/dev/null && set -o igncr; # cygwin encoding fix ^^^^^


SyntaxError: Unexpected identifier
    at Object.compileFunction (node:vm:352:18)
    at wrapSafe (node:internal/modules/cjs/loader:1033:15)
    at Module._compile (node:internal/modules/cjs/loader:1069:27)
    at Module.replacementCompile (<repo>/node_modules/append-transform/index.js:58:13)
    at Module._extensions..js (node:internal/modules/cjs/loader:1159:10)
    at Object.<anonymous> (<repo>/node_modules/append-transform/index.js:62:4)
    at Module.load (node:internal/modules/cjs/loader:981:32)
    at Function.Module._load (node:internal/modules/cjs/loader:822:12)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:77:12)
    at runMain (<user_directory>/.node-spawn-wrap-90184-b55ebc3e0b1b/node:68:10)

However if I run ./node_modules/.bin/nyc ./node_modules/.bin/ts-node ./node_modules/.bin/jasmine "src/**/*.spec.ts" --random=false directly on the command prompt then works fine.

I'm wondering why these different ways of invoking node commands behave so differently? And if there's a way for me to use Volta more effectively.

marko-hologram commented 1 year ago

Hey, not sure if I can help you much (not tied to Volta devs), but I'm just wondering why are you running these bin files directly instead of letting "npx" handle it?

For example, why is your "test:no-lint" npm script doing this ./node_modules/.bin/nyc npm run jasmine instead of just doing this npx nyc npm run jasmine? "npx" should run locally installed command in the project and you don't need to do any of this custom path nonsense in that case. At least that's how I believe it should work.

My point being, try to drop these custom node_modules paths in your commands and just let "npx" command handle it.

I'm not sure that this issue is related to Volta in any way 😄

LegitTrevorTiernan commented 1 year ago

Some pre-existing scripts are several years old so they may have done things the long way. 🤷‍♂️ I'll give modifying the scripts for npx a shot. Thanks. I'm curious why npx might drift from node_modules even after an npm ci. 🤔

LegitTrevorTiernan commented 1 year ago

Interesting. It produced the same result. 🤔

> merchant-monitoring-api@5.4.0 test:no-lint
> npx nyc npm run jasmine

/Users/trevortiernan/.volta/tools/image/npm/7.23.0/bin/npm:2
(set -o igncr) 2>/dev/null && set -o igncr; # cygwin encoding fix
        ^^^^^
marko-hologram commented 1 year ago

Interesting. It produced the same result. 🤔

> merchant-monitoring-api@5.4.0 test:no-lint
> npx nyc npm run jasmine

/Users/trevortiernan/.volta/tools/image/npm/7.23.0/bin/npm:2
(set -o igncr) 2>/dev/null && set -o igncr; # cygwin encoding fix
        ^^^^^

That's interesting, I thought npx would work for sure 😄 What if you just change your script to this?

"scripts": {
  "test:no-lint": "nyc npm run test"
}

So basically remove npx.

I see this is how its done in nyc docs https://github.com/istanbuljs/nyc#installation--usage

LegitTrevorTiernan commented 1 year ago

That fails as well. Although if I do an upgrade of nyc to latest (15.1) and "test:volta": "nyc npm run jasmine:volta", "jasmine:volta": "npx ts-node ./node_modules/.bin/jasmine --config=jasmine.json", This works great for me. I've been facing some combination of versions, volta, and script syntax/usage, resulting in errors that nvm does not. It's easy enough to work around, but knowing if there's a formula would be very helpful. 🤔