[go: up one dir, main page]
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.

Sign up for GitHub

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

Jump to bottom

BREAKING CHANGE: eslint-plugin-formatjs - Latest minor release forces upgrade to eslint 9 #4536

Closed
heath-freenome opened this issue Sep 23, 2024 · 7 comments
Closed

BREAKING CHANGE: eslint-plugin-formatjs - Latest minor release forces upgrade to eslint 9 #4536

heath-freenome opened this issue Sep 23, 2024 · 7 comments
Labels
bug

Comments

@heath-freenome
Copy link
Contributor
heath-freenome commented Sep 23, 2024 •
edited
Loading

Which package?

eslint-plugin-formatjs

Describe the bug

The recent 4.14.0 is giving me peer dependency issues, saying that it can't find eslint 9. I believe this is due to the internal upgrade to @typescript-eslint v8 which requires eslint 9.

npm error code ERESOLVE
npm error ERESOLVE unable to resolve dependency tree
npm error
npm error While resolving: @freenome/eslint-config-freenome@0.0.0-auto-updated
npm error Found: eslint@8.57.1
npm error node_modules/eslint
npm error   dev eslint@"8.57.1" from the root project
npm error
npm error Could not resolve dependency:
npm error peer eslint@"9" from eslint-plugin-formatjs@4.14.0
npm error node_modules/eslint-plugin-formatjs
npm error   eslint-plugin-formatjs@"4.14.0" from the root project
npm error
npm error Fix the upstream dependency conflict, or retry
npm error this command with --force or --legacy-peer-deps
npm error to accept an incorrect (and potentially broken) dependency resolution.

To Reproduce

Codesandbox URL

I tried but could not get it to install eslint 8

Reproducible Steps/Repo

Steps to reproduce the behavior:

  1. npm install eslint@8
  2. npm install eslint-config-formatjs
  3. See error declared above

Expected behavior

Doing a minor upgrade should not force me to use eslint 9, which I cannot do at this point because other dependencies aren't ready to make the shift.

I highly recommend rolling back the change made in 4.14.0 and pushing that into a major version update
@heath-freenome heath-freenome changed the title BREAKING CHANGE: Latest minor release forces an upgrade to eslint 9 BREAKING CHANGE: Latest minor release forces upgrade to eslint 9 Sep 23, 2024
@heath-freenome heath-freenome changed the title BREAKING CHANGE: Latest minor release forces upgrade to eslint 9 BREAKING CHANGE: eslint-config-formatjs - Latest minor release forces upgrade to eslint 9 Sep 23, 2024
@heath-freenome heath-freenome changed the title BREAKING CHANGE: eslint-config-formatjs - Latest minor release forces upgrade to eslint 9 BREAKING CHANGE: eslint-plugin-formatjs - Latest minor release forces upgrade to eslint 9 Sep 23, 2024
@graue
Copy link
Contributor
graue commented Sep 25, 2024 •
edited
Loading

I'm also running into this.

It might be as simple as changing the eslint peer dependency, which was previously '7 || 8', to '7 || 8 || 9'

Dropping 7 and 8 is definitely a breaking change. Based on the commit, f9a0e1b, it looks like it wasn't intentional - cc @michaelfaith

graue referenced this issue Sep 25, 2024
@michaelfaith @longlho
feat(eslint-plugin-formatjs): add support for eslint v9
f9a0e1b 
This change adds support for eslint v9.  In order to provide backwards compatibility, I added a `context-compat` module in order to message the `context` object into the appropriate form, depending on the version used.  This is mainly only being used for get the `parserServices`, which lives under the `sourceCode` object in v9, but directly on the `context` in earlier versions.

Additionally, the `RuleModule` type changed to include an additional generic parameter for pluginDocs.  Since the existing rules aren't adding any additional props specific for the plugin, there wasn't a need to create a new type for that.

Lastly, v9 is stricter about running exact duplicate tests through the `RuleTester`, and there was one exact duplicate that I removed.
@michaelfaith
Copy link
Contributor

It might be as simple as changing the eslint peer dependency, which was previously '7 || 8', to '7 || 8 || 9'

Dropping 7 and 8 is definitely a breaking change. Based on the commit, f9a0e1b, it looks like it wasn't intentional - cc @michaelfaith

That's actually what I did. I just went back and pulled up the PR and I had it with all three (https://github.com/formatjs/formatjs/pull/4505/files#diff-ee71fcd23af2b6ff9e4ee7d37e0e1978201a7b0e2624ea2e130927136e5d5058). I'm guessing that was changed by one of the maintainers after merging the PR?

I was having trouble with getting the BAZEL stuff running locally, so they picked it up and took it the rest of the way, after I got local jest tests green. But also, @heath-freenome is right to say that @typescript-eslint/utils would need something similar, if 7 and all of 8 were going to continue to be supported, since the latest major of that library only supports "^8.57.0 || ^9.0.0"

@longlho
Copy link
Member
longlho commented Sep 25, 2024

Sorry I'll major bump the packages and mark the recently released version as deprecated.

@michaelfaith
Copy link
Contributor
michaelfaith commented Sep 25, 2024 •
edited
Loading

It should be relatively easy to include support for 8.57 at least, if you wanted to have some backwards compatability (even if it's still breaking). But then you'd have to major again when v10 comes out and v8 support is dropped...

@longlho
Copy link
Member
longlho commented Sep 25, 2024

Thanks for the suggestion. Nope, with the capacity I have I can only support 1 major eslint version.

@longlho longlho closed this as completed Sep 25, 2024
@heath-freenome
Copy link
Contributor Author
heath-freenome commented Sep 25, 2024 •
edited
Loading

Sorry I'll major bump the packages and mark the recently released version as deprecated.

@longlho Can you also consider releasing a 4.14.1 that reverts the changes for 4.14.0 since my RenovateBot build system keeps trying to pick up 4.14.0?

graue added a commit to bikehopper/bikehopper-ui that referenced this issue Sep 25, 2024
@graue
pin eslint-plugin-formatjs to ~4.13.3
0d08088 
work around formatjs/formatjs#4536
@graue
Copy link
Contributor
graue commented Sep 25, 2024

FWIW, I pinned our project's dependency to 4.13.x by using ~4.13.3 as the version, so that may be an acceptable local workaround if the marking of the 4.14.0 version as deprecated doesn't do it for your CI setup.

@longlho, thanks for maintaining the library, and for your attention to the issue. @michaelfaith, thanks for doing the upgrade and taking the effort to thoughtfully try to make it backwards compatible, even though it didn't end up shipping that way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@longlho @graue @michaelfaith @heath-freenome