Skip to content

Chakra UI v2 to v3 migration #4566

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

Merged
merged 25 commits into from
Apr 28, 2025

Conversation

antpaw
Copy link
Contributor

@antpaw antpaw commented Apr 14, 2025

Reasons for making this change

fixes #4389

Update Chakra UI v2 to v3 based on the work of @MilanKrupa chakra-ui-v2-to-v3-migration

I can not run tests as the npm test does not succeed even on main. I seek feedback for this PR

Checklist

  • I'm updating documentation
  • I'm adding or updating code
    • I've added and/or updated tests. I've run npx nx run-many --target=build --exclude=@rjsf/docs && npm run test:update to update snapshots, if needed.
    • I've updated docs if needed
    • I've updated the changelog with a description of the PR
  • I'm adding a new feature
    • I've updated the playground with an example use of the feature

List of my commits:

  • fix chakra RadioWidget value change

  • fix chakra CheckboxesWidget value assign
    I think this is the issue mentioned previously, it was bugged because there was a <Field> wrapper around the form that messes up the ids

  • add chakra Fieldset to FieldTemplate
    Instead of removing the field i've added a fieldset

  • fix chakra SelectWidget content position via portal

  • add chakra NativeSelectWidget
    Users can opt out of the custom select by using this widget

  • update playground chakra-ui version

  • remove @chakra-ui/icon in favor of lucide-react
    @chakra-ui/icon is deprecated

@antpaw antpaw mentioned this pull request Apr 14, 2025
1 task
@antpaw antpaw force-pushed the chakra-ui-v2-to-v3-migration branch from c9e88c6 to 212cf02 Compare April 14, 2025 08:27
@heath-freenome
Copy link
Member

@antpaw can you rebase this to the rjsf-v6 branch?

@antpaw antpaw force-pushed the chakra-ui-v2-to-v3-migration branch from 212cf02 to 2058cf6 Compare April 15, 2025 06:53
@antpaw antpaw changed the base branch from main to rjsf-v6 April 15, 2025 06:54
@antpaw
Copy link
Contributor Author

antpaw commented Apr 15, 2025

@antpaw can you rebase this to the rjsf-v6 branch?

Done, I had to merge the newest main to rjsf-v6 branch first. I've also changed the base of this pr to rjsf-v6

@heath-freenome
Copy link
Member

heath-freenome commented Apr 17, 2025

@antpaw hmmm, I wonder why other themes are failing with your changes. Can you try starting with the rjsf-v6 branch and adding your changes on top of it instead of merging main?

@heath-freenome
Copy link
Member

@antpaw I just merged main into rjsf-v6

@antpaw
Copy link
Contributor Author

antpaw commented Apr 18, 2025

@heath-freenome Thank you for the feedback, I will look into this on Tuesday

@antpaw antpaw force-pushed the chakra-ui-v2-to-v3-migration branch from 2058cf6 to 65d7b7e Compare April 22, 2025 07:46
@antpaw
Copy link
Contributor Author

antpaw commented Apr 22, 2025

@heath-freenome Please review my changes based on your feedback, thanks.

@antpaw antpaw force-pushed the chakra-ui-v2-to-v3-migration branch 2 times, most recently from f33912d to ee5674a Compare April 22, 2025 08:40
@antpaw antpaw requested a review from heath-freenome April 22, 2025 08:43
Copy link
Member

@heath-freenome heath-freenome left a comment

Choose a reason for hiding this comment

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

Have you updated the test snapshots yet via npm run test:update?

<ChakraProvider resetCSS={false}>
<CSSReset />
<ChakraProvider value={defaultSystem}>
{/* <CSSReset /> TODO: figrue out styling issues */}
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure either. What happens if you uncomment it? Do the styles in the playground look correct without it?

@heath-freenome
Copy link
Member

heath-freenome commented Apr 23, 2025

@antpaw it looks like the shadcn theme is failing. I fixed this previously by first checking out the rjsv-v6 branch, doing npm run refresh-node-modules and then verifying it by doing npm run build. Then when you are done with that, check out your branch and delete package-lock.json from temporarily. Then do npm install to update the package-lock.json with only your changes to the package.json. Then run npm run build to ensure that everything builds properly. Then add back in the package-lock.json to your commits.

