Downgrade npmlog + upgrade gauge to address #623

[springmeyer]

This should address #623

• Still keeps at least ansi-regex@5.0.1 thanks to gauge@3.0.2 release
• And therefore can maintain node v10 support

TODO:

• Why were, before this PR, tests not failing on node v10 (if support regressed per #623)?

[wraithgar]

Why were, before this PR, tests not failing on node v10

I can answer this and save you some time: because this was a pre-emptive drop in support. Nothing in the code was changed that would have broke in node10.

The only thing that would catch something like this would be the --engine-strict flag.

[springmeyer]

Ah! Thank you for explaining that. I saw the --engine-strict mention in #623, but did not connect the dots on how it was the sole thing that caught this. Thanks.

[springmeyer]

Hmm, @wraithgar - I just tried, locally, running

npm install --production --engine-strict

against the master branch (without this PRs fix) and it works without errors:

added 54 packages from 21 contributors and audited 394 packages in 1.684s

3 packages are looking for funding
  run `npm fund` for details

found 0 vulnerabilities

$ npm ls gauge                            
@mapbox/node-pre-gyp@1.0.7 /Users/danespringmeyer/projects/node-pre-gyp
└─┬ npmlog@6.0.0
  └── gauge@4.0.0 

Any idea why that is not erroring? I would assume I'd get a non-zero return code from npm install or npm ci with --engine-strict

[springmeyer]

Perhaps I need to add engines to the package.json of node-pre-gyp?

[wraithgar]

It looks like npm@6 is not checking engines when installing from a package-lock. npm@7 always checks.

[bcoe]

:tada: thanks for taking on this work. I know I'm like the only person who runs tests with --engine-strict :laughing:, and that I can be a squeaky wheel.

Edit: my experience has been, this is good to treat as a breaking change, or you eventually get burned by someone upstream using a new feature, e.g., ESM in the case of having a dep move to Node 12.

[springmeyer]

@bcoe - no problem, I'd like to be like you. I'm still struggling however to replicate any kind of error or failure with node v10 (or 8 for that matter) and the master code before this fix. Even with npm@7. Would you be up for providing a PR to this repo's .travis.yml that demonstrates how to detect with --engine-strict the problem you were able to catch with CI? So, a PR that fails and then would pass once this PR is merged in.

[bcoe]

@springmeyer this is the approach I've been taking that catches things:

https://github.com/googleapis/nodejs-logging/blob/main/.github/workflows/ci.yaml#L22

[bcoe]

@springmeyer I did some local testing too, and a couple things I noticed:

• you need to be running on the version of Node.js that's minmally supported (the engines strict error only throws for me if I'm running npm i on Node 10).
• I was initially having problems with the package lock on npm@6 (so, as @wraithgar suggests, npm i npm@7 -g might be smart).

Once I did both these things, I'm getting the following with a local check out of node-pre-gyp:

node-pre-gyp$ npm i -production --engine-strict
npm ERR! code ENOTSUP
npm ERR! notsup Unsupported engine for npmlog@6.0.0: wanted: {"node":"^12.13.0 || ^14.15.0 || >=16"} (current: {"node":"10.24.1","npm":"6.14.15"})
npm ERR! notsup Not compatible with your version of node/npm: npmlog@6.0.0
npm ERR! notsup Not compatible with your version of node/npm: npmlog@6.0.0
npm ERR! notsup Required: {"node":"^12.13.0 || ^14.15.0 || >=16"}
npm ERR! notsup Actual:   {"npm":"6.14.15","node":"10.24.1"}

[springmeyer]

Got it, thank you @bcoe - npm@7 + using node v10 allowed me to replicate (and it caught another incompatible dependency - action-walk).

[springmeyer]

This is now published as @mapbox/node-pre-gyp@1.0.8

[minherz]

Thanks for this work 👏 it allows us to avoid dropping Node 10 on @google-cloud/profiler