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

remove engines.npm from package.json #3073

Closed
4 tasks done
pabigot opened this issue Oct 18, 2017 · 10 comments
Closed
4 tasks done

remove engines.npm from package.json #3073

pabigot opened this issue Oct 18, 2017 · 10 comments
Labels
type: bug a defect, confirmed by a maintainer

Comments

@pabigot
Copy link

pabigot commented Oct 18, 2017

Prerequisites

  • Checked that your issue isn't already filed by cross referencing issues with the common mistake label
  • Checked next-gen ES issues and syntax problems by using the same environment and/or transpiler configuration without Mocha to ensure it isn't just a feature that actually isn't supported in the environment in question or a bug in your code.
  • 'Smoke tested' the code to be tested by running it outside the real test suite to get a better sense of whether the problem is in the code under test, your usage of Mocha, or Mocha itself
  • Ensured that there is no discrepancy between the locally and globally installed versions of Mocha. You can find them with:
    node node_modules/.bin/mocha --version(Local) and mocha --version(Global). We recommend avoiding the use of globally installed Mocha.

Description

Node 4.0 introduced an engine dependency on npm >= 2.15.11. The blog post says this is because that version shipped with Node.js v4.0.0.

This is incorrect. Nodejs v4.0.0 shipped with npm 2.14.2. Nodejs v4.5.0, which I have to use, shipped with 2.15.9.

It is true that the current Nodejs 4 (v4.8.4 at time of writing) does use 2.15.11, but that update wasn't done until v4.6.2.

Please consider reducing the dependency to 2.14.2, if your intent is to support all Nodejs 4 releases.

Steps to Reproduce

Expected behavior: Installation of mocha 4.0.1 under nodejs 4.5.0 emits no diagnostics.

Actual behavior:

npm WARN engine [email protected]: wanted: {"node":">= 4.0.0","npm":">= 2.15.11"} (current: {"node":"4.5.0","npm":"2.15.9"})

Reproduces how often: Every time

Versions

Additional Information

@ScottFreeCode ScottFreeCode added the status: needs review a maintainer should (re-)review this pull request label Nov 20, 2017
@boneskull boneskull added type: chore generally involving deps, tooling, configuration, etc. good first issue new contributors should look here! status: accepting prs Mocha can use your help with this one! semver-patch implementation requires increase of "patch" version number; "bug fixes" status: waiting for author waiting on response from OP - more information needed and removed status: needs review a maintainer should (re-)review this pull request type: chore generally involving deps, tooling, configuration, etc. good first issue new contributors should look here! status: accepting prs Mocha can use your help with this one! semver-patch implementation requires increase of "patch" version number; "bug fixes" labels Dec 9, 2017
@boneskull
Copy link
Contributor

@pabigot Are you using AWS Lambda?

@boneskull
Copy link
Contributor

boneskull commented Dec 13, 2017

@pabigot Also, is there anything other than a warning? In other words, is this the only "symptom"?

@boneskull
Copy link
Contributor

See #3149 as well

@boneskull
Copy link
Contributor

I'm not entirely sure what the intent should be. Only the latest v4.x is maintained, so perhaps the engines field should be updated instead.

@boneskull
Copy link
Contributor

or we could add 4.0.0 to the build matrix...

@pabigot
Copy link
Author

pabigot commented Dec 13, 2017

No, I'm not using lambda. It is a warning, but that warning suggests the claim of supporting >=4 (rather than >=4.6.2) is not justified: if I saw this in a production dependency I'd be much more concerned that it reflected a real failure to support. Until action occurs on #3149 my preferred path is that the npm engine dependency be reduced to match the documented behavior and 4.0.0 be added to the build matrix.

I don't think it makes sense only support the latest release in any LTS series. You'd spend a lot of time updating dependencies and annoying users who don't want to update their infrastructure every couple weeks.

@boneskull
Copy link
Contributor

From what I understand, npm's behavior when the engines field is violated is a warning, but Yarn will actually error and refuse to install.

Given that a version of npm which fails to install Mocha is by definition incompatible, one can find that out rather directly by attempting to install Mocha with said incompatible version. 😄

I am proposing removal of the engines.npm field, since we can't be certain what it does or what value it provides in any given situation.

@boneskull boneskull changed the title mocha 4 npm engine dependency too strict remove engines.npm from package.json Dec 13, 2017
@boneskull boneskull added type: bug a defect, confirmed by a maintainer and removed status: waiting for author waiting on response from OP - more information needed labels Dec 13, 2017
@boneskull
Copy link
Contributor

@pabigot #3154

@pabigot
Copy link
Author

pabigot commented Dec 13, 2017

@boneskull I don't see any issues with the solution in #3154.

@boneskull
Copy link
Contributor

@pabigot #3154 is the fix to remove the field and thus the warning.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug a defect, confirmed by a maintainer
Projects
None yet
Development

No branches or pull requests

3 participants