-
-
Notifications
You must be signed in to change notification settings - Fork 32.6k
[code-infra] Remove modern bundles #45808
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
Conversation
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.
minor copy edits otherwise LGTM!
Co-authored-by: Sam Sycamore <71297412+mapache-salvaje@users.noreply.github.com> Signed-off-by: Jan Potoms <2109932+Janpot@users.noreply.github.com>
Co-authored-by: Sam Sycamore <71297412+mapache-salvaje@users.noreply.github.com> Signed-off-by: Jan Potoms <2109932+Janpot@users.noreply.github.com>
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.
Isn't this breaking? Seems odd that we are adding it now 🤔
Our current recommendations aren't working. And even if they were working it would be non-breaking as well, as it would just fall back gracefully to the esm bundle. It seems more appropriate to just remove it as the modern bundles don't really contain a significant improvement to bundle size. But yes, ideally, we should've just done this as part of v7. |
I'd say that this is slightly breaking, since people will need to adjust their setup in case they were explicitly using the aliased |
Yes, but they already have to do that as part of v7. (and X v8 too btw) |
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.
👍
Closes #45780
Fixes mui/mui-public#302
To Do: