This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 511c32a:

Asset size changes

Size Auditor did not detect a change in bundle size for any component!

Baseline commit: 1e5b888607626cae4ff69b5b9055bf8b9537c45b (build)

📊 Bundle size report

Perf Analysis (@fluentui/react)

No significant results to display.

Perf Analysis (@fluentui/react-northstar)

IMO this RFC makes root and slots even more similar, which is compatible with the proposal from RFC #19068, the 2 biggest changes being:

1. making root available on props
2. creating the notion of primary slot which allows to decide which slot is the one that has it's properties flattened on components properties

I believe that this changes are valid and makes slots implementation even more generic and explicit.

@ecraig12345 is this ready for review or do you plan to update it?

@layershifter It's not ready for review. I need to update it but haven't gotten to do it yet.

Updated to promote "discarded option 3" and have two proposed solutions:

Option A: allow setting a different "primary" slot; explicitly expose root (formerly the proposed solution)

Allow setting a different "primary" slot where top-level props and ref are applied. Also explicitly expose the root slot in props.

Option B: Use input as root and add wrapper slot for root DOM element (formerly discarded option 3)

Define root as the native input element, and add a wrapper slot for the actual root DOM element. (Do not expose root in props.)

To make it clearer my main open question that would decide between A and C is:

Should we special-case className and style to always go on the root slot?

• If yes, then we need to go with option A, or otherwise there'd be no way to add className and style to the primary slot.
• If we change our decision about this later, it would be a breaking change for any control where root is not the primary slot, and would be difficult to detect these cases.

The reason we might want to do this is based on the assumption that users would expect styling to be applied to the entire control, rather than an inner element. For example, it could be confusing why <Checkbox style={{ alignSelf: 'center' }} /> doesn't work to align the control.

Both FluentUI v8 and Northstar v0 put className on the outermost element.

I took a look at a few libraries and made a table of their behaviors here: #18804 (comment)

Should we special-case className and style to always go on the root slot?

No, as this will produce strange behavior. For example, both id & className (and other attributes) can be used as CSS selectors. For example:

.foo#bar {
  color: red;
}

From JSX it's not clear if they will go to different elements:

<Input id="bar" className="foo" />

<--! 💥 CSS selector will not work -->
<div className="foo">
  <input id="bar" />
</div>

I took a look at a few libraries and made a table of their behaviors here: #18804 (comment)

Honestly, there is not enough data to make a decision based on it: once we will remove Fluent UI flavors from the table, it's 50/50.

Should we special-case className and style to always go on the root slot?

No.
From my experience there is no correct way how to do props splitting correctly - you will always be able to find an edge case showing the current split is not 100% - see #18983 (comment) from @layershifter.
Instead of having magic which works in 95% of the cases but fails in the remaining 5% it is better to require developer to choose explicitly.

Northstar v0 put className on the outermost element

Unfortunately, Northstar is not that consistent, MenuItem renders li > a and className goes to the a

the assumption that users would expect styling to be applied to the entire control

There are different use cases - for positioning the control inside a flex, you need to style the outermost element.
But another use case is you need to change border-radius of the input - that needs to happed on the inner element.

I took a look at a few libraries and made a table of their behaviors here

This is the right approach how we should make decisions, thank you very much for that 👍

My main concern is not about where Input props should be applied to. It is about the impact of this decision on all other components. Let's say we have 50 components, the problem with the wrapper applies to Input, Checkbox, possibly Image, Popup?
But for the remaining 46 components, we are adding the problematic props vs root slot schizophrenia.

For Input let's do whatever we believe is the best - apply the props to Input or the outermost element (or do the split), add root or wrapper or input slot.
But please, for single element components let's keep just one way how to do things:

<Button className="only-here" />

@layershifter:

I took a look at a few libraries and made a table of their behaviors here: #18804 (comment)

Honestly, there is not enough data to make a decision based on it: once we will remove Fluent UI flavors from the table, it's 50/50.

I agree that table wasn't really enough to get a full picture. So I did a larger survey of components in a number of react-based libraries. I found that a large majority follow the rule "id/name goes to primary slot, and class goes to root slot".
The results are here: https://hackmd.io/r3ii_hFuSBGQAROL5HWGaw?view

Also for what it's worth, we may want to weight FluentUI flavors more heavily, since that's what many of our users will be upgrading from. Option C does make the upgrade scenario more difficult, as it changes the behavior in a way that can't be caught by build breaks or warnings. We could likely write a codemod to help with that though.

I propose an Option C:

Option C works and gets my vote 👍, and gives instant compatibility to form validation libraries. I just see no point in calling it root since we already use this terminology consistently across v9 as the root DOM element. IMO any kind of special knowledge for root in a specific scenario a step in the wrong direction. We can obviously change that without a breaking change, but it effectively introduces mental effort to figure out again how to make inputs work 😟

Let's just call this wrapper (😅) so that we can consistently ship a pattern for this kind of problem which is still unsolved for every react framework that uses inputs. This will be aligned in the common approach of 'do something special for inputs because we need to be compatible with form validation libs'