@heath-freenome
Copy link
Member

@antpaw I am going to be focused on getting RJSF v6 ready for the beta release next week. I'd love to get these changes in by Monday. Feel free to reach out to me on discord. I'll even make some time to pair with you in the RJSF discord channel if you need it.

@antpaw antpaw force-pushed the chakra-ui-v2-to-v3-migration branch 2 times, most recently from 23da8ae to e0bd2ef Compare April 24, 2025 07:27
@antpaw
Copy link
Contributor Author

antpaw commented Apr 24, 2025

@antpaw it looks like the shadcn theme is failing. I fixed this previously by first checking out the rjsv-v6 branch, doing npm run refresh-node-modules and then verifying it by doing npm run build. Then when you are done with that, check out your branch and delete package-lock.json from temporarily. Then do npm install to update the package-lock.json with only your changes to the package.json. Then run npm run build to ensure that everything builds properly. Then add back in the package-lock.json to your commits.

@heath-freenome Thank you, I've made the CI build the project by resetting the package-lock and only applying the actual changes to it.
Now the tests are not passing because of this "native" function:
https://github.com/chakra-ui/chakra-ui/blob/main/packages/react/src/styled-system/token-middleware.ts#L24
I think something is wrong with the test setup?

@antpaw
Copy link
Contributor Author

antpaw commented Apr 24, 2025

I've added the hack suggested here jsdom/jsdom#3363

global.structuredClone = (val) => {
  return JSON.parse(JSON.stringify(val));
};

and now it seams like the tests are not passing because <Form /> is not wrapped into <ChakraProvider />
I guess some magic needs to be done with __createChakraFrameProvider

@heath-freenome
Copy link
Member

heath-freenome commented Apr 24, 2025

I've added the hack suggested here jsdom/jsdom#3363

global.structuredClone = (val) => {
  return JSON.parse(JSON.stringify(val));
};

and now it seams like the tests are not passing because <Form /> is not wrapped into <ChakraProvider /> I guess some magic needs to be done with __createChakraFrameProvider

What if you were to create a Test wrapper component that is a wrapper around the chakra Form and it did something like this:

import { FormProps } from `@rjsf/core`;
import { ChakraProvider } from `@chakra/react`; // or where ever it needs to be imported

import Form from '../src';

export default WrappedForm(props: FormProps) {
  return (
    <ChakraProvider>
     <Form {...props} />
    </ChakraProvider>
  );
}

And then updated Xxx.test.tsx files to pass in the WrappedForm to the test rather than Form?

@antpaw
Copy link
Contributor Author

antpaw commented Apr 24, 2025

What if you were to create a Test wrapper component that is a wrapper around the chakra Form and it did something like this:

import { FormProps } from `@rjsf/core`;
import { ChakraProvider } from `@chakra/react`; // or where ever it needs to be imported

import Form from '../src';

export default WrappedForm(props: FormProps) {
  return (
    <ChakraProvider>
     <Form {...props} />
    </ChakraProvider>
  );
}

And then updated Xxx.test.tsx files to pass in the WrappedForm to the test rather than Form?

@heath-freenome yes, that works! Now I'm able to use the tests to develop

Snapshot Summary
 › 54 snapshots updated from 3 test suites.

Test Suites: 3 failed, 1 passed, 4 total
Tests:       29 failed, 54 passed, 83 total
Snapshots:   54 updated, 54 total
Time:        3.753 s

@antpaw antpaw force-pushed the chakra-ui-v2-to-v3-migration branch from d968a5d to e7fbad5 Compare April 24, 2025 22:14
@antpaw
Copy link
Contributor Author

antpaw commented Apr 24, 2025

@heath-freenome tests pass locally now, I don't know why the snapshot does not match on the CI

@heath-freenome
Copy link
Member

heath-freenome commented Apr 24, 2025

@heath-freenome tests pass locally now, I don't know why the snapshot does not match on the CI

@antpaw If you run them multiple times locally do they pass everytime? If not, then maybe some sort of random seed is affecting the generated CSS-in-JS classNames? Also, I just merged some changes to the rjsf-v6 branch that did change how some things were rendered in the gridForm maybe you need to rebase your MR on top of it before running npm run test:update?

