[wb1812.0.fixrenderstate] Make sure RenderState.Root is an internal c…

…oncept only (#2387)

## Summary:
This addresses an issue where `useRenderState` would return `RenderState.Root` during initial render to components nested inside our `RenderStateRoot` component, when they should only ever see `Initial` or `Standard`.

I suspect this was happening due to hook execution order and render order. React component render functions are executed from the inside out, and I suspect that because of this, the initial call to `useRenderState` is using the default context value, because the root component hasn't rendered it's value yet - a general flaw in hook-based context access.

However, for everything but `RenderStateRoot` and `InitialFallback` components, code doesn't need to know if it's the root render or not. It's an easier API if they just always see `Initial` or `Standard`. This change makes that happen. The only components that need to know are those that need to render the actual context, and our consumers don't need to do that.

This addresses an issue where the `useUniqueId` hook and it's initial render fallback version would throw an error on initial render. It's a precursor PR to unblock folks while we work on replacing our unique ID stuff with `useId`.

Issue: WB-1812

## Test plan:
`yarn test`
`yarn typecheck`

I also checked our Wonder Blocks consumers to verify that they don't care about the "root" render state (as they shouldn't).

## Release Info:
This is a major release of Core because we are changing the `RenderState` enum to remove a value, and changing the behavior of the `useRenderState` hook. In real terms, consumers should be unaffected, but changing exports like this should be a major release so following protocol.

Author: somewhatabstract

Reviewers: jandrade, nedredmond

Required Reviewers:

Approved By: jandrade

Checks: ✅ Chromatic - Get results on regular PRs (ubuntu-latest, 20.x), ✅ Lint / Lint (ubuntu-latest, 20.x), ✅ Test / Test (ubuntu-latest, 20.x, 2/2), ✅ Test / Test (ubuntu-latest, 20.x, 1/2), ✅ Check build sizes (ubuntu-latest, 20.x), ✅ Chromatic - Build on regular PRs / chromatic (ubuntu-latest, 20.x), ✅ Publish npm snapshot (ubuntu-latest, 20.x), ⏭️  Chromatic - Skip on Release PR (changesets), ✅ Prime node_modules cache for primary configuration (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ⏭️  dependabot, ✅ gerald

Pull Request URL: #2387

.changeset/bright-apes-sparkle.md

@@ -0,0 +1,6 @@
+---
+"@khanacademy/wonder-blocks-core": major
+---
+
+-   Remove `RenderState.Root` from exported enum
+-   Change `useRenderState` to only return `RenderState.Initial` or `RenderState.Standard`

__docs__/wonder-blocks-core/exports.use-render-state.mdx

@@ -1,8 +1,6 @@
 import {Meta} from "@storybook/blocks";
 
-<Meta
-    title="Packages / Core / Exports / useRenderState()"
-/>
+<Meta title="Packages / Core / Exports / useRenderState()" />
 
 # useRenderState()
 
@@ -16,6 +14,3 @@ The `useRenderState` hook will return either:
     the initial rehydration render on the client.
 -   `RenderState.Standard` if the component renders on the client after the initial
     rehydration.
-
-NOTE: Although the `RenderState` enum has a third state `Root`, this value is never
-returned by `useRenderState`.

packages/wonder-blocks-core/src/components/initial-fallback.tsx

@@ -1,6 +1,6 @@
 import * as React from "react";
 
-import {RenderState, RenderStateContext} from "./render-state-context";
+import {RenderStateInternal, RenderStateContext} from "./render-state-context";
 
 /**
  * We use render functions so that we don't do any work unless we need to.
@@ -88,7 +88,9 @@ export default class InitialFallback extends React.Component<Props, State> {
             // do their thing. Components don't mount during SSR, so we won't
             // hit this when server-side rendering.
             return (
-                <RenderStateContext.Provider value={RenderState.Standard}>
+                <RenderStateContext.Provider
+                    value={RenderStateInternal.Standard}
+                >
                     {children()}
                 </RenderStateContext.Provider>
             );
@@ -100,7 +102,9 @@ export default class InitialFallback extends React.Component<Props, State> {
         // and they're not in charge of initiating the next render.
         if (fallback) {
             return (
-                <RenderStateContext.Provider value={RenderState.Initial}>
+                <RenderStateContext.Provider
+                    value={RenderStateInternal.Initial}
+                >
                     {fallback()}
                 </RenderStateContext.Provider>
             );
@@ -110,22 +114,20 @@ export default class InitialFallback extends React.Component<Props, State> {
         return null;
     }
 
-    _maybeRender(
-        renderState: typeof RenderState[keyof typeof RenderState],
-    ): React.ReactNode {
+    _maybeRender(renderState: RenderStateInternal): React.ReactNode {
         const {children, fallback} = this.props;
 
         switch (renderState) {
-            case RenderState.Root:
+            case RenderStateInternal.Root:
                 return this._renderAsRootComponent();
 
-            case RenderState.Initial:
+            case RenderStateInternal.Initial:
                 // We're not the root component, so we just have to either
                 // render our placeholder or nothing.
                 // The second render is going to be triggered for us.
                 return fallback ? fallback() : null;
 
-            case RenderState.Standard:
+            case RenderStateInternal.Standard:
                 // We have covered the SSR render, we're now rendering with
                 // standard rendering semantics.
                 return children();
@@ -156,7 +158,7 @@ export default class InitialFallback extends React.Component<Props, State> {
             // We "fallthrough" to the root case. This is more obvious
             // and maintainable code than just ignoring the no-fallthrough
             // lint rule.
-            return this._maybeRender(RenderState.Root);
+            return this._maybeRender(RenderStateInternal.Root);
         }
     }
 

packages/wonder-blocks-core/src/components/render-state-context.ts

@@ -1,8 +1,57 @@
 import * as React from "react";
 
+/**
+ * The possible states of rendering.
+ *
+ * This is used to determine if we are rendering our initial, hydrateable state
+ * or not. Initial renders must be consistent between the server and client so
+ * that hydration will succeed.
+ *
+ * We use a render state like this instead of a simple check for being mounted
+ * or not, or some other way of each component knowing if it is rendering itself
+ * for the first time so that we can avoid cascading initial renders where each
+ * component has to render itself and its children multiple times to reach a
+ * stable state. Instead, we track the initial render from the root of the tree
+ * and switch everything accordingly so that there are fewer additional renders.
+ */
 export enum RenderState {
+    /**
+     * The initial render, either on the server or client.
+     */
+    Initial = "initial",
+
+    /**
+     * Any render after the initial render. Only occurs on the client.
+     */
+    Standard = "standard",
+}
+
+/**
+ * The internal states of rendering.
+ *
+ * This is different to the `RenderState` enum as this is internal to the
+ * Core package and solely for components that are going to provide new values
+ * to the render state context.
+ */
+export enum RenderStateInternal {
+    /**
+     * This is the root state. It indicates that nothing has actually changed
+     * then context value that tracks this. This is used solely by components
+     * that control the rendering state to know that they are in charge of
+     * that process.
+     */
     Root = "root",
+
+    /**
+     * This indicates something has taken charge of the rendering state and
+     * components should render their initial render state that is hydrateable.
+     */
     Initial = "initial",
+
+    /**
+     * This indicates that things are now rendering after the initial render
+     * and components can render without worrying about hydration.
+     */
     Standard = "standard",
 }
 
@@ -21,9 +70,9 @@ export enum RenderState {
  * standard:
  *   means that we're all now doing non-SSR rendering
  */
-const RenderStateContext = React.createContext<
-    typeof RenderState[keyof typeof RenderState]
->(RenderState.Root);
+const RenderStateContext = React.createContext<RenderStateInternal>(
+    RenderStateInternal.Root,
+);
 RenderStateContext.displayName = "RenderStateContext";
 
 export {RenderStateContext};

packages/wonder-blocks-core/src/components/render-state-root.tsx

@@ -1,9 +1,8 @@
 import * as React from "react";
 
-import {RenderState, RenderStateContext} from "./render-state-context";
-import {useRenderState} from "../hooks/use-render-state";
+import {RenderStateInternal, RenderStateContext} from "./render-state-context";
 
-const {useEffect, useState} = React;
+const {useEffect, useState, useContext} = React;
 
 type Props = {
     children: React.ReactNode;
@@ -18,12 +17,12 @@ const RenderStateRoot = ({
     throwIfNested = true,
 }: Props): React.ReactElement => {
     const [firstRender, setFirstRender] = useState<boolean>(true);
-    const renderState = useRenderState();
+    const renderState = useContext(RenderStateContext);
     useEffect(() => {
         setFirstRender(false);
     }, []); // This effect will only run once.
 
-    if (renderState !== RenderState.Root) {
+    if (renderState !== RenderStateInternal.Root) {
         if (throwIfNested) {
             throw new Error(
                 "There's already a <RenderStateRoot> above this instance in " +
@@ -35,7 +34,9 @@ const RenderStateRoot = ({
         return <>{children}</>;
     }
 
-    const value = firstRender ? RenderState.Initial : RenderState.Standard;
+    const value = firstRender
+        ? RenderStateInternal.Initial
+        : RenderStateInternal.Standard;
 
     return (
         <RenderStateContext.Provider value={value}>

packages/wonder-blocks-core/src/hooks/__tests__/use-unique-id.test.tsx

@@ -43,7 +43,7 @@ describe("useUniqueIdWithoutMock", () => {
         expect(factoryValues[0]).toBe(null);
     });
 
-    test("second client render retursn a unique id factory", () => {
+    test("second client render returns a unique id factory", () => {
         // Arrange
         const factoryValues: Array<any> = [];
         const TestComponent = (): React.ReactElement | null => {
@@ -89,20 +89,7 @@ describe("useUniqueIdWithoutMock", () => {
         expect(factoryValues[1]).toBe(factoryValues[2]);
     });
 
-    it("should throw an error if it isn't a descendant of <RenderStateRoot>", () => {
-        // Arrange
-
-        // Act
-        const underTest = () =>
-            renderHookStatic(() => useUniqueIdWithoutMock());
-
-        // Assert
-        expect(underTest).toThrowErrorMatchingInlineSnapshot(
-            `"Components using useUniqueIdWithoutMock() should be descendants of <RenderStateRoot>"`,
-        );
-    });
-
-    it("Should minimize the number of renders it does", () => {
+    it("should minimize the number of renders it does", () => {
         // Arrange
         const values1: Array<any> = [];
         const TestComponent1 = (): React.ReactElement | null => {
@@ -213,19 +200,6 @@ describe("useUniqueIdWithMock", () => {
         expect(factoryValues[1]).toBe(factoryValues[2]);
     });
 
-    it("should throw an error if it isn't a descendant of <RenderStateRoot>", () => {
-        // Arrange
-
-        // Act
-        const underTest = () =>
-            renderHookStatic(() => useUniqueIdWithoutMock());
-
-        // Assert
-        expect(underTest).toThrowErrorMatchingInlineSnapshot(
-            `"Components using useUniqueIdWithoutMock() should be descendants of <RenderStateRoot>"`,
-        );
-    });
-
     it("Should minimize the number of renders it does", () => {
         // Arrange
         const values1: Array<any> = [];

packages/wonder-blocks-core/src/hooks/use-render-state.ts

@@ -2,9 +2,18 @@ import {useContext} from "react";
 
 import {
     RenderState,
+    RenderStateInternal,
     RenderStateContext,
 } from "../components/render-state-context";
 
-export const useRenderState =
-    (): typeof RenderState[keyof typeof RenderState] =>
-        useContext(RenderStateContext);
+export const useRenderState = (): RenderState => {
+    const rawRenderState = useContext(RenderStateContext);
+    // For consumers, they do not care if the render state is initial or
+    // root. That is solely info for the RenderStateRoot component.
+    // To everything else, it's just the initial render or standard render.
+    if (rawRenderState === RenderStateInternal.Standard) {
+        return RenderState.Standard;
+    } else {
+        return RenderState.Initial;
+    }
+};

packages/wonder-blocks-core/src/hooks/use-unique-id.ts

@@ -20,12 +20,6 @@ export const useUniqueIdWithMock = (scope?: string): IIdentifierFactory => {
     const renderState = useRenderState();
     const idFactory = useRef<IIdentifierFactory | null | undefined>(null);
 
-    if (renderState === RenderState.Root) {
-        throw new Error(
-            "Components using useUniqueIdWithMock() should be descendants of <RenderStateRoot>",
-        );
-    }
-
     if (renderState === RenderState.Initial) {
         return SsrIDFactory;
     }
@@ -50,12 +44,6 @@ export const useUniqueIdWithoutMock = (
     const renderState = useRenderState();
     const idFactory = useRef<IIdentifierFactory | null | undefined>(null);
 
-    if (renderState === RenderState.Root) {
-        throw new Error(
-            "Components using useUniqueIdWithoutMock() should be descendants of <RenderStateRoot>",
-        );
-    }
-
     if (renderState === RenderState.Initial) {
         return null;
     }