Skip to content

Conversation

@layershifter
Copy link
Member

A fix for a problem that I discovered in microsoft/fluentui#25075.

After #207 we don't emit useless sequence hashes that should improve performance of classes merging in such cases. The accidental problem that came with it:

image

It's happening because we handle empty strings as valid classes and add spacing between them, i.e.:

mergeClasses('', '', '') // "  " (a string from two spaces)

This PR fixes that problem.

@github-actions
Copy link

github-actions bot commented Oct 5, 2022

📊 Bundle size report

Package & Exports Baseline (minified/GZIP) PR Change
core
makeStyles + mergeClasses (build time)
1.834 kB
872 B
1.842 kB
877 B
8 B
5 B
core
makeStyles + mergeClasses (runtime)
20.781 kB
7.72 kB
20.789 kB
7.724 kB
8 B
4 B
react
__css + mergeClasses (build time)
1.871 kB
869 B
1.879 kB
874 B
8 B
5 B
react
__styles + mergeClasses (build time)
3.573 kB
1.609 kB
3.581 kB
1.614 kB
8 B
5 B
react
makeStyles + mergeClasses (runtime)
22.546 kB
8.432 kB
22.554 kB
8.436 kB
8 B
4 B
Unchanged fixtures
Package & Exports Size (minified/GZIP)
core
makeResetStyles (runtime)
16.192 kB
6.041 kB
react
makeResetStyles (runtime)
17.957 kB
6.777 kB
react
makeStaticStyles (runtime)
9.443 kB
4.068 kB
🤖 This report was generated against d0511fc7844a552954f59c2f55f94e1ca26b013b

@layershifter layershifter marked this pull request as ready for review October 5, 2022 12:41
@layershifter layershifter requested a review from a team as a code owner October 5, 2022 12:41
@layershifter layershifter merged commit 9173966 into microsoft:main Oct 5, 2022
@layershifter layershifter deleted the fix/merge-classes-empty branch October 5, 2022 14:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants