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

[examples][base-ui] Add Base UI + Vite + Tailwind CSS example in TypeScript #37595

Merged
merged 17 commits into from
Sep 12, 2023

Conversation

dvkam
Copy link
Contributor

@dvkam dvkam commented Jun 14, 2023

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

@mui-bot
Copy link

mui-bot commented Jun 14, 2023

Netlify deploy preview

https://deploy-preview-37595--material-ui.netlify.app/

Bundle size report

No bundle size changes (Toolpad)
No bundle size changes

Generated by 🚫 dangerJS against 507f28d

@oliviertassinari oliviertassinari added the examples Relating to /examples label Jun 14, 2023
Copy link
Member

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

examples/base-vite-react-tailwind-ts/README.md Outdated Show resolved Hide resolved
examples/base-vite-react-tailwind-ts/package.json Outdated Show resolved Hide resolved
examples/base-vite-react-tailwind-ts/src/main.tsx Outdated Show resolved Hide resolved
examples/base-vite-react-tailwind-ts/src/App.tsx Outdated Show resolved Hide resolved
examples/base-vite-react-tailwind-ts/README.md Outdated Show resolved Hide resolved
dvkam and others added 2 commits June 14, 2023 19:55
@samuelsycamore
Copy link
Contributor

I don't see a reason to remove the CRA example when they can both coexist peacefully here. If the examples folder is getting too cluttered, we can consider other ways of organizing it.

@dvkam
Copy link
Contributor Author

dvkam commented Jun 14, 2023

I don't see a reason to remove the CRA example when they can both coexist peacefully here. If the examples folder is getting too cluttered, we can consider other ways of organizing it.

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.

dvkam and others added 2 commits June 14, 2023 20:02
@dvkam
Copy link
Contributor Author

dvkam commented Jun 18, 2023

If you want to keep the CRA examples and just add same with Vite let me know and I will make the necessary changes.

@samuelsycamore
Copy link
Contributor

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.

@dvkam
Copy link
Contributor Author

dvkam commented Jun 27, 2023

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).

@samuelsycamore
Copy link
Contributor

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.

@samuelsycamore samuelsycamore changed the title [examples] base-cra-tailwind-ts migration to Vite [examples][base-ui] Add Base UI + Vite + Tailwind CSS example in TypeScript Sep 11, 2023
@samuelsycamore
Copy link
Contributor

samuelsycamore commented Sep 11, 2023

Since this PR has gone stale, I've made the requested changes to get the base-ui-vite-tailwind-ts example merged (part of #36193). I've also added a couple small fixes to base-ui-vite-tailwind.

Copy link
Member

@oliviertassinari oliviertassinari left a comment

Choose a reason for hiding this comment

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

Looks good 👍

@samuelsycamore samuelsycamore merged commit e269c38 into mui:master Sep 12, 2023
5 checks passed
@dvkam dvkam deleted the base-vite-tailwind-ts branch November 28, 2023 16:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
examples Relating to /examples
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants