Back to Fluentui

RFC: root as a slot

docs/react-v9/contributing/rfcs/react-components/convergence/root-as-slot.md

4.40.2-hotfix29.1 KB
Original Source

RFC: root as a slot


@bsunderhus @layershifter @ling1726

Summary

This RFC proposes to treat root as a slot to simplify signature, improve Typings and avoid conflicts between root and other slots signatures.

Main changes would be:

  1. root becomes a regular slot on component state
  2. Support references on every slot
  3. Supports any React Element on every slot
  4. Allows a restricted as prop on every slot
  5. Stops using getNativeElementProps on every slot, will be used only for root

Problem statement

There are disparities in how root is treated compared to slots. Some discussions have popped up due to these divergences between slots and root:

  1. Do not support as prop for slots that render native DOM elements conflicts with 1st rule of ARIA opt-out mechanism
  2. Support for any React Elements on slots but not for root
  3. Right now Typings for root aren't available as there's no way to split root from internal state.
  4. There's no possible way of passing refs to slots but root does
  5. Primary slot

Major differences between root (shown below as 🌿) and slots (shown below as 🎰):

  1. References 🌿
  2. any React Elements as base element 🎰
  3. as prop 🌿
  4. Well defined Types 🎰

Detailed Design or Proposal

For the user of the component perspective there will be no changes in the signature.

Converge root to a slot

This RFC proposes to treat root as a slot.

Treating root as a slot means having to declare root Typings together with other slots. This will impact in a components state signature, where root will have to be declared.

⚠️ That doesn't mean root will be available in component's properties interface

Before

ts
export type Slots = {
  slot: ObjectShorthandProps<SlotProps>;
};
export interface Props extends ComponentProps<Slots> & React.HTMLAttributes<HTMLElement>
export interface State extends ComponentState<Slots>

// use*State hook
function useState({ slot, ...props }: Props): State {
  // ...
  return {
    ...props,
    slot: resolveShorthand(slot),
  };
}

With the modifications:

ts
export type Slots = {
  root: ObjectShorthandProps<React.HTMLAttributes<HTMLElement>, HTMLElement>;
  slot: ObjectShorthandProps<SlotProps>;
};
export interface Props extends ComponentProps<Slots> {
  /* ... */
}
export interface State extends ComponentState<Slots> {
  /* ... */
}

// use*State hook
function useState({slot, ...props}: Props): State {
  // ...
  return {
    slot: resolveShorthand(slot)
    root: getNativeElementProps(state.components.root, props),
  };
}

Make slots support as property (but with restrictions)

Cancels Do not support as prop for slots that render native DOM elements as it already conflicts with 1st rule of ARIA opt-out mechanism. This would add as property for ObjectShorthandProps signature for all slots.

To ensure as property is used for opt-out mechanism only native elements will be supported. Typings must be used to ensure which native elements are supported.

ObjectShorthandProps should be a model that shorthand properties should extend from, and to ensure as property is only provided in cases that opt-out mechanism is required the never type should be used to disable as property on default cases.

ts
type ObjectShorthandProps<Props = {}, Ref = unknown, As extends keyof JSX.IntrinsicElements = never> = Props &
  React.RefAttributes<Ref> & {
    as?: As;
    children?: Props['children'] | ShorthandRenderFunction<Props>;
  };

The case for useARIAButton is a good example of using this signature:

ts
export type ARIAButtonShorthandProps =
  | ObjectShorthandProps<JSX.IntrinsicElements['button'], HTMLButtonElement, /*as*/ 'button'>
  | ObjectShorthandProps<JSX.IntrinsicElements['div'], HTMLDivElement, /*as*/ 'div'>
  | ObjectShorthandProps<JSX.IntrinsicElements['span'], HTMLSpanElement, /*as*/ 'span'>
  | ObjectShorthandProps<JSX.IntrinsicElements['a'], HTMLAnchorElement, /*as*/ 'a'>;

ARIAButtonProps is a union between types from button, div, span and anchor native elements.

