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

[next@14] Update eslint-plugin-react-hooks to stable v5 #71291

Open
wants to merge 1 commit into
base: 14-2-1
Choose a base branch
from

Conversation

lorand-horvath
Copy link

@lorand-horvath lorand-horvath commented Oct 15, 2024

eslint-plugin-react-hooks should be bumped to stable v5 for next@14 as well.
Justification here: #71218 (comment)

@ijjk
Copy link
Member

ijjk commented Oct 15, 2024

Allow CI Workflow Run

  • approve CI run for commit: cf2bc75

Note: this should only be enabled once the PR is ready to go and can only be enabled by a maintainer

@lorand-horvath lorand-horvath changed the title Update eslint-plugin-react-hooks to stable v5 Update eslint-plugin-react-hooks to stable v5 for next@14 Oct 15, 2024
@lorand-horvath lorand-horvath changed the title Update eslint-plugin-react-hooks to stable v5 for next@14 [next@14] Update eslint-plugin-react-hooks to stable v5 Oct 15, 2024
Copy link
Member

@eps1lon eps1lon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using [email protected] is a breaking change. If anything, we should backport a downgrade. Existing apps just didn't see the breaking change during upgrade when they were using lockfiles.

@lorand-horvath
Copy link
Author

Using [email protected] is a breaking change.

@eps1lon Why so? At the moment next@14 has a dependency on ^4.5.0 || 5.0.0-canary-7118f5dd7-20230705. I actually wonder why the 5.0.0-canary (from July 2023) was added in the first place? I have a justification for why v5 is a better solution, if you don't mind reading it, here: #71218 (comment)

Also eslint-plugin-react-hooks@5 supports eslint versions ^3 to ^9 as per https://github.com/facebook/react/blob/main/packages/eslint-plugin-react-hooks/package.json

Anyway, if v5 is not acceptable, I can modify the PR to downgrade it to ^4.6.2.

Copy link
Member

eps1lon commented Oct 15, 2024

Using [email protected] is a breaking change.

Generally, because it's released in a SemVer major. Here specifically, it's the introduction of a new violation.

It's not a critical fix. Technically breaking again if you already relied on the new behavior considering for how long this has been out there. I don't see a path to safely backport nor the urgency to do this. You could always use resolutions or overrides to fix it.

lorand-horvath added a commit to lorand-horvath/next.js that referenced this pull request Oct 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants