RFC: Using mutable draft state within hooks

[dzearing]

This relates to PR #14268 (compose replacement updates, introducing mergeProps, getSlots, and resolveShorthandProps.)

Proposal

When creating component hooks, merge state by creating "draft state", allowing hooks to mutate that, in an effort to reduce perf overhead in constructing the final state for rendering.

Problem

Trying to keep hooks immutable forces multiple, unnecessary objects to be created between each hook, holding a slight deviation of state, forcing awkward merging between calls to ensure state is properly piped through them. This creates perf overhead and awkward code. We should evaluate patterns which avoid any extraneous overhead.

Solution

Prior art: https://immerjs.github.io/immer/docs/introduction

In immer, the produce helper is used to consume input, create draft state, allow changes to be applied, and the final result is returned to be consumed.

Likewise, we'd like to evaluate a similar solution for weaving state through some of our core hooks:

1. Take in user input
2. Create a draft state from input (a deep clone with props-centric exceptions)
3. Allow mutations on the draft from N standard behavior/state/styling hooks
4. Consume the final state in render

This has the following benefits:

• Original props are not mutated
• Performance; less spread calls, less GC pressure in bulk
• Structural sharing, hooks have an opportunity to manipulate the state rather than deferring responsibility elsewhere
• Deeper updates are simple (e.g. replacing { icon: { as: ... } })
• Boilerplate reduction. Less noise, more concise code.

Details

Many hooks take in state, read some values, and adjust those values. Small behavior hooks almost always do this.

const useToggleButton = (props: ButtonProps) => {
  let state = useButton(props); // clone the state.

  // clone the state, handle checked/defaultChecked, and onClick to toggle and take special care
  // to call the original onClick if provided.
  state = useChecked(state);

  return state; 
};

Often the state calculations must be chained through multiple hooks which may manipulate parts of the state.

// clone to preserve immutability
let state = useInputState(props);

// Grab a classname and style from the hook 
const { className, style } = useInputStyles(state);

// Use the computed style and tokens to create a new style
const style = convertTokensToStyle(state.tokens, style);

// Use the previously computed classname and variant to create a new variant classname
const className = useVariant('Input', state.variant, className);

// Apply focus rects given a ref pointing to the root element. Return a new ref if the given one is not defined.
const ref = useFocusRects(state.ref);

// Return the final result.
return {
...state,
style,
className,
ref,
);

There were numerous error-prone places which could have easily made an error using the wrong value:

• If useVariant referred to props.className, or state.className, it would not have used the computed value from the previous hook.
• If any of these hooks returned nested object updates, they would have needed to be deep cloned.

Some of these problems can be simplified by simplifying the inputs and ouputs to take in state and return new state:

let state = useInputState(props); // clone to preserve immutability
state = useInputStyles(state);  // clone to preserve immutability
state = useTokensToStyle(state);  // clone to preserve immutability
state = useVariants('Input', state);  // clone to preserve immutability
state = useFocusRects(state); // clone to preserve immutability

Now each hook can be responsible for correctly updating the state without accidents. However, it requires each hook to properly clone as neeeded, and the consumer must weave the state through each hook as shown.

We can simplify this further by simply creating a clone up front and letting the hooks mutate the state:

// Create draft state (deep clone with some exceptions)
const draftState = mergeProps(defaultProps, userProps);

useInputState(draftState);
useTokensToStyle(draftState);
useVariants('Input', draftState);
useFocusRects(draftState);

Benefits:

1. One clone (performance)
2. No opportunities to use "old data" (less errors)
3. No boilerplate weaving state through anything (less code)
4. Hooks can be self contained and add behavior (more modularity, easier integration)

Drawbacks:

1. Hook modifications are not specific in API surface.
2. If deep clone does not clone everything, some modifications may need extra care to avoid mutating originals.
3. Most hooks are designed to take in inputs and return specific outputs. Might be hard to differentiate without naming conventions. (Recommend referring to mutable state as draftState)

One size does not fit all

In some cases, this is overkill. Simple components which wrap a primitive and attach styling may be a case where a deep clone seems unneeded;

const FancyDiv = (props) => {
  const className = useStyles(?);
  return <div {...props} className={ className } />
};

Details in mergeProps deep clone exceptions

When creating draft state, deep merging would be used, with these exceptions being that they would be treated as literals and replace the previous value rather than be walked.

• className props are always joined properly
• Functions would be replaced, not appended
• Arrays would be replaced, not appended
• JSX would be replaced, not cloned
• Ref objects, which are instance sensitive, would be replaced
• Class instances would be replace rather than deep merged

This means that if you merged 2 functions:

mergeProps(
  { onClick: () => console.log('A') }, 
  { onClick: () => console.log('B') }, 
); 

Last one wins. (// { onClick: () => console.log('B') }) Same with JSX, arrays, ref objects, and class instances.

These could be manually patched if needed to be merged:

mergeProps(
  propsA,
  propsB,
  { onClick: appendFunctions(propsA.onClick, propsB.onClick) }
);

Refs would be good to auto-merge, but we can't easily distinguish a ref from a callback function or object.

[dzearing]

So far, the mutable draft state has been very useful and performant. However, there is still concern that this isn't intuitive and contradicts the expectations of hooks. (Clear immutable ins and outs.)

We may want to use a naming convention to separate hooks which operate on draft state. The integration works very similarly to "mixins" of the past. Another common term is "behaviors".

How about a "Mixin" suffix?

const Button = React.forwardRef((props, ref) => {
  const { render, state } = useButton();

  // The "Mixin" suffix indicates that classes will be mixed into the state.
  useButtonClassesMixin(state);

  return render(state);
});

👍 Clearly indicates the hook "mixes stuff into state". 👎 Wordy, makes for a long function name.

[msft-fluent-ui-bot]

Because this issue has not had activity for over 150 days, we're automatically closing it for house-keeping purposes.

Still require assistance? Please, create a new issue with up-to date details.