[material-ui] Remove UMD bundle #42172
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -4,19 +4,6 @@ | |||||
| <meta charset="utf-8" /> | ||||||
| <title>My page</title> | ||||||
| <meta name="viewport" content="initial-scale=1, width=device-width" /> | ||||||
| <!-- Fonts to support Material Design --> | ||||||
| <link rel="preconnect" href="https://fonts.googleapis.com" /> | ||||||
| <link rel="preconnect" href="https://fonts.gstatic.com" crossorigin /> | ||||||
|
|
@@ -26,91 +13,32 @@ | |||||
| /> | ||||||
| <!-- Icons to support Material Design --> | ||||||
| <link rel="stylesheet" href="https://fonts.googleapis.com/icon?family=Material+Icons" /> | ||||||
| <script type="importmap"> | ||||||
| { | ||||||
| "imports": { | ||||||
| "react": "https://esm.sh/react@18.3.0", | ||||||
| "react-dom": "https://esm.sh/react-dom@18.3.0", | ||||||
|
There was a problem hiding this comment. It feels like it would be best to use latest here, so we know a lot faster when it breaks with a latest version. Especially since we don't have a test for this anymore.
Suggested change
There was a problem hiding this comment. Another approach could be to version all dependencies with just their major version. To ensure compatibility across all dependencies while still enjoying the latest version. "react-dom": "https://esm.sh/react-dom@18",
"@mui/material": "https://esm.sh/@mui/material@5?external=react"There was a problem hiding this comment. @Janpot Yeah, we could maybe do this. However, I think this should be for all the examples, not just this one, so let's go with the latest to scope down the change. First, let's get consistency. Second, we will see if we should change the tradeoff for all examples. Setting the major version delays the report of bugs with the latest version and it's extra work. However, it also means that our examples are more likely to work at all times. I think that until we have more bandwidth than at the time of Me + Sebastian + Matt + Marija we shouldn't change the tradeoff. From what I have observed these last few years, we have been clearly slower, so I would delay this change. If you also consider that React, between two majors, almost never introduces breaking changes, they do deprecations first and that we are trending to do the same thing between MUI projects, then it might be fine. After a major, examples would continue to work, people would get new warnings, and we will fix them. A breakage would signal something isn't right. A quick benchmark, Next.js is cherry-picking its examples version: https://github.com/vercel/next.js/blob/canary/examples/with-redis/package.json. |
||||||
| "react/jsx-runtime": "https://esm.sh/react@18.3.0/jsx-runtime", | ||||||
| "@mui/material": "https://esm.sh/@mui/material?external=react" | ||||||
|
There was a problem hiding this comment. Oh, interesting approach, makes sense to be able to use MUI X without duplication. |
||||||
| } | ||||||
| } | ||||||
| </script> | ||||||
| </head> | ||||||
| <body> | ||||||
| <div id="root"></div> | ||||||
| <!-- Babel --> | ||||||
| <script src="https://unpkg.com/@babel/standalone/babel.min.js"></script> | ||||||
| <!-- Usage --> | ||||||
| <script type="text/babel" data-type="module"> | ||||||
| import * as React from 'react'; | ||||||
| import { createRoot } from 'react-dom'; | ||||||
| import { Button } from '@mui/material'; | ||||||
|
|
||||||
| function App() { | ||||||
| return <Button variant="contained">Button</Button>; | ||||||
| } | ||||||
|
|
||||||
|
There was a problem hiding this comment. We need CssBaseline for all examples, it's part of the recommanded approach to get started that we have in the docs. So I think it's a must add back. Same for the rest, I think that we should match the other examples. |
||||||
| createRoot(document.getElementById('root')).render(<App />); | ||||||
| </script> | ||||||
| </body> | ||||||
| </html> | ||||||
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.
Let's add a marker so we don't forget to switch back when master becomes default branch again
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.
Yeah, and I would add this in
material-ui/docs/src/modules/components/MaterialUIExampleCollection.js
Line 16 in 9b2e523