Repository: microsoft/griffel
Pull request #248 (2022-10-05 12:26:39)

fix: handle empty strings in mergeClasses()
A fix for a problem that I discovered in https://github.com/microsoft/fluentui/pull/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](https://user-images.githubusercontent.com/14183168/194059678-f8968b20-f7e1-4107-b2dc-dae6bbc9331f.png)

It's happening because we handle empty strings as valid classes and add spacing between them, i.e.:

```ts
mergeClasses('', '', '') // "  " (a string from two spaces)
```

This PR fixes that problem.

github-actions[bot]: ## 📊 Bundle size report

| Package & Exports                                                                                                            |    Baseline (minified/GZIP) |                          PR |                                                                                                                                                                                               Change |
| :--------------------------------------------------------------------------------------------------------------------------- | --------------------------: | --------------------------: | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------: |
| <samp>core</samp> <br /> <abbr title='bundle-size/makeStyles.fixture.js'>makeStyles + mergeClasses (build time)</abbr>       |     `1.834 kB`<br />`872 B` |     `1.842 kB`<br />`877 B` | `8 B` <img aria-hidden="true" src="https://microsoft.github.io/monosize/images/increase.png" /><br />`5 B` <img aria-hidden="true" src="https://microsoft.github.io/monosize/images/increase.png" /> |
| <samp>core</samp> <br /> <abbr title='bundle-size/makeStylesRuntime.fixture.js'>makeStyles + mergeClasses (runtime)</abbr>   |  `20.781 kB`<br />`7.72 kB` | `20.789 kB`<br />`7.724 kB` | `8 B` <img aria-hidden="true" src="https://microsoft.github.io/monosize/images/increase.png" /><br />`4 B` <img aria-hidden="true" src="https://microsoft.github.io/monosize/images/increase.png" /> |
| <samp>react</samp> <br /> <abbr title='bundle-size/__css.fixture.js'>\_\_css + mergeClasses (build time)</abbr>              |     `1.871 kB`<br />`869 B` |     `1.879 kB`<br />`874 B` | `8 B` <img aria-hidden="true" src="https://microsoft.github.io/monosize/images/increase.png" /><br />`5 B` <img aria-hidden="true" src="https://microsoft.github.io/monosize/images/increase.png" /> |
| <samp>react</samp> <br /> <abbr title='bundle-size/__styles.fixture.js'>\_\_styles + mergeClasses (build time)</abbr>        |  `3.573 kB`<br />`1.609 kB` |  `3.581 kB`<br />`1.614 kB` | `8 B` <img aria-hidden="true" src="https://microsoft.github.io/monosize/images/increase.png" /><br />`5 B` <img aria-hidden="true" src="https://microsoft.github.io/monosize/images/increase.png" /> |
| <samp>react</samp> <br /> <abbr title='bundle-size/makeStyles-runtime.fixture.js'>makeStyles + mergeClasses (runtime)</abbr> | `22.546 kB`<br />`8.432 kB` | `22.554 kB`<br />`8.436 kB` | `8 B` <img aria-hidden="true" src="https://microsoft.github.io/monosize/images/increase.png" /><br />`4 B` <img aria-hidden="true" src="https://microsoft.github.io/monosize/images/increase.png" /> |

<details>
<summary>Unchanged fixtures</summary>

| Package & Exports                                                                                                         |        Size (minified/GZIP) |
| ------------------------------------------------------------------------------------------------------------------------- | --------------------------: |
| <samp>core</samp> <br /> <abbr title='bundle-size/makeResetStyles-runtime.fixture.js'>makeResetStyles (runtime)</abbr>    | `16.192 kB`<br />`6.041 kB` |
| <samp>react</samp> <br /> <abbr title='bundle-size/makeResetStyles-runtime.fixture.js'>makeResetStyles (runtime)</abbr>   | `17.957 kB`<br />`6.777 kB` |
| <samp>react</samp> <br /> <abbr title='bundle-size/makeStaticStyles-runtime.fixture.js'>makeStaticStyles (runtime)</abbr> |  `9.443 kB`<br />`4.068 kB` |

</details>
<sub>🤖 This report was generated against <a href='https://github.com/microsoft/griffel/commit/d0511fc7844a552954f59c2f55f94e1ca26b013b'>d0511fc7844a552954f59c2f55f94e1ca26b013b</a></sub>

<!-- Sticky Pull Request Comment -->

Patch:
diff --git a/change/@griffel-core-69be5d24-4050-4443-a10d-75722b29e744.json b/change/@griffel-core-69be5d24-4050-4443-a10d-75722b29e744.json
new file mode 100644
index 0000000..38738b4
--- /dev/null
+++ b/change/@griffel-core-69be5d24-4050-4443-a10d-75722b29e744.json
@@ -0,0 +1,7 @@
+{
+  "type": "patch",
+  "comment": "fix: handle empty strings in mergeClasses()",
+  "packageName": "@griffel/core",
+  "email": "olfedias@microsoft.com",
+  "dependentChangeType": "patch"
+}
diff --git a/packages/core/src/mergeClasses.ts b/packages/core/src/mergeClasses.ts
index 3cf7a6e..d780977 100644
--- a/packages/core/src/mergeClasses.ts
+++ b/packages/core/src/mergeClasses.ts
@@ -46,7 +46,7 @@ export function mergeClasses(): string {
   for (let i = 0; i < arguments.length; i++) {
     const className = arguments[i];
 
-    if (typeof className === 'string') {
+    if (typeof className === 'string' && className !== '') {
       // All classes generated by `makeStyles()` are prefixed by a sequence hash, this allows to identify class sets
       // without parsing each className in a string
       const sequenceIndex = className.indexOf(SEQUENCE_PREFIX);


Test patch:
diff --git a/packages/core/src/mergeClasses.test.ts b/packages/core/src/mergeClasses.test.ts
index 876d29c..47d5aff 100644
--- a/packages/core/src/mergeClasses.test.ts
+++ b/packages/core/src/mergeClasses.test.ts
@@ -52,6 +52,11 @@ describe('mergeClasses', () => {
     expect(mergeClasses(undefined, false)).toBe('');
   });
 
+  it('handles empty classes', () => {
+    expect(mergeClasses('ui-button', '', 'ui-toogle-button')).toBe('ui-button ui-toogle-button');
+    expect(mergeClasses('', '')).toBe('');
+  });
+
   it('performs deduplication for multiple arguments', () => {
     const classes = makeStyles({
       block: { display: 'block' },