[docs] Improve export to CodeSandbox #22346
Conversation
|
No bundle size changes comparing 1ece36d...7a08768 |
| ); | ||
| const packagesWithDTPackage = Object.keys(deps) | ||
| .filter((name) => packagesWithBundledTypes.indexOf(name) === -1) | ||
| // All the Material-UI packages come with bundled types |
There was a problem hiding this comment.
No they don't. The existing logic is sound. Why not just add the new package to the existing logic?
There was a problem hiding this comment.
No they don't.
Do you have an example?
Why not just add the new package to the existing logic?
Assuming all the Material-UI packages should come with bundled types, why spend mental energy on maintaining such list?
Note that codesandbox is silent when a wrong package is included. For instance, take this demo https://material-ui.com/system/basics/#demo, open it to codesandbox, and look at the installed dependencies.
There was a problem hiding this comment.
For instance, take this demo material-ui.com/system/basics/#demo, open it to codesandbox, and look at the installed dependencies.
Maybe tell me what you think is wrong?
There was a problem hiding this comment.
@material-ui/utils
Cool, I will fix that so we can merge this pull request as is. Happy to have surfaced this issue.
Maybe tell me what you think is wrong?
There was a problem hiding this comment.
The assumption is still wrong. I don't understand why we can't keep the correct assumption and extend the packages that do ship types? Adding types to /utils is not really worth it at this this time.
There was a problem hiding this comment.
The current approach forces to keep track of a list of packages that support TypeScript. Assuming we will add 10 new packages, I don't see the value of having to think about it when we can go all-in on TypeScript. The value of TypeScript support of /utils is so material-ui-x can use the package more efficiently. e.g. https://github.com/mui-org/material-ui-x/blob/fc207098f9c919faa88ec5cb827a37f5d7b87e9b/packages/grid/x-grid/src/XGrid.tsx#L3
There was a problem hiding this comment.
So in both cases, it doesn't work (@types/material-uiSystem doesn't exist and will likely never), we might as well well pick the approach that has a chance to eventually work.
oliviertassinari
dismissed
eps1lon’s stale review
I believe the concern was answer with 1. #22346 (comment), 2. long term objective to go all-in on TypeScript, 3. #22367
Working on this page: https://material-ui-x.netlify.app/components/data-grid/pagination/.