Skip to content

Conversation

@GChuf
Copy link
Contributor

@GChuf GChuf commented Aug 6, 2020

Requirements

  • Filling out the template is required. Any pull request that does not include enough information to be reviewed in a timely manner may be closed at the maintainers' discretion.
  • All new code requires tests to ensure against regressions.

Description of the Change

Updates dependencies and dev dependencies, fixes some vulnerabilities.

Benefits

300+ less vulnerabilities & less dependencies (node_modules folder decreases in size from 415MB to 394MB and has 2000 less files)

@jsf-clabot
Copy link

jsf-clabot commented Aug 6, 2020

CLA assistant check
All committers have signed the CLA.

@coveralls
Copy link

coveralls commented Aug 7, 2020

Coverage Status

Coverage increased (+0.06%) to 93.849% when pulling ad84c62 on GChuf:dep2 into 24d1dfb on mochajs:master.

@boneskull
Copy link
Member

going to need to run a branch check on this one, lemme see

@boneskull
Copy link
Member

build is here: https://travis-ci.org/github/mochajs/mocha/builds/718013127, branch boneskull/pr/GChuf-dep2

@GChuf
Copy link
Contributor Author

GChuf commented Aug 14, 2020

Travis looks okay, what's up with appveyor not finding package.lock file? re-run tests?

@boneskull
Copy link
Member

appveyor's kind of hinky. looking to move to GH actions for that stuff, see #4402. should be fine to merge this otherwise

@boneskull
Copy link
Member

I'm seeing this during Rollup build:

(!) Error when using sourcemap for reporting an error: Can't resolve original location of error.
node_modules\escape-string-regexp\index.js: (7:12)
(!) `this` has been rewritten to `undefined`
https://rollupjs.org/guide/en/#error-this-is-undefined
node_modules\escape-string-regexp\index.js
5: import { newArrowCheck as _newArrowCheck } from "\0rollupPluginBabelHelpers.js";
6: 
7: var _this = this;
               ^
8: 
9: var escapeStringRegexp = function escapeStringRegexp(string) {
...and 1 other occurrence
created ./mocha.js in 10.1s

which is concerning, especially since no tests fail.

When you ran these upgrades, did you use npm audit fix --force or just upgrade everything? Did escape-string-regexp need upgrading?

@GChuf
Copy link
Contributor Author

GChuf commented Aug 16, 2020

I just upgraded escape-string-regexp to @latest. I didn't do it out of any need, so you can try downgrading to v3 or v2, see what happens. Worst case scenario it goes back to v1.0.5, I don't think it makes much difference anyway ...
I ran npm audit fix without --force, so that shouldn't be a problem.

@boneskull
Copy link
Member

I think that error might be due to the spec: true in the rollup config. I'll see if removing it addresses the problem

@boneskull boneskull added type: chore generally involving deps, tooling, configuration, etc. semver-patch implementation requires increase of "patch" version number; "bug fixes" labels Aug 18, 2020
@boneskull
Copy link
Member

okay, that seemed to work.

GChuf and others added 2 commits August 18, 2020 11:51
@boneskull
Copy link
Member

thanks!

@boneskull boneskull merged commit 2137163 into mochajs:master Aug 18, 2020
@boneskull boneskull added this to the next milestone Aug 18, 2020
@boneskull boneskull added the area: security involving vulnerabilities label Aug 18, 2020
irrationnelle pushed a commit to irrationnelle/mocha that referenced this pull request Aug 23, 2020
* Update dependencies & dev dependencies

* fix build, remove spec:true from babel config

update lint-staged config

Signed-off-by: Christopher Hiller <[email protected]>

Co-authored-by: Christopher Hiller <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: security involving vulnerabilities semver-patch implementation requires increase of "patch" version number; "bug fixes" type: chore generally involving deps, tooling, configuration, etc.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants