-
Notifications
You must be signed in to change notification settings - Fork 2.9k
RFC: Unstyled/headless components #35464
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
base: master
Are you sure you want to change the base?
Conversation
|
Pull request demo site: URL |
f81e722 to
f2b8caf
Compare
f2b8caf to
2f51180
Compare
📊 Bundle size report✅ No changes found |
| - ✅ Component files unchanged (still supports `useCustomStyleHook_unstable`) | ||
| - ✅ **~25% JS bundle size reduction** (tested) by excluding Griffel runtime | ||
|
|
||
| **Note:** To completely eliminate Griffel from an application, unstyled variants are needed for **all components that use Griffel**, including infrastructure components like `FluentProvider`. This ensures no Griffel runtime is bundled. |
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.
I think we already have this: https://github.com/microsoft/fluentui-contrib/tree/main/packages/react-themeless-provider
|
|
||
| export const useButtonStyles_unstable = (state: ButtonState) => { | ||
| // Only apply base class names, no styles | ||
| state.root.className = mergeClasses(buttonClassNames.root, state.root.className); |
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.
It seems like something that is missing here is that the style hooks are implicitly adding "state classes" when they apply styles based on component state (example).
I think we'll need to introduce well-known classes for these so users bringing their own styles can correctly respond to state changes.
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.
Just had another idea — not directly tied to this proposal, but it could help with the whole well-known classes issue. What if we exposed component state as data-attributes on DOM elements using the useComponentState hook? For example, we could set data-appearance="outline|underline|etc" and data-size="small|large|etc". That way, we could easily use these in styles, tests, and more. It also can eliminate some JS (runtime classes mapping)
Here are a couple of examples from other DS/UI libraries doing something similar:
|
|
||
| Unstyled variants are opt-in via bundler extension resolution (similar to [raw modules](https://storybooks.fluentui.dev/react/?path=/docs/concepts-developer-unprocessed-styles--docs#how-to-use-raw-modules), ensuring zero breaking changes. | ||
|
|
||
| **Performance Impact:** Internal testing shows **~25% JavaScript bundle size reduction** when using unstyled variants, as Griffel runtime and style implementations are excluded from the bundle. |
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.
What will be an increase once you will have actual CSS that matches what we have currently? Nobody is going to use components without any styles.
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.
It mainly depends on how much styles consumers want to use. Even if the number (25%) isn't completely accurate, users will still need to pay for any default styles they don't intend to use.
I tried to check the increase with the actual CSS version this by switching the style hook to CSS modules, but our tooling (monosize) only takes into account JS, not CSS. It's possible I missed a configuration or setting.
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.
I tried to check the increase with the actual CSS version this by switching the style hook to CSS modules, but our tooling (monosize) only takes into account JS, not CSS. It's possible I missed a configuration or setting.
It indeed won't measure CSS by default.
IMO it's crucial to provide measurements to compare apples to apples. As currently, "25% reduction" is a false message.
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.
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.
I got what you mean, however you won't use solely JavaScript, correct? 🐱 Otherwise cards would look like that:
P.S. In the Griffel scenario we will need have part of Griffel runtime + mappings for merging which contributes to JS size.
I've ran "Griffel + AOT + CSS extraction" scenario for Card.fixture.js:
- JS 82.432 kB / 25.161 kB
- CSS 13K / 2.52 kB
- Total: 95.4 K / 27.6 K
Can you please provide the same for "Without Griffel - "unstyled" + plain CSS"? (considering that CSS file for Card should match current styles)
From the RFC, I noticed that the initial plan is to update 10 components, so it would good to have the same for all them to be realistic.
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.
we should probably provide a realistic sample implementation with different styles and use that for comparison. and make sure we measure it
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.
we should probably provide a realistic sample implementation with different styles and use that for comparison. and make sure we measure it
Totally agree! Just to clarify, reducing bundle size wasn’t our only reason for this. The main goal is to support partners who have their own unique UI needs - like, their designs don’t use Fluent 2, they have specific tech/performance requirements, and they aren’t using Griffel or default Fluent styles. They’d rather not include stuff they don’t actually need.
| **Pros:** Single source of truth, automatic | ||
| **Cons:** Complex build config, harder to debug | ||
|
|
||
| ## Usage Examples |
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.
can you pls also include examples for styles applied conditionally based on state and/or props? for example how to style a toggle button with different background based on toggle state or how to style a secondary button?
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.
here is an example for Button component: https://github.com/microsoft/fluentui/pull/35491/files#diff-87994f1431cce0df0f1b30e5980466aae99f5bab0c73f40d39266af17e977caa
I'll add it to the doc



Previous Behavior
New Behavior
Related Issue(s)