Skip to content

Conversation

lumirlumir
Copy link
Member

@lumirlumir lumirlumir commented Jul 7, 2025

Prerequisites checklist

What is the purpose of this pull request?

What changes did you make? (Give an overview)

Hello,

In this PR, I've completed migration to React 19.

Starting with React 19, Context.Provider, forwardRef, and several other APIs have been deprecated in favor of a simpler API design.

While using legacy APIs don't currently throw an error, these APIs will be removed in the future, which could block us from upgrading to the latest version of React.

Here's a reference for it:

https://react.dev/blog/2024/12/05/react-19#improvements-in-react-19

image

image

I've made two notable changes in this PR:

  • First, I removed forwardRef and switched to passing refs directly as props.

  • Second, I updated <Context.Provider> to use just <Context>.

Related Issues

Ref: eslint/eslint.org#740

Is there anything you'd like reviewers to focus on?

Copy link

netlify bot commented Jul 7, 2025

Deploy Preview for eslint-code-explorer ready!

Name Link
🔨 Latest commit c1607ae
🔍 Latest deploy log https://app.netlify.com/projects/eslint-code-explorer/deploys/68709f14de11900008db111a
😎 Deploy Preview https://deploy-preview-116--eslint-code-explorer.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

"@types/node": "^18.19.44",
"@types/react": "^18.3.3",
"@types/react-dom": "^18.3.0",
"@types/node": "^20",
Copy link
Member Author

Choose a reason for hiding this comment

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

Currently, both the .nvmrc file and the engines field specify Node.js version 20, so I’ve aligned it to use version 20.

"engines": {
"node": ">= 20"
},

@lumirlumir lumirlumir marked this pull request as ready for review July 10, 2025 13:32
@lumirlumir lumirlumir requested review from a team July 10, 2025 13:35
@github-project-automation github-project-automation bot moved this to Needs Triage in Triage Jul 10, 2025
Copy link

@Copilot Copilot AI left a 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 completes the migration to React 19 by replacing legacy APIs with the new function-based ref and context shorthands.

  • Removed all React.forwardRef wrappers in favor of direct function components that accept ref props.
  • Swapped <Context.Provider> usages for the new <Context> shorthand.
  • Bumped Radix UI and TypeScript type versions in package.json to align with React 19.

Reviewed Changes

Copilot reviewed 14 out of 15 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/components/ui/toggle.tsx Switched Toggle to a function component with direct ref prop
src/components/ui/toggle-group.tsx Updated ToggleGroup and its items to use the new ref and context shorthand
src/components/ui/toast.tsx Refactored Toast primitives to direct function components forwarding ref and props
src/components/ui/text-field.tsx Removed forwardRef, added React.RefAttributes to TextFieldProps
src/components/ui/switch.tsx Converted Switch to function component with direct ref prop
src/components/ui/select.tsx Updated Select primitives to new ref‐accepting functions
src/components/ui/popover.tsx Refactored PopoverContent to accept ref directly and use context shorthand
src/components/ui/label.tsx Switched Label to a function component with direct ref prop
src/components/ui/dropdown-menu.tsx Converted all DropdownMenu primitives from forwardRef to direct‐ref function components
src/components/ui/dialog.tsx Updated Dialog primitives to function components forwarding ref
src/components/ui/button.tsx Changed Button to a function component using ref prop instead of forwardRef
src/components/ui/accordion.tsx Refactored Accordion primitives to direct function components with ref prop
src/components/theme-provider.tsx Replaced <ThemeProviderContext.Provider> with <ThemeProviderContext> shorthand
package.json Bumped @radix-ui/react-toast, @types/node, @types/react, and @types/react-dom versions
Comments suppressed due to low confidence (1)

src/components/theme-provider.tsx:50

  • [nitpick] With the new React 19 context shorthand (<Context> instead of <Context.Provider>), consider adding a comment or updating your README to clarify this syntax for future maintainers who may be unfamiliar with the pattern.
		<ThemeProviderContext {...props} value={{ theme, setTheme }}>

Comment on lines +31 to 43
const Toggle = ({
className,
variant,
size,
ref,
...props
}: React.ComponentPropsWithRef<typeof TogglePrimitive.Root> &
VariantProps<typeof toggleVariants>) => (
<TogglePrimitive.Root
ref={ref}
className={cn(toggleVariants({ variant, size, className }))}
{...props}
/>
Copy link
Preview

Copilot AI Jul 10, 2025

Choose a reason for hiding this comment

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

[nitpick] This manual pattern of destructuring props and forwarding refs is repeated across many UI components; consider creating a higher-order component or factory utility to wrap primitives with consistent ref and className handling to reduce boilerplate and improve maintainability.

Suggested change
const Toggle = ({
className,
variant,
size,
ref,
...props
}: React.ComponentPropsWithRef<typeof TogglePrimitive.Root> &
VariantProps<typeof toggleVariants>) => (
<TogglePrimitive.Root
ref={ref}
className={cn(toggleVariants({ variant, size, className }))}
{...props}
/>
const Toggle = withClassNameAndRef(
TogglePrimitive.Root,
toggleVariants

Copilot uses AI. Check for mistakes.

Copy link
Member

Choose a reason for hiding this comment

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

Yea I felt the same too. At lot of places I was able to see manually de structuring ref and then passing it again as ref={ref} although we were spreading rest of the props. This is a nitpick anyways.

Copy link
Member Author

@lumirlumir lumirlumir Jul 11, 2025

Choose a reason for hiding this comment

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

Sounds good to me.

But, If it's alright, could I handle this refactor in a separate PR? I feel like including it here would make the scope a bit too broad 😅.

@nzakas nzakas moved this from Needs Triage to Implementing in Triage Jul 10, 2025
@lumirlumir lumirlumir added the accepted There is consensus among the team that this change meets the criteria for inclusion label Jul 11, 2025
@lumirlumir
Copy link
Member Author

ping @harish-sethuraman

@lumirlumir
Copy link
Member Author

Hi, @eslint/eslint-team

If anyone has some time, would you mind taking a look at this PR?

Copy link
Member

@nzakas nzakas left a comment

Choose a reason for hiding this comment

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

I don't know enough about React to really evaluate the code, but I tested out the app and everything works as it should. So, let's merge this and see what happens.

@nzakas nzakas merged commit 72aa289 into main Aug 6, 2025
8 checks passed
@nzakas nzakas deleted the refactor-complete-migration-to-react-19 branch August 6, 2025 14:37
@github-project-automation github-project-automation bot moved this from Implementing to Complete in Triage Aug 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion chore
Projects
Status: Complete
Development

Successfully merging this pull request may close these issues.

3 participants