-
Notifications
You must be signed in to change notification settings - Fork 640
chore(deps): update to latest compiler plugins #7113
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
base: main
Are you sure you want to change the base?
Conversation
|
|
👋 Hi, this pull request contains changes to the source code that github/github depends on. If you are GitHub staff, we recommend testing these changes with github/github using the integration workflow. Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This pull request upgrades the React Hooks ESLint plugin from version 5.2.0 to 7.0.1 and replaces the React Compiler plugin with the new hooks plugin. The upgrade introduces new lint rules that detect React patterns that conflict with the React Compiler, requiring updates to eslint-disable comments throughout the codebase.
- Removes
eslint-plugin-react-compilerdependency and replaces it with the enhancedeslint-plugin-react-hooksv7.0.1 - Updates React compiler runtime and babel plugin to stable version 1.0.0
- Adds new eslint-disable comments for newly detected React Compiler violations across ~40 files
- Refactors some code patterns to be more compatible with React Compiler (e.g.,
useRenderForcingRef,ProgressBar.stories)
Reviewed Changes
Copilot reviewed 53 out of 54 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| package.json | Removes eslint-plugin-react-compiler, upgrades eslint-plugin-react-hooks to v7.0.1 |
| eslint.config.mjs | Removes react-compiler plugin config, updates to use react-hooks flat config |
| packages/react/package.json | Updates react-compiler-runtime and babel-plugin-react-compiler to stable v1.0.0 |
| packages/react/src/hooks/useRenderForcingRef.ts | Moves ref.current assignment from render to useEffect to avoid compiler violations |
| packages/react/src/hooks/useMediaUnsafeSSR.tsx | Refactors conditional setState from useEffect to render phase with proper guard |
| packages/react/src/ProgressBar/ProgressBar.stories.tsx | Removes unnecessary state and useEffect, simplifies to derived variable |
| packages/react/src/Token/_RemoveTokenButton.tsx | Removes runtime delete rest.children, uses destructuring to exclude children prop |
| Various component files | Adds eslint-disable comments for legitimate React Compiler violations (refs, set-state-in-effect, immutability, etc.) |
| useEffect(() => { | ||
| ref.current = refCurrent | ||
| }, [refCurrent]) |
Copilot
AI
Oct 31, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This useEffect creates a synchronization issue. The ref is being set in both the setRef callback (line 16) and in this effect (line 23). This means ref.current could be stale between when setRef is called and when the effect runs. Since setRef already updates ref.current = newRef on line 16, this effect is redundant and could cause the ref to be set twice with potentially different values, creating race conditions.
| if (features[mediaQueryString] !== undefined && matches !== features[mediaQueryString]) { | ||
| setMatches(features[mediaQueryString] as boolean) | ||
| } |
Copilot
AI
Oct 31, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Calling setMatches during render (outside of useEffect) can cause infinite re-render loops if the component re-renders for any reason while features[mediaQueryString] is defined. This violates React's principle that render functions should be pure. The original useEffect pattern at lines 46-48 (before this change) was the correct approach to sync state based on context changes.
| // eslint-disable-next-line react-hooks/refs | ||
| {...rest} |
Copilot
AI
Oct 31, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The eslint-disable comment on line 46 disables the react-hooks/refs rule for the spread operator, but the actual issue is on line 55 where rest.ref is explicitly accessed. The disable comment should be moved to line 54 (before the ref prop) rather than line 46, as spreading rest props is not the violation—accessing rest.ref is.
| // eslint-disable-next-line react-hooks/refs | ||
| ref={rest.ref as React.Ref<HTMLButtonElement>} |
Copilot
AI
Oct 31, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] With the new pattern of destructuring children: _children on line 24, the rest object no longer contains children. However, it still contains ref which is being explicitly extracted on line 55. For consistency and to avoid the eslint-disable comment, consider also destructuring ref in the function parameters: ref: forwardedRef, children: _children and then using ref={forwardedRef as React.Ref<HTMLButtonElement>}.
Update project to use v1.0 of React Compiler
Changelog
New
Changed
Removed
eslint-plugin-react-compilerdependency in favor of the latest react hooks pluginRollout strategy