Closed Bug 1677562 Opened 4 years ago Closed 2 years ago

Categories

(Developer Infrastructure :: Lint and Formatting, defect, P3)

Tracking

(firefox114 fixed)

RESOLVED FIXED
Tracking Status
firefox114 --- fixed

People

(Reporter: vringar, Assigned: standard8)

References

Details

Attachments

(2 files)

Attached file eresolve-report.txt

As part of OpenWPM we use eslint-plugin-mozilla as an external consumer.
However we are currently unable to npm install as there seems to be an dependency conflict inside the package.

I generated a minimal package.json to reproduce the issue.

{
  "devDependencies": {
    "eslint-plugin-mozilla": "^2.0.0"
  }
}

Running npm install in a directory that contains this package.json should produce the same eresolve-report.txt

Flags: needinfo?(standard8)

I've just had a look around, and this appears to be caused by npm version 7 handling peerDependencies differently.

If you run npm install --legacy-peer-deps then it installs fine - obviously using the old system.

As to the actual error:

npm ERR! While resolving: test@2.9.1
npm ERR! Found: prettier@2.1.2
npm ERR! node_modules/prettier
npm ERR!   peer prettier@">=1.13.0" from eslint-plugin-prettier@3.1.4
npm ERR!   node_modules/eslint-plugin-prettier
npm ERR!     peer eslint-plugin-prettier@"^3.0.1" from eslint-plugin-mozilla@2.9.1
npm ERR!     node_modules/eslint-plugin-mozilla
npm ERR!       eslint-plugin-mozilla@"^2.0.0" from the root project
npm ERR! 
npm ERR! Could not resolve dependency:
npm ERR! peer prettier@"^1.17.0" from eslint-plugin-mozilla@2.9.1

That looks to me like an issue in npm itself - eslint-plugin-mozilla is asking for ^1.17.0, eslint-plugin-prettier is asking for >= 1.13.0.

As those two ranges overlap, I think it should really use one that matches both. Otherwise, anyone using eslint-plugin-prettier is always going to be forced to use the latest prettier version, even though it isn't what is intended.

Unless I've misunderstood these peer dependencies.

We could decide to not specify the prettier version, but then that could be slightly strange since we don't always upgrade to the latest straight away, and I'd rather have what eslint-plugin-mozilla recommends to be similar to that which mozilla-central is actually using at the time.

Alternately, we could potentially update prettier for mozilla-central to 2.x, (although I think this would only be a work around), and that requires a bigger piece of work as there's formatting changes to do across mozilla-central for that.

Flags: needinfo?(standard8)

We stumbled upon the same problem both in Rally and Glean.js.

Mark, would it make sense to split the packge to only have eslint rules in one and the prettier stuff in the other?

Flags: needinfo?(standard8)

I'd prefer not to split the two as that makes maintenance and keeping things in sync a bit harder and potentially other Mozilla projects picking up the ESLint parts but not the formatting (I think both are important). An alternative could be could be to move the prettier inclusion into the top-level .eslintrc.js, but again that looses the formatting parts.

It is unclear if npm is fixing this or not - there's some related issues, but there's some closed issues saying it is a hard problem, and others saying these types of issues are fixed in the latest release (though doesn't appear that way). This also shouldn't block you using it, you should still be able to use the npm install --legacy-peer-deps I believe. I'd also recommend using a lock file and npm ci as that avoids it after initial set-up.

I am planning on updating to prettier 2.x - and I was hoping to maybe synchronise the deps at that time. I started looking at that a couple of weeks ago, but I have to do some checks with other people before we can upgrade to it which I haven't got around to yet.

I guess the other alternative is we just change the peer dependency to match prettier's if that would work.

Flags: needinfo?(standard8)

I've just been taking another look at this, and think there are currently only two choices, neither of which work. One is to match the peer deps, the other is to always upgrade to the latest prettier as soon as prettier is out.

The first isn't viable, as eslint-plugin-prettier also specifies an eslint version of ">=5.0.0". That's fine for that module, but for us we cannot guarantee it due to how we integrate with ESLint.

I think the latter might be possible, but as experience shows, we are not always able to update straight away.

Therefore I've filed an issue on npm on this, I suspect it might be a wontfix, but lets find out: https://github.com/npm/cli/issues/2999

I'm hoping to get to upgrading prettier soon, though I'm currently trying to clear out some other things first.

Severity: -- → S3
Priority: -- → P3
Product: Firefox Build System → Developer Infrastructure
Depends on: 1809497
Assignee: nobody → standard8
Status: NEW → ASSIGNED
Pushed by mbanner@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/c3ae57ff543c Remove eslint-plugin-prettier now that Prettier and ESLint are separated. r=Gijs,devtools-reviewers,ochameau
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: