[core] React 19 compatibility #605

michaldudak · 2024-09-12T17:00:29Z

Work in progress to ensure Base UI is compatible with React 19.

After this is approved, I'll revert all the changes to package.json files, and, if the checks are green, merge it into master.

mui-bot · 2024-09-12T17:04:39Z

Netlify deploy preview

https://deploy-preview-605--base-ui.netlify.app/

Generated by 🚫 dangerJS against e1403e9

packages/mui-base/src/utils/reactVersion.ts

oliviertassinari

Connecting the dots with the other projects: Aaron went through a lot of those changes mui/material-ui#42824. I think it would be great to have his review.

docs/src/blocks/GoogleAnalytics.tsx

docs/src/components/GoogleAnalytics.tsx

oliviertassinari · 2024-09-14T15:13:11Z

docs/src/design-system/ToggleButtonGroup.tsx

-) => JSX.Element;
+) => React.JSX.Element;


👍 to bring this to the main branch

I intend to have everything (except for package.json changes) merged to master.

oliviertassinari · 2024-09-15T00:45:26Z

packages/mui-base/src/utils/useComponentRenderer.ts

@@ -79,7 +79,7 @@ export function useComponentRenderer<

  let resolvedRenderProp:
    | ComponentRenderFn<React.HTMLAttributes<any>, OwnerState>
-    | React.ReactElement;
+    | React.ReactElement<any>;


? mui/material-ui#43402

Suggested change

| React.ReactElement<any>;

| React.ReactElement<unknown>;

We don't really care about the shape of the props here, especially that it's an internal hook and its type shouldn't be publicly visible. To constrain it a bit more, I changed it to Record<string, unknown>

oliviertassinari · 2024-09-15T00:47:49Z

packages/mui-base/src/utils/useRenderPropForkRef.ts

+  let childRef;
+  if (isReactVersionAtLeast(19)) {
+    childRef = typeof render !== 'function' ? render.props.ref : null;
+  } else {
+    childRef = typeof render !== 'function' ? render.ref : null;
+  }


Better fork after the typeof.

I'm sure this is best: mui/material-ui#42818 (comment). The explore we are exploring in mui/material-ui#43022. To decide then use the same solution through the codebase.

Better fork after the typeof

You mean const childRef = typeof render !== 'function' ? isReactVersionAtLeast(19) ? render.props.ref : render.ref : null;? This makes it a nested ternary which I believe we disallow in our ESLint rules.

I'm sure this is best: mui/material-ui#42818 (comment).

How do you measure "best"? The version check is faster than property check (https://jsbench.me/mrm16egz7n/1) and the function call explains pretty well that we're branching because something changed in React version.

@michaldudak I was proposing to go from:

let childRef; if (isReactVersionAtLeast(19)) { childRef = typeof render !== 'function' ? render.props.ref : null; } else { childRef = typeof render !== 'function' ? render.ref : null; }

to:

let childRef; if (typeof render !== 'function') { childRef = isReactVersionAtLeast(19) ? render.props.ref : render.ref; } else { childRef = null; }

something we can later convert to:

let childRef; if (typeof render !== 'function') { childRef = getReactElementRef(render); } else { childRef = null; }

How do you measure "best"?

To be defined 😄. We had another iteration in mui/material-ui#43022 with @sai6855. I could see the purpose of going with the React version check. I don't see one path as clearly better than the other, so happy either way.

The only ask is that we are moving to a place where Base UI is exporting one helper and Material UI, etc. using it. I don't think that this should be duplicated.

I was proposing to go...

All right, makes sense 👍

The only ask is that we are moving to a place where Base UI is exporting one helper and Material UI, etc. using it. I don't think that this should be duplicated.

I see your point. I don't feel comfortable sharing Base UI utils just yet, though, even internally. When we're closer to the release and breaking changes are less likely, we can review the whole codebase and move utilities where they ultimately belong.

I don't feel comfortable sharing Base UI utils just yet, though, even internally

This doesn't go in opposition with the ask 👍. We could duplicate the code of the helper until this happens (e.g. we have done this in MUI X mui/mui-x#14528 we had the same warnOnce() helper duplicated between packages). The outcome I'm trying to go after is: learn once, fix everywhere once. If one is better than the other, there is no reason for one project to do it differently than the other. If the difference is marginal, we are much better serviced using the same approach, so we can move everything at once when we learn more about the limits.

packages/mui-base/src/utils/useAnchorPositioning.ts

…ility

michaldudak · 2024-09-17T20:48:43Z

docs/data/components/tooltip/UnstyledTooltipFollowCursor.js

@@ -7,7 +7,7 @@ import { styled } from '@mui/system';
 export default function UnstyledTooltipFollowCursor() {
  return (
    <div style={{ display: 'flex', gap: 12 }}>
-      <Tooltip.Root followCursorAxis="both">
+      <Tooltip.Root trackCursorAxis="both">


It's not clear to me why it started to fail now, but I guess it's a good thing it fails.

michaldudak · 2024-09-17T20:51:01Z

packages/mui-base/src/Field/Validity/FieldValidity.test.tsx

@@ -23,7 +23,7 @@ describe('<Field.Validity />', () => {
    fireEvent.change(input, { target: { value: 'test' } });
    fireEvent.blur(input);

-    const [data] = handleValidity.args[8];
+    const [data] = handleValidity.lastCall.args;


@atomiks, could you please verify that with these changes the intent of these checks is the same?

Yes, that's fine

…ility

michaldudak added 3 commits September 12, 2024 10:23

Update dependencies

e4b2791

Fix handling render props

f73b90c

Diry fixes of other unit tests

61442e7

michaldudak added the core Infrastructure work going on behind the scenes label Sep 12, 2024

Dedupe

a50c971

michaldudak added 3 commits September 13, 2024 08:50

Add reactVersion utility

532b7c7

Fix ref type issues

18cd409

Fix other TS errors

fba6e6a

atomiks reviewed Sep 13, 2024

View reviewed changes

packages/mui-base/src/utils/reactVersion.ts Outdated Show resolved Hide resolved

michaldudak added 6 commits September 13, 2024 09:47

Fix reactVersion

8e9cb19

Use React.version

a3468ad

Dedupe

22302ba

Disable next worker threads

a48cf6c

Fix Positioner's anchor prop

3f47481

Wrap GA in NoSsr

6e13642

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Sep 14, 2024

oliviertassinari added the React 19 support PRs required to support React 19 label Sep 15, 2024

oliviertassinari reviewed Sep 15, 2024

View reviewed changes

oliviertassinari mentioned this pull request Sep 15, 2024

[material-ui] Improve getReactElementRef() utils mui/material-ui#43022

Merged

Merge remote-tracking branch 'upstream/master' into react-19-compatib…

08c4972

…ility

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Sep 17, 2024

michaldudak added 7 commits September 17, 2024 13:08

Dedupe

a71517d

Fix TS error

c1c2f41

Maintain compatibility with React 18

10978a0

Clean up

ddb980c

Refactor useRenderPropForkRef

7690aad

Reduce the use of ReactElement<any>

76e7adb

Remove the use of ReactElement<any>

3e174c8

michaldudak commented Sep 17, 2024

View reviewed changes

Remove @ts-ignore

9b037d0

michaldudak marked this pull request as ready for review September 17, 2024 20:54

michaldudak requested a review from colmtuite as a code owner September 17, 2024 20:54

Deps

0544b4e

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Sep 18, 2024

Merge remote-tracking branch 'upstream/master' into react-19-compatib…

75f56af

…ility

github-actions bot added PR: out-of-date The pull request has merge conflicts and can't be merged and removed PR: out-of-date The pull request has merge conflicts and can't be merged labels Sep 18, 2024

Merge remote-tracking branch 'upstream/master' into react-19-compatib…

26c88a1

…ility

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Sep 20, 2024

getInertValue

e1403e9

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[core] React 19 compatibility #605

[core] React 19 compatibility #605

michaldudak commented Sep 12, 2024 •

edited

Loading

mui-bot commented Sep 12, 2024 •

edited

Loading

oliviertassinari left a comment

oliviertassinari Sep 14, 2024

michaldudak Sep 17, 2024

oliviertassinari Sep 15, 2024

michaldudak Sep 17, 2024

oliviertassinari Sep 15, 2024 •

edited

Loading

michaldudak Sep 17, 2024

oliviertassinari Sep 17, 2024 •

edited

Loading

michaldudak Sep 17, 2024

oliviertassinari Sep 17, 2024 •

edited

Loading

michaldudak Sep 17, 2024

michaldudak Sep 17, 2024

atomiks Sep 18, 2024

[core] React 19 compatibility #605

Are you sure you want to change the base?

[core] React 19 compatibility #605

Conversation

michaldudak commented Sep 12, 2024 • edited Loading

mui-bot commented Sep 12, 2024 • edited Loading

Netlify deploy preview

oliviertassinari left a 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

oliviertassinari Sep 15, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oliviertassinari Sep 17, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oliviertassinari Sep 17, 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

michaldudak commented Sep 12, 2024 •

edited

Loading

mui-bot commented Sep 12, 2024 •

edited

Loading

oliviertassinari Sep 15, 2024 •

edited

Loading

oliviertassinari Sep 17, 2024 •

edited

Loading

oliviertassinari Sep 17, 2024 •

edited

Loading