[Preview NG] Introduce hooks to manage preview communication by jeremywiebe · Pull Request #3495 · Khan/perseus

jeremywiebe · 2026-04-15T16:58:33Z

Summary:

This PR is part of a series building a typed, hook-based preview system for the Perseus editor. The new system replaces the untyped window.iframeDataStore + raw postMessage(string) communication with structured, validated message passing via usePreviewController and usePreviewPresenter hooks. The new system is being built alongside the old one — no existing behavior changes until the final PR in the series flips the switch.

Previous PRs in this series:

Adds the two core hooks that implement the new preview communication protocol.

usePreviewController(iframeRef) is used on the editor/parent side. It sends sanitized preview data to an iframe via typed postMessage, listens for height updates back, and filters incoming messages by event.source to ensure they come from the correct iframe. Returns {sendData, height}.

usePreviewPresenter() is used inside the preview iframe. It listens for content data from the parent, sends back height reports and data requests via the typed message protocol. Throws if used outside an iframe. Returns {data, reportHeight}.

Both hooks use the message types and validators from #3474 and the data sanitizer from #3492. Purely additive — no existing behaviour is changed.

Issue: LEMS-3741

Test plan:

pnpm test packages/perseus-editor/src/preview/use-preview-controller.test.ts packages/perseus-editor/src/preview/use-preview-presenter.test.ts — 48 tests covering initialization, message sending/receiving, sanitization, event.source filtering, iframe validation, height reporting, error handling, and cleanup.

github-actions · 2026-04-15T16:59:55Z

🗄️ Schema Change: No Changes ✅

github-actions · 2026-04-15T17:00:03Z

Size Change: 0 B

Total Size: 502 kB

ℹ️ View Unchanged

Filename	Size
`packages/kas/dist/es/index.js`	20.6 kB
`packages/keypad-context/dist/es/index.js`	1 kB
`packages/kmath/dist/es/index.js`	6.36 kB
`packages/math-input/dist/es/index.js`	98.5 kB
`packages/math-input/dist/es/strings.js`	1.61 kB
`packages/perseus-core/dist/es/index.item-splitting.js`	12 kB
`packages/perseus-core/dist/es/index.js`	25.2 kB
`packages/perseus-editor/dist/es/index.js`	103 kB
`packages/perseus-linter/dist/es/index.js`	9.42 kB
`packages/perseus-score/dist/es/index.js`	9.78 kB
`packages/perseus-utils/dist/es/index.js`	403 B
`packages/perseus/dist/es/index.js`	198 kB
`packages/perseus/dist/es/strings.js`	8.46 kB
`packages/pure-markdown/dist/es/index.js`	1.39 kB
`packages/simple-markdown/dist/es/index.js`	6.71 kB

_{compressed-size-action}

github-actions · 2026-04-15T17:00:09Z

🛠️ Item Splitting: No Changes ✅

github-actions · 2026-04-15T17:00:53Z

npm Snapshot: Published

Good news!! We've packaged up the latest commit from this PR (fafe8ac) and published it to npm. You
can install it using the tag PR3495.

Example:

pnpm add @khanacademy/perseus@PR3495

If you are working in Khan Academy's frontend, you can run the below command.

./dev/tools/bump_perseus_version.ts -t PR3495

If you are working in Khan Academy's webapp, you can run the below command.

./dev/tools/bump_perseus_version.js -t PR3495

claude

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.

