make-overrides.md

RFC: Replace makeStyles() implementation / API updates

---

@layershifter

Summary

This RFC proposes to replace existing makeStyles() implementation with a new one, i.e. proposes API changes.

Background

In microsoft/fluentui#16082 we have implemented a version of makeStyles() that uses atomic classes. Recent work on makeOverrides() shows significant performance improvements microsoft/fluentui#16923 (around 15%) by changes in the implementation design.

To avoid confusion, the names makeOverrides() and makeStyles are both used in this RFC. This RFC also proposes the replacement of makeStyles with the makeOverrides implementation.

Problem statement

makeStyles() implementation has two problems that will cause issues at scale: matchers and missing slots. The snippet below highlights these issues:

import { ax, makeStyles } from "@fluentui/react-make-styles";

const useRootStyles = makeStyles<TSelectors>([
  [null, { color: "red" }],
  [
    s => s.primary /* <- a matcher function, will be executed on each render */,
    { color: "blue" }
  ]
]);

const useIconStyles = makeStyles<TSelectors>([
  /* styles for each slot are defined separately */
  [null, { background: "black" }],
  [s => s.primary, { background: "white" }]
]);

function Component() {
  const rootClasses = useRootStyles();

  // React hooks cannot be called conditionally, https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
  // styles *must* be rendered even if they are not used in markup
  const iconClasses = useIconStyles();

  return (
    <div className={ax(rootClasses, props.className)}>
      {props.icon && <div className={ax(iconClasses, props.icon.className)} />}
    </div>
  );
}

To compute classnames we need to understand what styles should be applied and thus execute matchers on each render as memoization of user's input is more expensive.

This moves us to a next issue: there is no way to define styles for multiple slots/components with a single call of the makeStyles(). Each slot will require a separate call of makeStyles(). Since these calls are represented by React hooks we can't bail out even if these slots are not rendered.

Side note:

Initially we have been focused on implementation of atomic CSS and merging style definitions. As a source for inspirations we have used Facebook stylex talk:

• there was nothing like ax() function to merge atomic classes
• there was no contextual RTL support (parts of app can be rendered with different text directions)

Detailed Design or Proposal

To solve these issues we made a step back to the original API of makeStyles() (current iteration is a second version) and as we introduced ax() to merge atomic classnames we explored different solutions.

Proposed API solve the problem in a "vandal" way 🪓 Matchers are moved to user's scope thus we can have all definitions in a single makeStyles() call => we have a single React hook. See a modified snippet below:

import { ax, makeOverrides } from "@fluentui/react-make-styles";

const useStyles = makeStyles({
  /* 👍 no matchers, no need to execute on each render */
  root: { color: "red" },
  rootPrimary: { color: "blue" },

  /* 👍 styles for each slot are defined together (not a requirement) */
  icon: { background: "black" },
  iconPrimary: { background: "white" }
});

function Component() {
  /* 👍 a single call of React hook */
  const classes = useStyles();

  return (
    <div
      className={ax(
        classes.root /* The concept of matching is replaced with selective classname concat */,
        props.primary && classes.rootPrimary,
        props.className
      )}
    >
      {props.icon && (
        <div
          className={ax(
            classes.icon,
            props.primary && classes.iconPrimary,
            props.icon.className
          )}
        />
      )}
    </div>
  );
}

Pros and Cons

These changes result in performance improvements in makeOveriddes:

• matcher functions are no longer executed on each render in styling functions
• styles are executed and merged only when slots are present
• less React hooks to call for slots

Surprisingly, this also moves us closer to CSS Modules. The same snippet with CSS Modules, for example:

import cx from "classnames";
import * as classes from "./Component.css"; // the same styles can be written in CSS

function Component() {
  return (
    <div
      className={cx(
        classes.root,
        props.primary && classes.rootPrimary,
        props.className
      )}
    >
      {props.icon && (
        <div
          className={cx(
            classes.icon,
            props.primary && classes.iconPrimary,
            props.icon.className
          )}
        />
      )}
    </div>
  );
}

In microsoft/fluentui#16923 Avatar, from @fluentui/react-avatar (check packages/react-avatar/src/components/Avatar/useAvatarStyles.ts), was used to validate performance improvements and explore potential issues. It is uncertain if/how these issues are critical, but there are two things that should be mentioned:

const useStyles = makeOverrides({
  /* ... Complex apps/components can contain many definitions ... */

  rootShape20: { width: '20px', height: '20px' },
  rootShape24: { width: '24px', height: '24px' },
  rootShape28: { width: '28px', height: '28px' },
})

export const useAvatarStyles = (state: AvatarState): AvatarState => {
  const classes = useStyles();

  state.className = ax(
    classes.root,

    // 👎 Matchers have been moved to ax() calls, it looks a bit verbose
    //    (in previous implementation matchers have been close to styles)
    // 👎 It might be tricky find proper names to express definition names
    //    (we can end with "rootPrimaryCirclularGhostEtc.")

    state.size === 20 && classes.rootShape20,
    state.size === 24 && classes.rootShape24,
    /* ... many selectors ... */
    state.size === 128 && classes.rootShape128
  );

  return state;
};

Discarded Solutions

NA

Open Issues

NA