@antpaw
Copy link
Contributor Author

antpaw commented Apr 25, 2025

@heath-freenome tests pass locally now, I don't know why the snapshot does not match on the CI

@antpaw If you run them multiple times locally do they pass everytime? If not, then maybe some sort of random seed is affecting the generated CSS-in-JS classNames? Also, I just merged some changes to the rjsf-v6 branch that did change how some things were rendered in the gridForm maybe you need to rebase your MR on top of it before running npm run test:update?

@heath-freenome I've rebased the branch and tested locally a couple of time, and I can not make the test fail locally

@heath-freenome
Copy link
Member

heath-freenome commented Apr 25, 2025

@antpaw I just merged another PR that conflicts with yours (sorry). If you can resolve the conflicts, I am tempted to merge and then figure out what is going on myself. It seems puzzling that your local stuff works. Do run another npm run test:update before you push though, just in case.

@heath-freenome
Copy link
Member

heath-freenome commented Apr 25, 2025

@antpaw I know why your snapshots are failing, you need to locally run npm run build again after a rebase so that you pick up the changes made in core. Once you've done that, then run npm run test:update which should fix your snapshots. I figured this out based on the fact that the only tests that are failing are the grid ones that I updated with my previous PR.

@antpaw antpaw force-pushed the chakra-ui-v2-to-v3-migration branch from bb23e14 to dc728a0 Compare April 26, 2025 08:51
@antpaw
Copy link
Contributor Author

antpaw commented Apr 26, 2025

@antpaw I know why your snapshots are failing, you need to locally run npm run build again after a rebase so that you pick up the changes made in core. Once you've done that, then run npm run test:update which should fix your snapshots. I figured this out based on the fact that the only tests that are failing are the grid ones that I updated with my previous PR.

@heath-freenome I've rebased onto the newest rjsf-v6
I'm having issues running npm run build

➜  react-jsonschema-form git:(chakra-ui-v2-to-v3-migration) ✗ npm run build

> build
> nx run-many --target=build


 NX   (0 , minimatch_1.minimatch) is not a function
➜  chakra-ui git:(chakra-ui-v2-to-v3-migration) ✗ npm run build

> @rjsf/[email protected] build
> npm run build:ts && npm run build:cjs && npm run build:esm && npm run build:umd


> @rjsf/[email protected] build:ts
> tsc -b tsconfig.build.json && tsc-alias -p tsconfig.build.json

../core/src/components/fields/SchemaField.tsx:173:6 - error TS2786: 'FieldComponent' cannot be used as a JSX component.
 Its type 'Function' is not a valid JSX element type.

173     <FieldComponent
        ~~~~~~~~~~~~~~


Found 1 error.

npm ERR! Lifecycle script `build:ts` failed with error: 
npm ERR! Error: command failed 
npm ERR!   in workspace: @rjsf/[email protected] 
npm ERR!   at location: /Users/antpaw/Projects/HiveMQ/react-jsonschema-form/packages/chakra-ui 
npm ERR! Lifecycle script `build` failed with error: 
npm ERR! Error: command failed 
npm ERR!   in workspace: @rjsf/[email protected] 
npm ERR!   at location: /Users/antpaw/Projects/HiveMQ/react-jsonschema-form/packages/chakra-ui 

I can't make Chakra's Range Slider work, otherwise the playground looks good, it only fails where other themes also fail so I think it's not related to chakras upgrade.

Copy link
Member

@heath-freenome heath-freenome left a comment

Choose a reason for hiding this comment

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

@antpaw I am going to merge this and fix the tests in the build. Do you think, once I am done, that you can get Slider working?

@heath-freenome heath-freenome merged commit eb105ec into rjsf-team:rjsf-v6 Apr 28, 2025
2 of 4 checks passed
@antpaw
Copy link
Contributor Author

antpaw commented Apr 28, 2025

@antpaw I am going to merge this and fix the tests in the build. Do you think, once I am done, that you can get Slider working?

@heath-freenome Thank you! I've already tried a couple of attempts to solve the slide issue, with no luck. I will reach out the chakra ui folks, so that they can take a look at the playground issue, this merge helps a lot here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

update chakra to v3
3 participants