React 19 compatibility #2934

vladmoroz · 2024-06-04T11:06:54Z

Access ref from props when possible
Remove propTypes usage
Replace our ComponentPropsWithoutRef type with the built-in React type

Handy link:
https://react.dev/blog/2024/04/25/react-19-upgrade-guide

vladmoroz · 2024-06-04T11:14:06Z

packages/react/accordion/src/Accordion.tsx

-Accordion.propTypes = {
-  type(props) {
-    const value = props.value || props.defaultValue;
-    if (props.type && !['single', 'multiple'].includes(props.type)) {
-      return new Error(
-        'Invalid prop `type` supplied to `Accordion`. Expected one of `single | multiple`.'
-      );
-    }
-    if (props.type === 'multiple' && typeof value === 'string') {
-      return new Error(
-        'Invalid prop `type` supplied to `Accordion`. Expected `single` when `defaultValue` or `value` is type `string`.'
-      );
-    }
-    if (props.type === 'single' && Array.isArray(value)) {
-      return new Error(
-        'Invalid prop `type` supplied to `Accordion`. Expected `multiple` when `defaultValue` or `value` is type `string[]`.'
-      );
-    }
-    return null;
-  },
-};
-


Prop types have been deprecated for a while and are now completely removed
https://react.dev/blog/2024/04/25/react-19-upgrade-guide#removed-deprecated-react-apis

Using TypeScript already handles this

vladmoroz · 2024-06-04T11:17:54Z

packages/react/progress/src/Progress.tsx

+    if ((maxProp || maxProp === 0) && !isValidMaxNumber(maxProp)) {
+      console.error(getInvalidMaxError(`${maxProp}`, 'Progress'));
+    }
+
    const max = isValidMaxNumber(maxProp) ? maxProp : DEFAULT_MAX;
+
+    if (valueProp !== null && !isValidValueNumber(valueProp, max)) {
+      console.error(getInvalidValueError(`${valueProp}`, 'Progress'));
+    }
+


Relocated the logic from propTypes that TypeScript can't help with

vladmoroz · 2024-06-04T11:18:13Z

packages/react/separator/src/Separator.tsx

-Separator.propTypes = {
-  orientation(props, propName, componentName) {
-    const propValue = props[propName];
-    const strVal = String(propValue);
-    if (propValue && !isValidOrientation(propValue)) {
-      return new Error(getInvalidOrientationError(strVal, componentName));
-    }
-    return null;
-  },
-};
-
 /* -----------------------------------------------------------------------------------------------*/

-// Split this out for clearer readability of the error message.
-function getInvalidOrientationError(value: string, componentName: string) {
-  return `Invalid prop \`orientation\` of value \`${value}\` supplied to \`${componentName}\`, expected one of:
-  - horizontal
-  - vertical
-
-Defaulting to \`${DEFAULT_ORIENTATION}\`.`;
-}
-


TypeScript already handles this

vladmoroz · 2024-06-04T11:18:33Z

packages/react/toast/src/Toast.tsx

+
+  if (!label.trim()) {
+    console.error(
+      `Invalid prop \`label\` supplied to \`${PROVIDER_NAME}\`. Expected non-empty \`string\`.`
+    );
+  }
+


Relocated from propTypes

vladmoroz · 2024-06-04T11:18:44Z

packages/react/toast/src/Toast.tsx

+
+    if (!altText.trim()) {
+      console.error(
+        `Invalid prop \`altText\` supplied to \`${ACTION_NAME}\`. Expected non-empty \`string\`.`
+      );
+      return null;
+    }
+


Relocated from propTypes

vladmoroz · 2024-06-04T11:21:28Z

packages/react/presence/src/Presence.tsx

+// Before React 19 accessing `element.props.ref` will throw a warning and suggest using `element.ref`
+// After React 19 accessing `element.ref` does the opposite
+// https://github.com/facebook/react/pull/28348
+//
+// Access the ref using the method that doesn't yield a warning
+function getElementRef(element: React.ReactElement) {
+  // Pre React 19 there's a getter on `element.props.ref` that throws a warning when attempting to access it.
+  // This is safe to rely on. (As in... obviously, old React versions won't change).
+  // https://github.com/facebook/react/blob/408258268edb5acdfdbf77bc6e0b0dc6396c0e6f/packages/react/src/jsx/ReactJSXElement.js#L89-L99
+  const getter = Object.getOwnPropertyDescriptor(element.props, 'ref')?.get;
+  const hasPropWarning = getter && 'isReactWarning' in getter && getter.isReactWarning;
+
+  if (hasPropWarning) {
+    return (element as any).ref;
+  }
+
+  return element.props.ref;
+}
+


Ref access instance 1 of 2

A bit messy checking for the getter presence, but a naïve attempt to access either element.props.ref or element.ref directly like in #2811 yields a warning pre React 19

vladmoroz · 2024-06-04T11:21:51Z

packages/react/primitive/src/Primitive.tsx

-// Temporary while we await merge of this fix:
-// https://github.com/DefinitelyTyped/DefinitelyTyped/pull/55396
-// prettier-ignore
-type PropsWithoutRef<P> = P extends any ? ('ref' extends keyof P ? Pick<P, Exclude<keyof P, 'ref'>> : P) : P;
-type ComponentPropsWithoutRef<T extends React.ElementType> = PropsWithoutRef<
-  React.ComponentProps<T>
->;
-
-type Primitives = { [E in typeof NODES[number]]: PrimitiveForwardRefComponent<E> };


This stuff is integrated into @types/react

vladmoroz · 2024-06-04T11:22:09Z

packages/react/slot/src/Slot.tsx

+// Before React 19 accessing `element.props.ref` will throw a warning and suggest using `element.ref`
+// After React 19 accessing `element.ref` does the opposite
+// https://github.com/facebook/react/pull/28348
+//
+// Access the ref using the method that doesn't yield a warning
+function getElementRef(element: React.ReactElement) {
+  // Pre React 19 there's a getter on `element.props.ref` that throws a warning when attempting to access it.
+  // This is safe to rely on. (As in... obviously, old React versions won't change).
+  // https://github.com/facebook/react/blob/408258268edb5acdfdbf77bc6e0b0dc6396c0e6f/packages/react/src/jsx/ReactJSXElement.js#L89-L99
+  const getter = Object.getOwnPropertyDescriptor(element.props, 'ref')?.get;
+  const hasPropWarning = getter && 'isReactWarning' in getter && getter.isReactWarning;
+
+  if (hasPropWarning) {
+    return (element as any).ref;
+  }
+
+  return element.props.ref;
+}
+


Ref access instance 2 of 2

vladmoroz · 2024-06-04T11:22:52Z

packages/react/slot/src/Slot.tsx

+      // @ts-ignore
+      ref: forwardedRef ? composeRefs(forwardedRef, childrenRef) : childrenRef,


Setting the ref property has always yielded a type error, just didn't break the build before

I wonder what's the future of the Slot component as React considers cloneElement a legacy api. 🤔

I wonder what's the future of the Slot component as React considers cloneElement a legacy api. 🤔

We'll see—I haven't seen a good alternative there, and "legacy" doesn't mean it's deprecated yet. I think it will stay around for a while.

packages/react/focus-scope/src/FocusScope.test.tsx

package.json

lucasmotta · 2024-06-10T14:53:17Z

packages/react/presence/src/Presence.tsx

