search

testdouble / scripty

Because no one should be shell-scripting inside a JSON file.
MIT License
963 stars 23 forks source link

Support npm >= 7 through RFC'd config section #105

Closed danielgindi closed 2 years ago

danielgindi commented 2 years ago

This supports both the old way in npm 6, and npm >= 7 through the new RFC'd config section.

searls commented 2 years ago

Thanks!

rosston commented 2 years ago

I've confirmed this works for me in the example project with both npm v6 and v8.

Here's what I did:

  1. Checked out the code in this PR
  2. Ran npm i in the project root
  3. Use node 14, and the default npm 6
  4. In the example project, add "scripty": {"dryRun": true} to the package.json
  5. Confirm that npm t does a dry run
  6. Change the example/package.json to use "config": {"scripty": {"dryRun": true}} instead
  7. Confirm that npm t still does a dry run
  8. Use node 16, and the default npm 8
  9. Confirm that npm t still does a dry run

One thing I noticed however is that npm t in the root project is failing on this PR, regardless of npm version. I haven't dug into that at all yet. We should probably get this project back on CI (travis-ci.org doesn't build anymore), and then that test failure would be more obvious. 🙂

rosston commented 2 years ago

And just for my reference later: I'm pretty sure moving this PR forward/merging it will resolve https://github.com/testdouble/scripty/issues/93 and https://github.com/testdouble/scripty/issues/103.

jasonkarns commented 2 years ago

Do we need to add a test case to cover an npm_package_config_scripty_X env var still plays well with scripty's own env var parsing?

Does the npm lifecycle env var still exist? That's the only other one outside of config that I think we rely on.

Should we also document that npm_package_config_scripty_* work or no?

danielgindi commented 2 years ago

Does the npm lifecycle env var still exist? That's the only other one outside of config that I think we rely on.

Yes. It's in the RFC, and validated out there in real life 😉

Should we also document that npm_package_config_scripty_* work or no?

Should be obvious the amount as it was before without config... The full env behavior is in the rfc, which is the major breaking change in npm 7.

danielgindi commented 2 years ago

Are we merging this? (Does not even require a major version release)

searls commented 2 years ago

Thanks! Merged and released as 2.1.0