Skip to content
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

[core] Upgrade Toolpad to React 19 #4488

Open
wants to merge 27 commits into
base: master
Choose a base branch
from

Conversation

bharatkashyap
Copy link
Member

@bharatkashyap bharatkashyap commented Nov 28, 2024

  • Fixes Update @toolpad/core to React 19 #4499
  • Update react, react-dom, @types/react and @types/react-dom to ^19
  • Update all packages across monorepo (including docs and studio*)
  • Run npx codemod@latest types-react-codemod preset-19
  • Run npx codemod@latest react/19/migration-recipe (excluding react/prop-types-typescript )

@bharatkashyap bharatkashyap added the core Infrastructure work going on behind the scenes label Nov 28, 2024
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 28, 2024
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Dec 8, 2024
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Dec 11, 2024
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Dec 11, 2024
@bharatkashyap bharatkashyap changed the title [core] Upgrade to react 19 [core] Upgrade Toolpad to React 19 Dec 11, 2024
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Dec 11, 2024
@bharatkashyap
Copy link
Member Author

bharatkashyap commented Dec 11, 2024

Upgrading all packages allowed most of the breaking types checks to pass, but there are some strange integration test failures.

1. "can move elements in page"

Fails locally as well, moving text fields causes the value to disappear - not sure what's causing this.

@apedroferreira any ideas?

Before After
normal-swap.mov
r-19-swap.mov

2. "data providers" and "data providers crud"

pass locally, fail in CI

@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Dec 11, 2024
@apedroferreira
Copy link
Member

Upgrading all packages allowed most of the breaking types checks to pass, but there is a strange integration test failure, because of a bug I can't really pinpoint the cause of.

Moving text fields causes the value to disappear - not sure what's causing this.

@apedroferreira any ideas?

Before After
normal-swap.mov
r-19-swap.mov

Not sure at first glance... probably something to do with the logic we have for the controlled state and possibly bindings, probably will take some deeper looking into. I could also try once I have the time as I have also touched that logic some time ago.

@apedroferreira
Copy link
Member

apedroferreira commented Dec 12, 2024

I found the fix for the text fields, looks like I can't push it to your repo directly?
In the studio components, in Form line 61, change the effect to:

// Reset form in effect as suggested in https://react-hook-form.com/api/useform/reset/
  React.useEffect(() => {
    if (isSubmitSuccessful) {
      form.reset();
    }
  }, [form, isSubmitSuccessful]);

It looks like this effect recommended by react-hook-form runs more times than intended on React 19 and resets the fields.
I think this fix should be fine?

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Dec 16, 2024
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Dec 17, 2024
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Dec 18, 2024
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Dec 19, 2024
@bharatkashyap
Copy link
Member Author

@apedroferreira @Janpot The two data grid related tests which are exceeding timeouts is most likely due to mui/mui-x#15770 I've skipped those two pending the fix of this issue to unblock the upgrade, wdyt?

The tests pass if I increase the timeout locally to a very high number:
Screenshot 2024-12-18 at 3 04 02 PM

I can not reproduce the Argos change reported as well:
Screenshot 2024-12-19 at 3 04 49 PM Given that I've approved the change to get this PR to a reviewable stage.

@bharatkashyap bharatkashyap marked this pull request as ready for review December 19, 2024 09:39
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Dec 23, 2024
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Dec 25, 2024
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Dec 25, 2024
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Dec 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Infrastructure work going on behind the scenes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update @toolpad/core to React 19
2 participants