Thanks everyone for the input! I'll give a more detailed response later but quick FYI about @behowell's comparison table: note that similar to what @miroslavstastny said about @fluentui/react-northstar, @fluentui/react is also wildly inconsistent about whether DOM root or "primary" element gets the top-level native props such as className--maybe roughly 50/50 split. 😬 And then there's an awkward rootProps or inputProps to handle the other one, and lots of related github issues.

I'm proposing Option D based on a meeting including @ecraig12345, @miroslavstastny, @levithomason and others:

Start with Option C, with the following changes:

• className and style are always put on the root slot, even if another slot is a primary slot.
  • For example: <Checkbox id="myId" className="myClass"> -> <div class="myClass"><input id="myId" /></div>
• There is a prop for the primary slot, only when root is not the primary slot.
  • For example, Checkbox would have an input prop for its primary slot, and a root prop for its root slot.
  • Button would not have root or button props, since root is its primary slot.
• If the same prop is specified on both the component and the primary slot prop, then the slot prop wins. (This is somewhat arbitrary, and we do not recommend that users rely on this, but we need to pick one and document it.)
  • For example: <Checkbox id="foo" input={{ id: 'bar' }}> -> "bar" wins: <div><input id="bar" /></div>

Some background on the reasoning:

• All of the options have some sort of inconsistency, but putting className on the root is likely what most people would expect.
  • The vast majority of component libraries I could find put className on the root of a component, notably including FluentUI v8 and Northstar v0. See my comment above for more info.
  • Follows the principle of least surprise.
• We need a prop for the primary slot when it is not the root slot because we need a way to add className/style to the primary slot. But, we don't want to overcomplicate simple components: we don't want to expose a root prop in cases where root is the primary slot because it is entirely redundant and unnecessary.

I'm proposing Option D based on a meeting including @ecraig12345, @miroslavstastny, @levithomason and others:

Start with Option C, with the following changes:

• className and style are always put on the root slot, even if another slot is a primary slot.

  • For example: <Checkbox id="myId" className="myClass"> -> <div class="myClass"><input id="myId" /></div>

• There is a prop for the primary slot, only when root is not the primary slot.

  • For example, Checkbox would have an input prop for its primary slot, and a root prop for its root slot.
  • Button would not have root or button props, since root is its primary slot.

• If the same prop is specified on both the component and the primary slot prop, then the slot prop wins. (This is somewhat arbitrary, and we do not recommend that users rely on this, but we need to pick one and document it.)

  • For example: <Checkbox id="foo" input={{ id: 'bar' }}> -> "bar" wins: <div><input id="bar" /></div>

Some background on the reasoning:

• All of the options have some sort of inconsistency, but putting className on the root is likely what most people would expect.

  • The vast majority of component libraries I could find put className on the root of a component, notably including FluentUI v8 and Northstar v0. See my comment above for more info.
  • Follows the principle of least surprise.

• We need a prop for the primary slot when it is not the root slot because we need a way to add className/style to the primary slot. But, we don't want to overcomplicate simple components: we don't want to expose a root prop in cases where root is the primary slot because it is entirely redundant and unnecessary.

I think we are kind of bound by our slot approach to be as consistent as possible. Other libraries do not expose a way to attempt to style or apply attributes to almost ever DOM element slot of a component. The circumstances really change for Fluent IMO because we are actually selling our slot architecture in this way.

Let's look at evergreen's checkbox:

<label @class @style @data>
  <input type="checkbox" @id @name />
  <div />
  <span />
</label>

It is just a mix and match of attributes, and styling the spanor input requires some weird mix of css selectors. Adding any aria-* to the checkbox results in adding it to the <label> tag.

I think we need consistency becuase we are actively pushing slots as a part of our general architecture. Therefore my vote is still on option C

My vote is still on C.

IMO we should be consistent and props should go to the same slot:

// "id" & "className" should go to the same slot
<Input id="bar" className="foo" />

• no need to add input slot
• no need to explain why something goes there and something else goes there

My 2c;

Agree with @behowell above; className and style of a component should always affect the root. Predictability is key. When you use the components, you need to position them in a flexbox or target a grid cell name and you will intuitively create a class and stick it on the component. Same goes for style. In v8 and below, we mostly followed this rule... with the exception of a few cases where className ended up going to an element inside the root, and it completely confused people. Really want to avoid repeating that mistake.

Beyond those 2 props, ideally 99% of the other top level props "do the right thing" and you only need to break into slot props in a very very rare occasion. I think Option C (with the exceptions) should lead to this. You can style the <Input/> like you expect, but also throwing a name or id on it for form scenarios will also do the right thing.

@miroslavstastny @ling1726 @bsunderhus Thanks for the approvals! Just to be 100% clear, which option are you approving?

I've updated the RFC to reflect the Option D proposal from above.
@miroslavstastny @ling1726 @bsunderhus - could you take a look at the new Proposed Design and make sure it still aligns with what you approved? Thanks!

As @miroslavstastny exposed, we're still more inclined to the never do prop splitting approach (Option C).

We have the feeling that this discussion is kind of stuck and none of the the parties are coming to an agreement, so if majority prefers Option D, let's go with Option D and simply get it done 💪🏼, we can always come back to this discussion latter