Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use "--omit=dev" internally on newer npm version #86

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ericcornelissen
Copy link

Closes #81

Update the behaviour when the --production flag is used on the CLI of this project to, instead of just using --production when invoking npm audit, do:

  1. get the version of npm being used,
  2. check the version against a version range12
  3. use '--production' for older npm version, or
  4. use '--omit=dev' for newer npm version.

Note that this change is not a breaking change for this project.

While I updated the tests, the tests as they are now are not ood. The current test suite assumes you're running them with a newer version of npm. It would be better if the tests are somehow run with a controlled version of npm instead, or better yet, with various versions of npm.

Footnotes

  1. The version range is probably incorrect as is. I based it on the GitHub issue with the idea to either let it evolve over time as reports come in, or adjust it after this commit if it's preferred that the range is (likely) correct before this is released.

  2. To perform the version range check I added a new runtime dependency, namely "semver". See: https://www.npmjs.com/package/semver

Update the behaviour when the `--production` flag is used on the CLI of
this project to, instead of just using '--production', do:
1. get the version of npm being used,
2. check the version against a version range,
3. use '--production' for older npm version, or
4. use '--omit=dev' for newer npm version.

Two notes regarding point 2:
1. The version range is probably incorrect as is. I based it on the
   GitHub issue with the idea to either let it evolve over time as
   reports come in, or adjust it after this commit if it's preferred
   that the range is (likely) correct before this is released.
2. To perform the version range check I added a new runtime dependency,
   namely "semver". See: https://www.npmjs.com/package/semver

Lastly, while I updated the tests, the tests as they are now are not
ood. The current test suite assumes you're running them with a newer
version of npm. It would be better if the tests are somehow run with a
controlled version of npm instead, or better yet, with various versions
of npm.
@ericcornelissen
Copy link
Author

@jeemok, I'm not sure what to make of the failing Node.js CI / build jobs - running the npm run audit command locally I'm getting the same error on the master branch (at c7cdc15) as I'm seeing in the CI job logs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Recommended "--omit=dev" option not found for NPM 8
1 participant