-
-
Notifications
You must be signed in to change notification settings - Fork 32.4k
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
[examples][base-ui] Add Base UI + Vite + Tailwind CSS example in TypeScript #37595
Conversation
Netlify deploy previewhttps://deploy-preview-37595--material-ui.netlify.app/ Bundle size report |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for looking into this.
I have left a few feedback, the main flow of thought is that I think this example and https://github.com/mui/material-ui/tree/master/examples/base-vite-tailwind should almost be identical (at the JS vs. TS difference)
Co-authored-by: Olivier Tassinari <[email protected]> Signed-off-by: dvkam <[email protected]>
Co-authored-by: Olivier Tassinari <[email protected]> Signed-off-by: dvkam <[email protected]>
I don't see a reason to remove the CRA example when they can both coexist peacefully here. If the |
Main reason for the migration and removal of CRA is this issue #37367 plus the fact that CRA is completetly removed from the official React documentation. See the comment of Dan Abramov here reactjs/react.dev#5487 (comment). Examples with outdated tech like CRA should not be used imo. Especially when its easy to replace with Vite. |
Co-authored-by: Olivier Tassinari <[email protected]> Signed-off-by: dvkam <[email protected]>
Co-authored-by: Olivier Tassinari <[email protected]> Signed-off-by: dvkam <[email protected]>
If you want to keep the CRA examples and just add same with Vite let me know and I will make the necessary changes. |
Thanks for your patience @dvkam ! Let's go ahead and move forward here by replacing this CRA example with Vite as you've proposed here. I don't think we need the CRA example in this case after all. |
Hey @samuelsycamore thanks for the update, I will make the needed changes asap. Do you want me to adapt this example so its the same like this one: https://github.com/mui/material-ui/blob/master/examples/base-vite-tailwind/README.md but with TS? Just need to know how you want me to proceed (see the two open threads above). |
Thanks for checking in @dvkam ! Yes, let's aim for consistency across these examples and follow the lead of the one you linked here. |
Since this PR has gone stale, I've made the requested changes to get the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good 👍
With this pull request CRA is replaced with Vite in the base-cra-tailwind-ts example. Adresses #37367
Removed old unnecessary files that were generated by CRA.
Additionally added a components folder and moved Button, Slider and Player components into it.
Preview: https://github.com/dvkam/material-ui/tree/base-vite-tailwind-ts/examples