[core] Add createMuiStrictModeTheme #20523
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
testing if unused imports are removed from the bundle
This reverts commit 0762f98.
|
@material-ui/core: parsed: +2.01% , gzip: +1.62% Details of bundle changes.Comparing: 29d9ace...f43ffe2 Details of page changes
|
| ...other | ||
| } = props; | ||
|
|
||
| return ( | ||
| <Fade in={open} timeout={transitionDuration} {...other}> | ||
| <TransitionComponent in={open} timeout={transitionDuration} {...other}> |
There was a problem hiding this comment.
Oops, I had forgotten about this one in #17047 (comment).
|
Bundle size looks good:
|
| @@ -74,7 +76,7 @@ const Zoom = React.forwardRef(function Zoom(props, ref) { | |||
| }; | |||
|
|
|||
| return ( | |||
| <Transition | |||
| <TransitionComponent | |||
There was a problem hiding this comment.
Should we call the root component prop Component to match the other non transition component convention?
There was a problem hiding this comment.
Not a fan of the generic term if the component has a specific task.
There was a problem hiding this comment.
I wonder. It could feel surprising 🤔.
There was a problem hiding this comment.
That the component responsible for Transition is named TransitionComponent over Component? Is this seriously a concern?
There was a problem hiding this comment.
Would it be like we change the convention for the style rule, naming the root class name .transition over .root?
There was a problem hiding this comment.
What do you think of the idea to introduce a Components?: { Root: React.ComponentType, … } prop for v5 and empowering deep component customizations with styled component?
Similarly to what we do for the class names:
classNameprop at the rootclasses.rootobject at the rootclasses.xxxobject for the nested elements
we could have:
Componentprop at the rootComponents.Rootobject at the rootComponents.xxxobject for the nested components
NMinhNguyen
reviewed
Apr 13, 2020
oliviertassinari
added
core
oliviertassinari
approved these changes
Apr 13, 2020
|
@oliviertassinari @eps1lon |
This was referenced Apr 20, 2020
|
Generally these issues are indicative of a over-bundling. |
|
Anyway it forces bundler to process two versions of the same package. |
What is the issue with that? A bundler should be able to do that. We have SemVer for that. Seems like you don't care about versioning packages. Maybe some bundlers can be configured to deduplicate every package to the highest version. |
|
Nevermind. I missed you already published new version. |
This was referenced Apr 29, 2020
|
Hey thank you all for this, much relief from my console warnings. May we get typescript types for this please? I noticed the Transitions all have types (ex. unstable_StrictModeTransition..) but not for createMuiStrictModeTheme. for now though will just be doing |
PRs welcome as usual. We're not really pursuing this right now since there are other issues that aren't fixable without a breaking change. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Possible solution for #13394 (comment)
I realized that a gist would be too hard to integrate if you already have a custom theme. We want to remove as much friction as possible since testing Material-UI in StrictMode is very useful.
So we're going with a (hopefully) tree-shakeable solution that won't affect prod bundles:
docs bundle should not only show marginal size increases.
We use the
<Component ComponentImplementation={FancyImplementation} />pattern for dependency injection. Bundle aliases would not be sufficient. All the added props are undocumented for the time being. Otherwise we would need to prefix them withunstable_and I'm too lazy for that 😉 So they stay private for now. Only the added components are public (withunstable_) since they are required if you build your own components on top of our transitions (like we do with e.g. the Notifications component).Related to #13394