InputControl: Use less Emotion styled API #58948
Closed
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
mirka
force-pushed
the
try/input-control-perf
branch
from
2be26f7 to
03f52b0
Compare
|
Size Change: -341 B (0%) Total Size: 1.7 MB
ℹ️ View Unchanged
|
mirka
force-pushed
the
try/input-control-perf
branch
from
03f52b0 to
fb7130b
Compare
|
From my experimentation, it does seem like our use of Emotion is creating an unnecessary drag on component performance. It is harder to confirm with our "official" performance tests ( However, I am able to observe meaningful differences with the following test methods:
Observing with these two methods, it seems like minimizing our use of Emotion recovers most of the performance gap between TextControl and InputControl. Although InputControl also involves a lot of state reducer logic, This is kind of good, because we have no reason to be using a runtime library like Emotion anymore and we've already been talking about migrating to a build-time solution. Optimizing Emotion performanceAlthough I think we should just migrate away from Emotion rather than rewrite our Emotion code, I'll note here for completeness that there seems to be a usage pattern that is particularly bad for performance. A big improvement comes from removing dynamic prop-based // Bad
const StyledDiv = styled.div`
color: ${ props => props.color };
`;
<StyledDiv />
// Immediate improvement, even though it's still dynamic
const classes = cx( styles.dynamicColor( props.color ) );
<div className={ classes } />Next stepsI will start investigating approaches to migrating away from Emotion. My first choice would be CSS Modules since their standard syntax feels like a safer bet in the long run. But I'd also like to experiment with Linaria because it's probably easier to integrate into our current build process, and would require much less code rewriting effort compared to CSS Modules. It could be a relatively quick win. |
I agree with this. In a few places we noticed that Emotion was causing unnecessary performance impact. cc @ellatrix |
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.
Follow-up to #56524 (comment)