⚠️ using union types for declaring properties is not perfect https://catchts.com/unions

No reason for getNativeElementProps

The problem reported on Widening Types, makes this a little bit controversial.

Although types should be enough to ensure that only supported properties will be passed to a slot, this is less true for root at least due to the Widening Types problem. Aside from root, it's safe to say that getNativeElementProps is not necessary.

Add references to all slots

Right now ref is not supported on Typings for slots. This proposes that ref should be part of the internal interface of ObjectShorthandProps to allow access of references to internal slots of a given component.

ts
type ObjectShorthandProps<Props = {}, Ref = unknown, As extends keyof JSX.IntrinsicElements = never> = Props &
  React.RefAttributes<Ref> & {
    as?: As;
    children?: Props['children'] | ShorthandRenderFunction<Props>;
  };

Implementation Details

Changes are located mostly on react-utilities/compose methods and Typings.

ObjectShorthandProps will have to support as and ref properties with Generics:

ts
type ObjectShorthandProps<Props = {}, Ref = unknown, As extends keyof JSX.IntrinsicElements = never> = Props &
  React.RefAttributes<Ref> & {
    as?: As;
    children?: Props['children'] | ShorthandRenderFunction<Props>;
  };

All other Typings would be adapted for the changes provided by ObjectShorthandProps.

getSlots method would stop having special cases for root and only a simple loop around all provided slots would be enough.

ts
export function getSlots<R extends ObjectShorthandPropsRecord>(
  state: ComponentState<R>,
  slotNames: (keyof R)[] = [],
): {
  slots: Slots<R>;
  slotProps: SlotProps<R>;
} {
  const slots = {} as Slots<R>;
  const slotProps = {} as SlotProps<R>;

  for (const slotName of slotNames) {
    const [slot, props] = getSlot(state, slotName);
    slots[slotName] = slot;
    slotProps[slotName] = props;
  }
  return { slots, slotProps: slotProps };
}

Pros and Cons

Pros

  1. Treating root as a simple slot simplifies Typings
  2. Simplification of Typings also comes with better inference performance
  3. References available on every slot
  4. Not using getNativeElementProps for all slots means possible performance improvement

Cons

  1. Widening Types problem
  2. Explicit slots declaration makes easier for developers to make mistakes

Possible mistakes

  • Since root becomes a regular slot, it might be possible developers forget to include it in the list of shorthands
  • Since typings become a responsibility of the developer, typing errors might be prone, e.g: bad usage of Partial or Required (this can be mitigated by forcing optional on props and required on state)

Both of these could be mitigated easily enough by good documentation and possibly also lint rules or danger checks.

⚠️ Widening types problem ⚠️

Since Typings for root is a subset of the interface that declares the properties of a given component, widening mechanism from assigning types will ensure that root Typings are compatible with props which is not necessarily true! This implicates in some conflicts on properties spreading through state and root slot.

The problem can be verified in the next example:

tsx
type ComponentProps<S extends ObjectShorthandPropsRecord> = Omit<
  { [Key in keyof S]?: ShorthandProps<S[Key]> },
  'root'
> &
  S['root']; // this will make Slots['root'] part of Props

type Slots = {
  root: ObjectShorthandProps<React.HTMLAttributes<HTMLElement>, HTMLElement>;
};

interface Props extends ComponentProps<Slots> {
  /**
   * Property that can be passed through props
   * but doesn't belong on the element itself,
   * it's only used internally by state
   */
  stateSpecificProperty?: unknown;
}
interface State extends ComponentState<Slots> {
  stateSpecificProperty: unknown;
}

// ...
// use*State hook
return {
  // This should error 🚨 because of stateSpecificProperty,
  // but it doesn't due to type widening
  root: resolveShorthand(props),
};

A easy solution would be to use getNativeElementProps to filter out properties in this specific case:

tsx
// use*State hook
return {
  // This filters out stateSpecificProperty ✅
  root: getNativeElementProps(state.components.root, props),
};