[Bug]: StrictMode breaks v9 theming

[dzearing]

Library

React Components / v9 (@fluentui/react-components)

System Info

react 18.0.0
@fluentui/react-component 9.0.0-rc.15

Are you reporting Accessibility issue?

No response

Reproduction

https://codesandbox.io/s/nice-villani-7vgw53?file=/src/index.tsx

Bug Description

Actual Behavior

Ugly buttons

Expected Behavior

Pretty buttons (remove the StrictMode to see)

Logs

No response

Requested priority

High

Products/sites affected

No response

Are you willing to submit a PR to fix?

no

Validations

• Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
• The provided reproduction is a minimal reproducible example of the bug.

[dzearing]

Maybe React 18 related, not sure. (#22241)

[micahgodbolt]

probably ID related. i repro'ed it with a base react 18 repro

[micahgodbolt]

Issue comes down to how we inject styles into the DOM at render, but clean up on useEffect return. This approach is necessary because useEffect/useLayoutEffect is too slow to be injecting styles into the DOM without a ton of style recalculation. React 18 provides a useInsertionEffect that is tailor made to support CSS in JS libraries inserting styles into the document.

The solution will be forking behavior and using the new hook when it is available. I have a prototype branch working now, and currently gathering feedback on the solution.

https://github.com/micahgodbolt/fluentui/pull/1/files

[layershifter]

@micahgodbolt current code can be simply refactored to use useInsertionEffect always and fallback to useLayoutEffect when it's not present like in Emotion 🐱 This is safe for older React versions as:

useInsertionEffect is new in React 18. It works roughly like useLayoutEffect except you don't have access to refs to DOM nodes at this time.
reactwg/react-18#110

This allows reduce amount of conditions & branches. Refactor can also remove need of usePrevious() as it will be handled in the effect 💪

We can keep creating an element during render as it has clear dependencies (styleTagId & targetDocument) then useInsertionEffect will also have clear dependencies ⬇️

Edit: 👆 we can't as it's the actual source of the problem: rendering happens twice (we create element1 and element2 due different ids) and then an effect will run with element2 that will be deleted while element1 remains in DOM 💥

---

This approach is necessary because useEffect/useLayoutEffect is too slow to be injecting styles into the DOM without a ton of style recalculation.

Do we have a confirmation for that? It sounds true for Griffel's case (it inserts tons of rules to elements, but it will cause recalculations even now as any insert to style causes them), but there we insert a single CSS rule to a new element. I captured two profiles with React 17 and results look the same:

useLayoutEffect

during render

---

I made some adjustments to the original code in the PR (micahgodbolt#1):

const useInsertionEffect = React["useInsertion" + "Effect"]
  ? React["useInsertion" + "Effect"]
  : React.useLayoutEffect;

// ---

useInsertionEffect(() => {
  styleTag.current = createStyleTag(targetDocument, styleTagId);

  if (styleTag.current) {
    insertSheet(styleTag.current, cssRule);

    return () => {
      styleTag.current.remove();
    };
  }
}, [cssRule, targetDocument, styleTagId]);

---

Note, the usage of Emotion's hack (React["useInsertion" + "Effect"]) is required otherwise we will get a problem on consumer's side:

P.S. Tested with React 17 & React 18 in StrictMode & without - works like a charm 💎