_{Tip: disable this comment in your organization's Code Review settings.}

benchristel

Thanks for doing this! Our iframe communication code has been neglected for a long time.

I still need to do a thorough review of the tests. My overall thoughts at this point are that we're trying to use AI to redesign a big chunk of existing code, and I don't think AI is good at this type of work. Hopefully my inline comments demonstrate why.

It seems like a lot of things could be simplified if we rewrote this by hand with a similar architecture to what's proposed here. Happy to pair on it if you want!

benchristel · 2026-04-27T14:42:11Z

+        // iframeRef is intentionally excluded - it's a stable ref that shouldn't trigger re-runs
+        // eslint-disable-next-line react-hooks/exhaustive-deps


iframeRef is intentionally excluded - it's a stable ref that shouldn't trigger re-runs

This hook can't guarantee that iframeRef is actually a stable ref, since the caller can pass anything with a current: HTMLIFrameElement property. I'd prefer to add iframeRef to the deps — if it is a stable reference, it won't trigger re-renders, and if it's not, we get predictable behavior (and no linter suppression).

benchristel · 2026-04-27T15:00:01Z

+     * The iframe's unique identifier (from data-id attribute). Use for
+     * debugging/logging, but not for message routing.


Why is it not okay to use the id for message routing?

Actually, thinking about this again, I don't think that we need an id at all.

The usePreviewController hook accepts an iframe ref. That ref is what's used to send messages down into the iframe. Messages that are posted back to the parent are filtered against the hook's instance of the iframe ref (if (event.source !== iframeRef.current?.contentWindow) { ... }) so we already have filtering in both directions.

I also checked and we don't log this ID anywhere.

benchristel · 2026-04-27T15:20:03Z

+    React.useEffect(() => {
+        const iframe = window.frameElement as HTMLIFrameElement | null;
+        if (iframe == null) {
+            throw new Error(
+                "usePreviewPresenter must be used within an iframe",
+            );
+        }
+
+        // ID is used for debugging/logging, not message routing
+        const id = iframe.dataset.id;
+        const mobile = iframe.dataset.mobile === "true";
+        const lintGutter = iframe.dataset.lintGutter === "true";
+
+        if (id == null) {
+            throw new Error(
+                "usePreviewPresenter could not identify its id from the hosting iframe",
+            );
+        }
+
+        setIframeId(id);
+        setIsMobile(mobile);
+        setHasLintGutter(lintGutter);
+    }, []);


Does this need to be a useEffect? window.frameElement should always exist if this code is running in an iframe — we don't need to wait for any React rendering to happen before getting the dataset attributes.

I think we could get rid of the useState calls above and just return the values from this hook:

const iframe = window.frameElement as HTMLIFrameElement | null; // ... return { data, isMobile: iframe.dataset.mobile === "true", hasLintGutter: iframe.dataset.lintGutter === "true", id: iframe.dataset.id, reportHeight, reportLintWarnings, };

benchristel · 2026-04-27T15:22:24Z

+            }
+
+            // Handle content data
+            // Note: ID check is for extra validation/debugging; actual routing is by event.source


Note: ID check is for extra validation/debugging

Do we need the ID check, then? Routing by event.source is sufficient and correct. Also, there's no debugging code here, so it's not clear what that part of the comment refers to.

jeremywiebe · 2026-04-28T15:09:02Z

- * not by comparing ID strings.
- */
-interface PreviewMessageWithId extends PreviewMessageBase {
-    id: string;


Based on Ben's question about why the ID shouldn't be used for routing, I realized that the "gating" by message source that we do in the message handlers is sufficient. The id is just an extra value that doesn't add any further safety and so is just extra work for no benefit.

…sePreviewClient hooks with tests

…ntroller/Presenter

… in test file

…() calls

…eRef to the deps list

…viewControllerResult

…reviewPresenterResult

…sing switch case)

jeremywiebe self-assigned this Apr 15, 2026

jeremywiebe added the olc-5.0.23be5 label Apr 15, 2026

jeremywiebe force-pushed the jer/preview-ng-5-hooks branch from 16904a3 to 818c218 Compare April 15, 2026 19:20

jeremywiebe force-pushed the jer/preview-ng-4-sanitize-api-options branch from d534a50 to 0419ad1 Compare April 15, 2026 19:27

jeremywiebe force-pushed the jer/preview-ng-5-hooks branch from 818c218 to feb3f11 Compare April 15, 2026 19:27

This was referenced Apr 15, 2026

[Preview NG] Export preview system hooks from @khanacademy/perseus-editor #3499

Open

[Preview NG] Migrate storybook preview page to usePreviewPresenter hook #3500

