Slots: Move slot options to new 'slots' prop

[JasonGore]

Pull request checklist

• Addresses an existing issue: Foundation: 'as' Props #8334, Foundation: Render Functions #8335
• Include a change request file using $ npm run change

Description of changes

Due to some thoughts I had while implementing #8754 reinforced by further discussion on said PR, I've decided to make some changes regarding slot options.

Fundamentally slot props are overloaded allowing both "what to render" and "how to render" to coexist in the same props object. However, this approach creates additional friction by merging "what" and "how" into the same complex object. It also prevents devs from using shorthand when providing any slot implementation.

This PR creates a new slots prop that is automatically added to components through a helper interface. This slots prop now contains the slot options for all of the slot props identified for the component.

This change will also make it a bit harder to impact perf negatively when using props and slots by minimizing need to instantiate complex objects and more easily allowing use of constant objects.

Slots API

Before:

Slots interface before:

{
  root: {
    props: rootProps,
    render: { },
    component: { }
  },
  content: {
    props: contentProps,
    render: { },
    component: { }
  }
}

After:

{
  root: rootProps,  // (also supports shorthand, such as strings)
  content: contentProps,  // (also supports shorthand, such as strings)
  slots: {
    root: {
      render: { },
      component: { }
    },
    content: {
      render: { },
      component: { }
    },
  }
}

Practical Examples

Before:

<Button
  // This object is combining dynamic content (`iconName`) with static render values (`render`).
  // Devs may find themselves unnecessarily creating new objects for otherwise static content.
  icon: {
    props: { iconProps },
    render: (iconProps, DefaultComponent) => (
      <b>
        Icon: <DefaultComponent {...iconProps} />
      </b>
    )
  }
/>

After:

<Button
  // Dynamic content separated out.
  icon={ iconProps }
  // Static content that can be defined as a const externally, 
  // or even as a component variant with default `slots` props.
  slots={{
    icon: {
      render: (iconProps, DefaultComponent) => (
        <b>
          Icon: <DefaultComponent {...iconProps} />
        </b>
      )
    }
  }}
/>

Before

// New object! Gross.
<Button icon={{ props: iconProps }} />

After

// Back to passthrough.
<Button icon={ iconProps } />

Focus areas to test

(optional)

Microsoft Reviewers: Open in CodeFlow

[msft-github-bot]

Component perf results:

Scenario	Target branch avg total (ms)	PR avg total (ms)	Target branch avg per item (ms)	PR avg per item (ms)
PrimaryButton	101.313	106.535	1.013	1.065
BaseButton	34.513	35.114	0.345	0.351
NewButton	119.785	132.822	1.198	1.328
button	4.865	4.563	0.049	0.046
DetailsRows without styles	188.649	187.449	1.886	1.875
DetailsRows	206.598	208.809	2.066	2.088
Toggles	49.150	51.234	0.491	0.512
NewToggle	70.090	71.134	0.701	0.711

[dzearing]

Overall I like it.

Instead of this:

{
  slots: {
    root: {
      component: Foo
    }
  }
}

Could we simplify?

{
  slots: {
    root: Foo
  }
}

[JasonGore]

I still think we want to support an expandable slot options bag. This PR just moves it.

Components have the signature:

component: (props: TProps, context?: any) => ReactElement | null;

While render functions have the signature:

render: (props: TProps, component: ComponentType<TProps>) => ReactElement | null;

It is not possible to differentiate between them via typing or at run time. I think it's a good idea to have them separated to prevent us from stomping or conflicting with the use of context while also having strong typing on the use of component argument. It will also let us add more options over time such as element which I had considered adding as part of this PR.

[dzearing]

Another idea, make the type of the slot: React.ReactType<TProps> | ISlotOptions

That way both work:

{
  slots: {
    upButton: UpButton,
    downButton: {
      render: (p, C) => <Tooltip><C {...p} /></Tooltip>
    }
  }
}

If your goal is to just render something different, the API is clean and simple. If your goal is to wrap stuff, we can support alternative, more verbose ways. I think the common case is the replace, while the wrapping scenario is a minority.

[JasonGore]

Yes, but we will lose all type safety. That's the fault of TS3.x regression on top of some issues with unions. I'd rather wait to add this until TS3.5 is released (which would be a backwards compatible change.)

[dzearing]

love it!