-
Notifications
You must be signed in to change notification settings - Fork 218
Upgrade React to v19.2.3 (and fix type errors in useRef hooks) #4237
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
Conversation
Co-authored-by: d10c <[email protected]>
Co-authored-by: d10c <[email protected]>
Co-authored-by: d10c <[email protected]>
…ependabot merge) Co-authored-by: d10c <[email protected]>
…erability Co-authored-by: d10c <[email protected]>
- Upgrade react and react-dom from 18.3.1 to 19.2.3 - Upgrade @types/react and @types/react-dom to 19.2.3 (matching versions) - Fix useRef hooks to include explicit initial values (React 19 requirement) - Update ref prop types to handle React 19's stricter null handling - Fix eslint config for reactHooks.configs compatibility Co-authored-by: d10c <[email protected]>
The previous change removed the .flat accessor which caused ESLint to use the old eslintrc format config (with plugins as array of strings) instead of the flat config format (with plugins as object). This led to CI failures. Co-authored-by: d10c <[email protected]>
Updated react, react-dom, @types/react, and @types/react-dom to use "^19.2.3" instead of "19.2.3" to match the convention used by other dependencies in the project. Co-authored-by: d10c <[email protected]>
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 PR upgrades React and React DOM from v18.3.1 to v19.2.3, along with their corresponding type definitions. The upgrade requires fixing type errors caused by React 19's stricter type requirements for useRef hooks, which now require explicit initial values.
Key changes:
- Upgraded React packages to v19.2.3 with matching type definitions
- Updated
useRefcalls to include explicitundefinedinitial values where DOM refs are used - Updated ref-related types throughout the codebase to include
| nullto align with React 19's type system
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
package.json |
Updated React and React DOM to ^19.2.3, along with their type definitions |
package-lock.json |
Resolved dependencies including React 19.2.3, scheduler 0.27.0, removed @types/prop-types dependency |
useScrollIntoView.tsx |
Added generic type parameter E and updated ref type to accept E | null |
AlertTableResultRow.tsx |
Updated selectedItemRef prop type to include | null |
AlertTablePathRow.tsx |
Updated selectedItemRef prop type to include | null |
AlertTablePathNodeRow.tsx |
Updated selectedItemRef prop type to include | null |
MethodRow.tsx |
Added explicit undefined initial value to useRef call |
ModelAlertsResults.tsx |
Added explicit undefined initial value to useRef call |
AlertTableResultRow.spec.tsx |
Updated test mock ref type to include | null |
AlertTablePathRow.spec.tsx |
Updated test mock ref type to include | null |
Files not reviewed (1)
- extensions/ql-vscode/package-lock.json: Language not supported
Comments suppressed due to low confidence (1)
extensions/ql-vscode/src/view/model-alerts/ModelAlertsResults.tsx:95
- The ref is created and used in a useEffect hook to scroll into view, but it's never actually attached to any DOM element in the JSX. The ref should be attached to one of the elements in the return statement (likely the outermost div or TitleContainer) using
ref={ref}for the scroll functionality to work.
const ref = useRef<HTMLElement | undefined>(undefined);
useEffect(() => {
if (
revealedModel &&
createModeledMethodKey(modelAlerts.model) ===
createModeledMethodKey(revealedModel)
) {
ref.current?.scrollIntoView({
behavior: "smooth",
block: "center",
});
}
}, [modelAlerts.model, revealedModel]);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
I would want to squash these commits before merging, and therefore recommend reviewing all files at once instead of commit-by-commit. |
React 19 requires explicit initial values for
useRef. Previously valid patterns likeuseRef<HTMLElement>()now fail type checking.Changes
React upgrade
reactandreact-domto^19.2.3@types/reactand@types/react-domto^19.2.3(matching versions with runtime packages, using caret prefix per project convention)Type fixes
undefinedinitial values touseRef<HTMLElement>()calls inModelAlertsResults.tsxandMethodRow.tsxuseScrollIntoViewsignature to acceptRefObject<E | null>instead ofRefObject<HTMLElement>AlertTableResultRow,AlertTablePathRow, andAlertTablePathNodeRowto include| nullConfiguration
reactHooks.configs.flat['recommended-latest']is used for flat config compatibility (the non-flat config uses old eslintrc format which is incompatible with ESLint flat config)Example
Before:
After:
Remember to update the changelog if there have been user-facing changes!
Original prompt
This pull request was created from Copilot chat.
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.