Draft

Base automatically changed from jer/preview-ng-4-sanitize-api-options to main April 16, 2026 22:52

jeremywiebe force-pushed the jer/preview-ng-5-hooks branch from feb3f11 to 9b6a03d Compare April 16, 2026 22:57

jeremywiebe marked this pull request as ready for review April 20, 2026 17:35

claude Bot reviewed Apr 20, 2026

View reviewed changes

jeremywiebe requested review from a team, catandthemachines and mark-fitzgerald April 20, 2026 17:37

benchristel reviewed Apr 27, 2026

View reviewed changes

jeremywiebe force-pushed the jer/preview-ng-5-hooks branch from 9f775ca to c555bef Compare April 27, 2026 23:37

github-actions Bot added item-splitting-change schema-change Attached to PRs when we detect Perseus Schema changes in it and removed schema-change Attached to PRs when we detect Perseus Schema changes in it item-splitting-change labels Apr 27, 2026

jeremywiebe commented Apr 28, 2026

View reviewed changes

jeremywiebe added 2 commits April 28, 2026 08:53

[jer/preview-ng-5-hooks] [jer/preview-hooks] Add usePreviewHost and u…

07cffa5

…sePreviewClient hooks with tests

[jer/preview-ng-5-hooks] Rename usePreviewHost/Client to usePreviewCo…

d11038a

…ntroller/Presenter

jeremywiebe added 13 commits April 28, 2026 08:53

[jer/preview-ng-5-hooks] Unify duplicate createQuestionPreview helper…

0166141

… in test file

[jer/preview-ng-5-hooks] Formatting

d899b88

[jer/preview-ng-5-hooks] Inline iframe (only used once)

387ba9d

[jer/preview-ng-5-hooks] Use '/' for targetOrigin for all postMessage…

ea682e2

…() calls

[jer/preview-ng-5-hooks] Remove ESLint suppression and just add ifram…

5c02f8e

…eRef to the deps list

[jer/preview-ng-5-hooks] Fix naming of UsePreviewHostResult to UsePre…

90e733d

…viewControllerResult

[jer/preview-ng-5-hooks] Remove unused 'lint report' message

c6edd36

[jer/preview-ng-5-hooks] Fix naming of UsePreviewClientResult to UseP…

8a6622f

…reviewPresenterResult

[jer/preview-ng-5-hooks] Inline iframe var

f7dfac7

[jer/preview-ng-5-hooks] Fix up example in docs

30acc29

[jer/preview-ng-5-hooks] Remove message id: not actually useful/needed

7853b6b

[jer/preview-ng-5-hooks] Make each hook's message handler the same (u…

889e884

…sing switch case)

[jer/preview-ng-5-hooks] Fixups

fafe8ac

jeremywiebe force-pushed the jer/preview-ng-5-hooks branch from 3d272ea to fafe8ac Compare April 28, 2026 16:09

		// iframeRef is intentionally excluded - it's a stable ref that shouldn't trigger re-runs
		// eslint-disable-next-line react-hooks/exhaustive-deps

		* The iframe's unique identifier (from data-id attribute). Use for
		* debugging/logging, but not for message routing.

Conversation

jeremywiebe commented Apr 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary:

Test plan:

Uh oh!

github-actions Bot commented Apr 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🗄️ Schema Change: No Changes ✅

Uh oh!

github-actions Bot commented Apr 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Apr 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🛠️ Item Splitting: No Changes ✅

Uh oh!

github-actions Bot commented Apr 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

npm Snapshot: Published

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Claude Code Review

Uh oh!

benchristel left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

benchristel Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

benchristel Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

jeremywiebe Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

benchristel Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

benchristel Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

jeremywiebe Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jeremywiebe commented Apr 15, 2026 •

edited

Loading

github-actions Bot commented Apr 15, 2026 •

edited

Loading

github-actions Bot commented Apr 15, 2026 •

edited

Loading

github-actions Bot commented Apr 15, 2026 •

edited

Loading

github-actions Bot commented Apr 15, 2026 •

edited

Loading