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

Sort devDependencies alphabetically #4692

Closed
wants to merge 1 commit into from

Conversation

outsideris
Copy link
Contributor

In devDependencies, some modules isn't sorted alphabetically.

So, package.json will be updated whenever running npm install. It can make conflicts with others.

Signed-off-by: Outsider <outsideris@gmail.com>
@outsideris outsideris added the dependencies Pull requests that update a dependency file label Jul 19, 2021
@coveralls
Copy link

Coverage Status

Coverage remained the same at 94.348% when pulling 6c3518d on outsideris:resort-dependencies into 3722201 on mochajs:master.

@juergba
Copy link
Contributor

juergba commented Jul 20, 2021

@outsideris These five devDependencies build a group:

    "@babel/preset-env": "7.12.17",
    "@babel/plugin-transform-regenerator": "7.12.1",
    "regenerator-transform": "0.14.5",
    "@babel/runtime": "7.12.5",
    "regenerator-runtime": "0.13.7",

I had to pin their versions in order to fix an EvalError when using Mocha in browser environment, see #4643.

So, package.json will be updated whenever running npm install. It can make conflicts with others.

I don't understand this statement. package.json isn't updated with npm install neither in windows nor Ubuntu.

So if this contribution just sorts devDependencies without any deeper reason, then IMO it does more harm than anything else.
Do you have any alternative proposition to solve this async/await/transpiling/EvalError problem?

@outsideris
Copy link
Contributor Author

I didn't know about #4643 .

In macOs, package.json updated.

$ git clone git@github.com:mochajs/mocha.git

$ cd mocha

$ npm i

added 2479 packages, and audited 2480 packages in 15s

24 vulnerabilities (4 low, 7 moderate, 13 high)

To address issues that do not require attention, run:
  npm audit fix

To address all issues (including breaking changes), run:
  npm audit fix --force

Run `npm audit` for details.

$ git diff package.json
diff --git a/package.json b/package.json
index a645d2ad..117caed4 100644
--- a/package.json
+++ b/package.json
@@ -81,11 +81,9 @@
   "devDependencies": {
     "@11ty/eleventy": "^0.11.0",
     "@11ty/eleventy-plugin-inclusive-language": "^1.0.0",
-    "@babel/preset-env": "7.12.17",
     "@babel/plugin-transform-regenerator": "7.12.1",
-    "regenerator-transform": "0.14.5",
+    "@babel/preset-env": "7.12.17",
     "@babel/runtime": "7.12.5",
-    "regenerator-runtime": "0.13.7",
     "@mocha/docdash": "^3.0.1",
     "@rollup/plugin-babel": "^5.1.0",
     "@rollup/plugin-commonjs": "^14.0.0",
@@ -137,6 +135,8 @@
     "nyc": "^15.1.0",
     "pidtree": "^0.5.0",
     "prettier": "^1.19.1",
+    "regenerator-runtime": "0.13.7",
+    "regenerator-transform": "0.14.5",
     "remark": "^12.0.1",
     "remark-github": "^9.0.1",
     "remark-inline-links": "^4.0.0"

I didn't know package.json updated alphabetically only on macOS.

I will check it why it happened.

@outsideris
Copy link
Contributor Author

And async/await/transpiling/EvalError problem is related dependency order in package.json, not only versions?

@juergba
Copy link
Contributor

juergba commented Jul 24, 2021

And async/await/transpiling/EvalError problem is related dependency order in package.json, not only versions?

I don't know. Btw I'm using npm v6.14.13.

@juergba
Copy link
Contributor

juergba commented Jul 29, 2021

@outsideris a new version v0.13.9 of regenerator-runtime has been published a few days ago.

This could solve our async/await/transpiling/EvalError problem, I'm not sure though.
If yes, we could remove those version-pinned devDependencies again from our package.json.

I'm going to test ...

@outsideris
Copy link
Contributor Author

This could solve our async/await/transpiling/EvalError problem, I'm not sure though.

@juergba Good news. I'm adding tests to prevent async/await/transpiling/EvalError problem.
I'm not sure that I understand the problem exactly, I will submit a PR with the tests as draft. And I will test regenerator-runtime as well.

@outsideris outsideris added the invalid not something we need to work on, such as a non-reproducing issue or an external root cause label Aug 4, 2021
@outsideris outsideris closed this Aug 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file invalid not something we need to work on, such as a non-reproducing issue or an external root cause
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants