Fix flaky TeX Chromatic snapshots for Matcher and Sorter widgets

.changeset/stale-carrots-tease.md

@@ -0,0 +1,5 @@
+---
+"@khanacademy/perseus": patch
+---
+
+Fix flakey TeX Chromatic snapshots for Matcher and Sorter widgets.

packages/perseus/src/components/__tests__/with-asset-context.test.tsx

@@ -0,0 +1,94 @@
+import {render, screen} from "@testing-library/react";
+import * as React from "react";
+
+import AssetContext from "../../asset-context";
+import {withAssetContext} from "../with-asset-context";
+
+// Minimal test-only component used to exercise withAssetContext. It accepts
+// the props the HOC injects (`setAssetStatus` and `assetKey`) and surfaces
+// them in two ways the tests can observe: as a `data-asset-key` attribute on
+// the rendered button (for inspecting which key the HOC generated), and via
+// a click handler that calls `setAssetStatus` (so tests can verify the
+// injected function reaches the wrapped component and is callable).
+type MockComponentProps = {
+    label: string;
+    setAssetStatus: (assetKey: string, loaded: boolean) => void;
+    assetKey: string;
+};
+
+function MockComponent(props: MockComponentProps): React.ReactElement {
+    return (
+        <button
+            onClick={() => props.setAssetStatus(props.assetKey, true)}
+            data-testid={props.label}
+            data-asset-key={props.assetKey}
+        >
+            {props.label}
+        </button>
+    );
+}
+
+describe("withAssetContext", () => {
+    it("injects setAssetStatus from AssetContext as a prop", () => {
+        // Arrange
+        const setAssetStatusSpy = jest.fn();
+        const Wrapped = withAssetContext(MockComponent, "widget");
+
+        // Act — clicking calls setAssetStatus via the injected prop, which
+        // proves the function from the Provider reached MockComponent.
+        render(
+            <AssetContext.Provider
+                value={{
+                    assetStatuses: {},
+                    setAssetStatus: setAssetStatusSpy,
+                }}
+            >
+                <Wrapped label="hello" />
+            </AssetContext.Provider>,
+        );
+        screen.getByTestId("hello").click();
+
+        // Assert
+        expect(setAssetStatusSpy).toHaveBeenCalled();
+    });
+
+    it("injects an assetKey with the configured prefix", () => {
+        // Arrange, Act
+        const Wrapped = withAssetContext(MockComponent, "widget");
+        render(<Wrapped label="hello" />);
+
+        // Assert
+        expect(screen.getByTestId("hello").dataset.assetKey).toMatch(
+            /^widget-/,
+        );
+    });
+
+    it("generates a unique assetKey per rendered instance", () => {
+        // Arrange, Act
+        const Wrapped = withAssetContext(MockComponent, "widget");
+        render(
+            <>
+                <Wrapped label="first" />
+                <Wrapped label="second" />
+            </>,
+        );
+
+        // Assert
+        const first = screen.getByTestId("first");
+        const second = screen.getByTestId("second");
+        expect(first.dataset.assetKey).not.toBe(second.dataset.assetKey);
+    });
+
+    it("uses the AssetContext default no-op when no Provider is above", () => {
+        // Arrange, Act — render without an AssetContext.Provider.
+        const Wrapped = withAssetContext(MockComponent, "widget");
+        render(<Wrapped label="hello" />);
+
+        // Assert — the HOC still generates an assetKey (useId works without
+        // a Provider) and the default no-op setAssetStatus is callable (so
+        // the click handler doesn't crash).
+        const button = screen.getByTestId("hello");
+        expect(button.dataset.assetKey).toMatch(/^widget-/);
+        expect(() => button.click()).not.toThrow();
+    });
+});

packages/perseus/src/components/with-asset-context.tsx

@@ -0,0 +1,43 @@
+import * as React from "react";
+
+import AssetContext from "../asset-context";
+
+type SetAssetStatus = (assetKey: string, loaded: boolean) => void;
+
+type InjectedProps = {
+    setAssetStatus: SetAssetStatus;
+    assetKey: string;
+};
+
+export function withAssetContext<P extends InjectedProps>(
+    WrappedComponent: React.ComponentType<P>,
+    keyPrefix: string,
+): React.ForwardRefExoticComponent<
+    React.PropsWithoutRef<Omit<P, keyof InjectedProps>> &
+        React.RefAttributes<any>
+> {
+    const WithAssetContextComponent = React.forwardRef<
+        any,
+        Omit<P, keyof InjectedProps>
+    >((props, ref) => {
+        const {setAssetStatus} = React.useContext(AssetContext);
+        const uniqueId = React.useId();
+        const assetKey = `${keyPrefix}-${uniqueId}`;
+
+        return (
+            <WrappedComponent
+                // eslint-disable-next-line no-restricted-syntax -- TypeScript can't verify that spreading `Omit<P, K>` + adding `K`-properties reconstitutes P (the "P could be a different subtype" limitation). The runtime behavior is sound: we provide every field P needs.
+                {...(props as P)}
+                setAssetStatus={setAssetStatus}
+                assetKey={assetKey}
+                ref={ref}
+            />
+        );
+    });
+
+    WithAssetContextComponent.displayName = `withAssetContext(${
+        WrappedComponent.displayName || WrappedComponent.name || "Component"
+    })`;
+
+    return WithAssetContextComponent;
+}

packages/perseus/src/widgets/matcher/__docs__/matcher-initial-state-regression.stories.tsx

@@ -57,30 +57,28 @@
     },
 };
 
-// TODO: Test flaky, commenting until it's fixed
-// Verifies TeX rendering in both column labels and item cards. The 500ms delay
-// gives MathJax time to finish rendering and onMeasure to settle before
-// Chromatic takes its snapshot.
-// export const WithTeX: Story = {
-//     decorators: [matcherRendererDecorator],
-//     parameters: {
-//         chromatic: {delay: 1000},
-//     },
-//     args: {
-//         ...sharedArgs,
-//         labels: ["$f(x)$", "$f'(x)$"],
-//         left: [
-//             "$f(x) = \\dfrac{1}{x}$",
-//             "$f(x) = \\dfrac{1}{x^2}$",
-//             "$f(x) = \\dfrac{1}{x^3}$",
-//         ],
-//         right: [
-//             "$f'(x) = -\\dfrac{1}{x^2}$",
-//             "$f'(x) = -\\dfrac{2}{x^3}$",
-//             "$f'(x) = -\\dfrac{3}{x^4}$",
-//         ],
-//     },
-// };
+// Verifies TeX rendering in both column labels and item cards. Matcher
+// coordinates AssetContext, marking itself settled once both columns have
+// measured at heights consistent with the shared constraint — so Chromatic
+// snapshots only fire after the measurement cascade settles, no manual
+// delay required.
+export const WithTeX: Story = {
+    decorators: [matcherRendererDecorator],
+    args: {
+        ...sharedArgs,
+        labels: ["$f(x)$", "$f'(x)$"],
+        left: [
+            "$f(x) = \\dfrac{1}{x}$",
+            "$f(x) = \\dfrac{1}{x^2}$",
+            "$f(x) = \\dfrac{1}{x^3}$",
+        ],
+        right: [
+            "$f'(x) = -\\dfrac{1}{x^2}$",
+            "$f'(x) = -\\dfrac{2}{x^3}$",
+            "$f'(x) = -\\dfrac{3}{x^4}$",
+        ],
+    },
+};
 
 // Verifies the orderMatters state: both columns render as draggable cards
 // (white background, visible border) rather than the left column appearing

packages/perseus/src/widgets/matcher/matcher.test.tsx

@@ -1,5 +1,6 @@
 import {
     generateTestPerseusItem,
+    generateTestPerseusRenderer,
     splitPerseusItem,
 } from "@khanacademy/perseus-core";
 import {act} from "@testing-library/react";
@@ -31,6 +32,13 @@ describe("matcher widget", () => {
 
         jest.useRealTimers();
 
+        // Replace the real TeX dependency with a synchronous stub. Real TeX
+        // uses MathJax, which doesn't render reliably in jsdom and would
+        // leave Matcher stuck on its loading spinner (since it waits for
+        // `onRender` to fire before rendering its items). The stub fulfills
+        // the same contract — calling `onRender` once the component mounts —
+        // so the rest of the production code thinks TeX is ready and
+        // proceeds normally.
         jest.spyOn(Dependencies, "getDependencies").mockReturnValue({
             ...testDependencies,
             TeX: ({
@@ -216,4 +224,146 @@ describe("matcher widget", () => {
             expect(score).toHaveBeenAnsweredIncorrectly();
         });
     });
+
+    describe("AssetContext tracking", () => {
+        it("marks left column stable on mount when left options are empty", () => {
+            // Arrange
+            const question = generateTestPerseusRenderer({
+                content: "[[☃ matcher 1]]",
+                widgets: {
+                    "matcher 1": {
+                        type: "matcher",
+                        options: {
+                            labels: ["", ""],
+                            padding: true,
+                            orderMatters: false,
+                            left: [],
+                            right: ["a", "b"],
+                        },
+                    },
+                },
+            });
+
+            // Act
+            const {renderer} = renderQuestion(question);
+            const matcher: Matcher = renderer.findWidgets("matcher 1")[0];
+
+            // Assert
+            expect(matcher._leftStable).toBe(true);
+            expect(matcher._rightStable).toBe(false);
+        });
+
+        it("marks both columns stable on mount when both are empty", () => {
+            // Arrange
+            const question = generateTestPerseusRenderer({
+                content: "[[☃ matcher 1]]",
+                widgets: {
+                    "matcher 1": {
+                        type: "matcher",
+                        options: {
+                            labels: ["", ""],
+                            padding: true,
+                            orderMatters: false,
+                            left: [],
+                            right: [],
+                        },
+                    },
+                },
+            });
+
+            // Act
+            const {renderer} = renderQuestion(question);
+            const matcher: Matcher = renderer.findWidgets("matcher 1")[0];
+
+            // Assert
+            expect(matcher._leftStable).toBe(true);
+            expect(matcher._rightStable).toBe(true);
+        });
+
+        it("does not mark sides stable on first onMeasure when heights differ from initial constraint", () => {
+            // Arrange — fresh render; matcher state is leftHeight=0, rightHeight=0
+            // before any measurement. currentConstraint = max(0, 0) = 0.
+            const {renderer} = renderQuestion(question1);
+            const matcher: Matcher = renderer.findWidgets("matcher 1")[0];
+            // Force-reset stability flags (the natural cascade may have already
+            // started by the time we get here).
+            matcher._leftStable = false;
+            matcher._rightStable = false;
+
+            // Act — fire onMeasureLeft with a non-zero height against the zero
+            // initial constraint. height (74) !== currentConstraint (0).
+            act(() => {
+                matcher.onMeasureLeft({heights: [74, 74, 74], widths: []});
+            });
+
+            // Assert — first cycle is not stable; the `currentConstraint > 0`
+            // guard prevents premature settlement.
+            expect(matcher._leftStable).toBe(false);
+        });
+
+        it("receives an asset key prefixed with 'matcher-' from the HOC", () => {
+            // Arrange, Act
+            const {renderer} = renderQuestion(question1);
+            const matcher: Matcher = renderer.findWidgets("matcher 1")[0];
+
+            // Assert
+            expect(matcher.props.assetKey).toMatch(/^matcher-/);
+        });
+
+        it("generates a unique asset key per matcher instance", () => {
+            // Arrange, Act — render the same question twice to compare keys
+            // across separate Matcher instances.
+            const {renderer: firstRenderer} = renderQuestion(question1);
+            const firstMatcher: Matcher =
+                firstRenderer.findWidgets("matcher 1")[0];
+            const {renderer: secondRenderer} = renderQuestion(question1);
+            const secondMatcher: Matcher =
+                secondRenderer.findWidgets("matcher 1")[0];
+
+            // Assert
+            expect(firstMatcher.props.assetKey).not.toBe(
+                secondMatcher.props.assetKey,
+            );
+        });
+
+        it("settles the asset on unmount", () => {
+            // Arrange
+            const {renderer, unmount} = renderQuestion(question1);
+            const matcher: Matcher = renderer.findWidgets("matcher 1")[0];
+            const setAssetStatusSpy = jest.spyOn(matcher, "_setAssetStatus");
+
+            // Act
+            unmount();
+
+            // Assert
+            expect(setAssetStatusSpy).toHaveBeenCalledWith(true);
+        });
+
+        it("marks both sides stable and settles the asset when measurements match the constraint", () => {
+            // Arrange — get the matcher and prep its state to simulate
+            // a converged constraint (post-cycle-1).
+            const {renderer} = renderQuestion(question1);
+            const matcher: Matcher = renderer.findWidgets("matcher 1")[0];
+            const setAssetStatusSpy = jest.spyOn(matcher, "_setAssetStatus");
+            matcher._leftStable = false;
+            matcher._rightStable = false;
+            act(() => {
+                matcher.setState({leftHeight: 74, rightHeight: 74});
+            });
+
+            // Act — fire both onMeasures with heights matching the constraint,
+            // mirroring what cycle 2 of the natural cascade looks like.
+            act(() => {
+                matcher.onMeasureLeft({heights: [74, 74, 74], widths: []});
+            });
+            act(() => {
+                matcher.onMeasureRight({heights: [74, 74, 74], widths: []});
+            });
+
+            // Assert — both sides now stable, settle was triggered.
+            expect(matcher._leftStable).toBe(true);
+            expect(matcher._rightStable).toBe(true);
+            expect(setAssetStatusSpy).toHaveBeenCalledWith(true);
+        });
+    });
 });