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

Update dependencies & dev dependencies #4396

Merged
merged 2 commits into from
Aug 18, 2020
Merged

Update dependencies & dev dependencies #4396

merged 2 commits into from
Aug 18, 2020

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
Contributor

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

@boneskull
Copy link
Contributor

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
Contributor

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
Contributor

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
Contributor

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
Contributor

okay, that seemed to work.

@boneskull
Copy link
Contributor

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