+// https://github.com/facebook/react/pull/28348
+//
+// Access the ref using the method that doesn't yield a warning.
+function getElementRef(element: React.ReactElement) {


🤯

Should we extract this to a utility and reuse in other places instead of having the duplicated code?

Would have to be a separate package—I'd say we can keep it like this for now and extract it later if there's 3+ uses. I think two copy-pastes is OK for now.

lucasmotta · 2024-06-10T14:57:03Z

packages/react/slot/src/Slot.tsx

+      // @ts-ignore
+      ref: forwardedRef ? composeRefs(forwardedRef, childrenRef) : childrenRef,


I wonder what's the future of the Slot component as React considers cloneElement a legacy api. 🤔

lucasmotta · 2024-06-10T14:57:25Z

packages/react/slot/src/Slot.tsx

+// https://github.com/facebook/react/pull/28348
+//
+// Access the ref using the method that doesn't yield a warning.
+function getElementRef(element: React.ReactElement) {


Same method as before, let's extract to a utility.

It looks like we could save bundle size here. How about going with

return React.version.split('.')[0] >= '19' ? element.props.ref : element.ref;

or

return element.props.propertyIsEnumerable('ref') ? element.props.ref : element.ref;

for this method? It seems to pass the tests.

CarlosZiegler · 2024-06-12T18:40:46Z

Amazing work, how can I test ? some RC ?

vladmoroz · 2024-06-12T19:46:31Z

@CarlosZiegler yes, the latest RC versions include this PR

chungweileong94 and others added 13 commits April 2, 2024 22:04

Access ref from props before element.ref

82bff21

Apply ref access fix to presence component

45b510a

adding react 19 to peer deps

2be7074

Upgrade to React 18.3.1

44c9aa9

Merge branch 'chungweileong94/main' into vlad/react-upgrades

4daf046

Merge branch 'ben/react-19' into vlad/react-upgrades

980acb3

Add missing React 19 peer deps

21da11f

Remove propTypes

bdbbc87

Update ReactDOM usage

09c0140

yarn

7a5e1b3

Upgrade to React 19 and access refs in a backwards compatible way

eccd0b0

Recover Storybook

8750d9c

Remove an outdated type

2bbd60a

vladmoroz requested review from StephenHaney, lucasmotta and hadihallak as code owners June 4, 2024 11:06

Vlad Moroz added 4 commits June 4, 2024 13:09

Suppress a type error, tweak formatting

6e97323

Track version changes

a5a3075

Don't throw in render, just log the error

b02edfc

Update and upgrade tests

9f74b8a

vladmoroz mentioned this pull request Jun 4, 2024

Adding react 19 to peer deps #2924

Closed

vladmoroz commented Jun 4, 2024

View reviewed changes

Roll back tests deps changes

303c39a

vladmoroz mentioned this pull request Jun 4, 2024

[Slot] Access ref from props else fallback to element.ref #2811

Closed

eps1lon reviewed Jun 4, 2024

View reviewed changes

packages/react/focus-scope/src/FocusScope.test.tsx Show resolved Hide resolved

eps1lon reviewed Jun 4, 2024

View reviewed changes

package.json Outdated Show resolved Hide resolved

eps1lon mentioned this pull request Jun 4, 2024

React 19 library compat eps1lon/react#10

Closed

22 tasks

Vlad Moroz added 3 commits June 4, 2024 15:08

^ in react version affects tests?

f1bd85f

Upgrade testing-library and wrap FocusScope tests in act

007ba3a

Upgrade @testing-library/react to 16.0

d8f2071

Thijs-Lambert mentioned this pull request Jun 4, 2024

[bug]: React 19 forwardRef warnings shadcn-ui/ui#3898

Open

2 tasks

vladmoroz force-pushed the vlad/react-upgrades branch 5 times, most recently from 810b590 to d8f2071 Compare June 4, 2024 17:00

Vlad Moroz added 3 commits June 4, 2024 22:37

Of cooooourse

f3575fa

Remove unnecessary changes

ceb0e7c

Refine getElementRef

e890cc1

vladmoroz force-pushed the vlad/react-upgrades branch from fbed4de to 80a94d7 Compare June 5, 2024 08:10

eps1lon mentioned this pull request Jun 5, 2024

compat with react 19 diegomura/react-pdf#2756

Open

Vlad Moroz added 2 commits June 5, 2024 10:50

Upgrade start-server-and-test

e2f4e2e

Flaky cypress test?

f1f02eb

vladmoroz force-pushed the vlad/react-upgrades branch from 80a94d7 to f1f02eb Compare June 5, 2024 08:51

Remove unnecessary changes

ac7d8a4

oliviertassinari mentioned this pull request Jun 5, 2024

[core] Support React 19 mui/material-ui#42381

Open

10 tasks

SeanCassiere mentioned this pull request Jun 6, 2024

Tracking React 19 SeanCassiere/nv-rental-clone#362

Open

lucasmotta reviewed Jun 10, 2024

View reviewed changes

lucasmotta approved these changes Jun 10, 2024

View reviewed changes

vladmoroz merged commit 06de2d4 into main Jun 10, 2024
5 checks passed

vladmoroz deleted the vlad/react-upgrades branch June 10, 2024 16:02

vladmoroz mentioned this pull request Jun 11, 2024

[Progress] Assume indeterminate value #2947

Merged

LekoArts mentioned this pull request Jun 21, 2024

chore(elements): Update Radix deps clerk/javascript#3606

Merged

9 tasks

oliviertassinari mentioned this pull request Jul 3, 2024

[material-ui] Fix Accessing element.ref mui/material-ui#42818

Merged

1 task

eps1lon mentioned this pull request Jul 16, 2024

Incompatible with React 19 due to use of __SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED tailwindlabs/headlessui#3167

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

React 19 compatibility #2934

React 19 compatibility #2934

vladmoroz commented Jun 4, 2024 •

edited

Loading

vladmoroz Jun 4, 2024

vladmoroz Jun 4, 2024

vladmoroz Jun 4, 2024

vladmoroz Jun 4, 2024

vladmoroz Jun 4, 2024

vladmoroz Jun 4, 2024 •

edited

Loading

vladmoroz Jun 4, 2024

vladmoroz Jun 4, 2024

vladmoroz Jun 4, 2024

lucasmotta Jun 10, 2024

vladmoroz Jun 10, 2024

lucasmotta Jun 10, 2024 •

edited

Loading

vladmoroz Jun 10, 2024

lucasmotta Jun 10, 2024

lucasmotta Jun 10, 2024

oliviertassinari Jul 3, 2024 •

edited

Loading

CarlosZiegler commented Jun 12, 2024

vladmoroz commented Jun 12, 2024

		// @ts-ignore
		ref: forwardedRef ? composeRefs(forwardedRef, childrenRef) : childrenRef,

React 19 compatibility #2934

React 19 compatibility #2934

Conversation

vladmoroz commented Jun 4, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vladmoroz Jun 4, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lucasmotta Jun 10, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oliviertassinari Jul 3, 2024 • edited Loading

Choose a reason for hiding this comment

CarlosZiegler commented Jun 12, 2024

vladmoroz commented Jun 12, 2024

vladmoroz commented Jun 4, 2024 •

edited

Loading

vladmoroz Jun 4, 2024 •

edited

Loading

lucasmotta Jun 10, 2024 •

edited

Loading

oliviertassinari Jul 3, 2024 •

edited

Loading