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

mocha devDependency adds content to node_modules after npm install --production #4474

Closed
danludwig opened this issue Oct 12, 2020 · 8 comments
Closed

Comments

@danludwig
Copy link

@danludwig danludwig commented Oct 12, 2020

Prerequisites

  • Checked that your issue hasn't already been filed by cross-referencing issues with the faq 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 that you not install Mocha globally.

Description

Sometime between Wednesday September 30th 2020 and Thursday October 1st 2020, the behavior of mocha during a clean npm install --production changed. Previously, the command resulted in no node_modules folder being created. Since, the command has consitently resulted in the creation of a node_modules folder with the following contents:

  • define-properties
  • es-abstract
  • es-to-primitive
  • function-bind
  • has
  • has-symbols
  • is-callable
  • is-date-object
  • is-regex
  • is-symbol
  • object-inspect
  • object-keys
  • string.prototype.trimend
  • string.prototype.trimstart

Steps to Reproduce

  1. git clone https://github.com/danludwig/npm-module-bug-reports.git
  2. cd npm-module-bug-reports/mocha-2020-10-bug
  3. Observe that the ./package.json has only a devDependency on mocha
  4. npm install --production
  5. Observe that a node_modules folder was created with the above listed contents.

Expected behavior: [What you expect to happen]

Assuming the commands to reproduce above were followed, continue with:

  1. cd ../mocha-2020-10-expected
  2. Observe that the ./package.json has only a devDependency on chai
  3. npm install --production
  4. Observe that no node_modules folder was created.

Actual behavior: [What actually happens]

Reproduces how often: [What percentage of the time does it reproduce?]

100% since October 1st 2020. 0% on and before September 30, 2020

Versions

  • The output of mocha --version and node node_modules/.bin/mocha --version: 8.1.3
  • The output of node --version: v12.13.0
  • Your operating system
    • name and version: Windows 10, MacOS Catalina 10.15.6, Ubuntu 18.04
    • architecture (32 or 64-bit): 64-bit
  • Your shell (e.g., bash, zsh, PowerShell, cmd): bash, gitbash, powershell
  • Your browser and version (if running browser tests): n/a
  • Any third-party Mocha-related modules (and their versions): n/a
  • Any code transpiler (e.g., TypeScript, CoffeeScript, Babel) being used (and its version): n/a

Additional Information

This is an important issue as AWS Lambda@Edge code packages have a size limitation of 1048576 bytes, and the automated build tooling provided by AWS SAM will include these node_modules contents, violating the service limit. The only workaround is to add a cleanup step in the build process to delete the node_modules folder after building but before publishing the package for use in Lambda@Edge.

@boneskull
Copy link
Member

@boneskull boneskull commented Oct 12, 2020

in your test repo, your expected dir has chai in package.json and the bug dir has mocha in package.json

I don't understand this

@danludwig
Copy link
Author

@danludwig danludwig commented Oct 12, 2020

@boneskull the purpose of that is simply to demonstrate that a dev dependency should not contribute anything to the contents of a node_modules folder when npm install --production is used. I was unable to demonstrate that with the mocha dev dependency. I could have picked something else besides mocha and chai. But if one project contained both mocha and chai as dev dependencies, it would be harder to demonstrate that the node_modules folder contents came from mocha and not other dev dependency/ies.

@boneskull
Copy link
Member

@boneskull boneskull commented Oct 12, 2020

OK. Well, I can certainly reproduce it. but what I can't do is tell you why it's happening. this looks like an npm bug to me. try

$ yarn install --production

and find that node_modules is empty.

it seems to have something to do with the lack of dev: true in the generated package-lock.json. but even blasting package-lock.json and retrying w/o a lockfile has the same result.

@boneskull
Copy link
Member

@boneskull boneskull commented Oct 12, 2020

@boneskull
Copy link
Member

@boneskull boneskull commented Oct 12, 2020

I'm going to close this as I don't think it's a Mocha bug. I'm not sure what else to do... I wonder if downgrading npm would help.

@danludwig
Copy link
Author

@danludwig danludwig commented Oct 13, 2020

@boneskull we've already seen the bug 1113 report at the npm issued board. That bug was reported way back in April, and doesn't explain why we started seeing this behavior in mocha starting on October 1st. I have build artifacts showing that the behavior did not manifest at all up to and including September 30th. Could it be that one of the mocha dependencies, such as eslint, received a version bump where a shrinkwrap was introduced? If that's the case, could this be an issue that could be corrected in mocha by falling back to a lower dependency version?

@boneskull boneskull reopened this Oct 13, 2020
@boneskull
Copy link
Member

@boneskull boneskull commented Oct 13, 2020

It's an npm bug for sure, but we can work around it. Mocha requires promise.allsettled in its production deps, which depends on es-abstract. es-abstract seems to be the "problem" repo. If we replace promise.allsettled with a different ponyfill, it avoids installing es-abstract and addresses the issue.

@boneskull boneskull added confirmed-bug and removed invalid labels Oct 13, 2020
boneskull added a commit that referenced this issue Oct 13, 2020
@danludwig
Copy link
Author

@danludwig danludwig commented Oct 13, 2020

This is great news, thank you. Will keep an eye on #4476

@boneskull boneskull closed this in 60e3662 Oct 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.