Encapsulate ServerItemRenderer to limit its external API by handeyeco · Pull Request #4126 · Khan/perseus

handeyeco · 2026-08-26T15:58:03Z

Summary:

I had a random idea this morning: what if we just wrap ServerItemRenderer (SIR) in a forwardRef and use useImperativeHandle to explicitly declare its external API? Turns out we were already wrapping SIR in a forwardRef so all I had to do was (1) stop exporting the inner SIR and (2) explicitly declare the API based on what we're actually using externally.

Before the external API was something like (not including the methods starting with _ like _setCurrentFocus):

// methods
focus
blur
focusPath
blurPath
getDOMNodeForPath
getInputPaths
getNumHints
getPromptJSON
getUserInput
getWidgetIds
getSerializedState

// properties
questionRenderer
props

Now it's:

// methods
focus
blur
getPromptJSON
getUserInput
getWidgetIds
getSerializedState

// properties
questionRenderer

And the methods starting with _ are not accessible at all.

github-actions · 2026-08-26T16:20:37Z

npm Snapshot: Published

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

Example:

pnpm add @khanacademy/perseus@PR4126

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

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

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

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

Want another snapshot? Comment /snapshot again.

github-actions · 2026-08-26T16:26:55Z

Size Change: +145 B (+0.03%)

Total Size: 520 kB

📦 View Changed

Filename	Size	Change
`packages/perseus/dist/es/index.js`	203 kB	+145 B (+0.07%)

ℹ️ View Unchanged

Filename	Size
`packages/kas/dist/es/index.js`	20.6 kB
`packages/keypad-context/dist/es/index.js`	1.07 kB
`packages/kmath/dist/es/index.js`	6.31 kB
`packages/math-input/dist/es/index.js`	98.6 kB
`packages/math-input/dist/es/strings.js`	1.63 kB
`packages/perseus-core/dist/es/index.item-splitting.js`	13.2 kB
`packages/perseus-core/dist/es/index.js`	28.8 kB
`packages/perseus-editor/dist/es/index.js`	106 kB
`packages/perseus-linter/dist/es/index.js`	10.1 kB
`packages/perseus-score/dist/es/index.js`	9.91 kB
`packages/perseus-utils/dist/es/index.js`	403 B
`packages/perseus/dist/es/strings.js`	12.9 kB
`packages/pure-markdown/dist/es/index.js`	1.39 kB
`packages/simple-markdown/dist/es/index.js`	6.12 kB

_{compressed-size-action}

handeyeco · 2026-08-26T19:07:38Z

    // TODO(LEMS-3185): remove serializedState
    blur(): void;
    focus(): boolean | null | undefined;
-    props: any;


This isn't an API that Renderers need to implement, it's just part of React.

handeyeco · 2026-08-26T19:09:15Z

-        const {renderer} = renderQuestion(itemWithMockWidget);
-
-        // Act
-        const node = renderer.getDOMNodeForPath(["mock-widget 1"]);


This is no longer an external API. It gets exercised when testing the APIs that use it.

handeyeco · 2026-08-26T19:09:34Z

-        });
-
-        // Act
-        const numHints = renderer.getNumHints();


getNumHints didn't seem to be used by anything.

handeyeco · 2026-08-26T19:11:51Z

-            </RenderStateRoot>,
+            <LoadingContext.Provider value={{onRendered}}>
+                <RenderStateRoot>
+                    <ServerItemRenderer


The functional ServerItemRenderer wrapper was originally made to consumer LoadingContext. This PR stops exporting the unwrapped, class-based ServerItemRenderer. So to test this functionality we need to use the context provider (which is more realistic anyway).

handeyeco · 2026-08-26T19:12:22Z

-            });
-
-            // Act
-            act(() => renderer.focusPath(["mock-widget 1"]));


This is no longer an external API. It gets exercised when testing the APIs that use it.

handeyeco · 2026-08-26T19:13:02Z

 class Renderer
    extends React.Component<Props, State>
-    implements GetPromptJSONInterface
+    implements RendererInterface, GetPromptJSONInterface


I just thought this made sense...

handeyeco · 2026-08-26T19:29:39Z

+    React.useImperativeHandle(
+        ref,
+        () => {
+            const instance = (): ServerItemRenderer => {


A lot of this complexity can go away when we switch SIR entirely to a functional component.

jeremywiebe

Love it!

jeremywiebe · 2026-08-26T20:42:58Z

+            };
+
+            return {
+                focus: () => instance().focus(),


Do you think the invariant is worthwhile over just doing this? (I guess I'm asking, do you think it's necessary to throw for unmounted SIRs?)

Suggested change

focus: () => instance().focus(),

focus: () => innerRef.current?.focus(),

I pushed back against the AI on this too. I think the reason it insists on doing this is type narrowing. The return type for getUserInput is UserInputMap, but doing innerRef.current?.getUserInput() changes it to UserInputMap | undefined. Using invariant narrows the type so innerRef.current can't be undefined.

I could do innerRef.current!.getUserInput() but at least with invariant we get a good error message if something goes horribly wrong. Open to push-back.

EDIT: and also this will go away when we switch SIR to a functional component.

No, I don't have such a strong feeling about it. I see now what it's avoiding and that makes sense. I'll be happy when SIR is functional.

handeyeco · 2026-08-27T14:23:53Z

/snapshot

…terface

… ServerItemRenderer to only allow explicitly declared imperative APIS

handeyeco · 2026-08-27T16:46:36Z

After making a ZND with the snapshot, there were some issues with infinite rerenders. I'm working on resolving these issues (see #4133)

handeyeco · 2026-08-27T17:45:19Z

/snapshot

…text' into server-item-renderer-imperative-handle

handeyeco self-assigned this Aug 26, 2026

handeyeco added the olc-5.0.83442 label Aug 26, 2026

handeyeco changed the title ~~Remove props from RendererInterface~~ Encapsulate ServerItemRenderer to limit its external API Aug 26, 2026

handeyeco commented Aug 26, 2026

View reviewed changes

handeyeco marked this pull request as ready for review August 26, 2026 20:02

handeyeco requested review from Myranae and nishasy as code owners August 26, 2026 20:02

handeyeco requested review from a team, jeremywiebe and mark-fitzgerald August 26, 2026 20:02

jeremywiebe approved these changes Aug 26, 2026

View reviewed changes

mark-fitzgerald approved these changes Aug 26, 2026

View reviewed changes

handeyeco changed the base branch from main to ref-keypad-context August 27, 2026 16:40

handeyeco added 10 commits August 27, 2026 11:43

[server-item-renderer-imperative-handle] Add ServerItemRenderer handle

6332ba2

[server-item-renderer-imperative-handle] remove unused APIs

2ef7e4d

[server-item-renderer-imperative-handle] restore eslint exception

ba21b03

[server-item-renderer-imperative-handle] don't export inner SIR

39bb24b

[server-item-renderer-imperative-handle] Remove props from RendererIn…

71ce9b5

…terface

[server-item-renderer-imperative-handle] some cleanup

39e6ef1

[server-item-renderer-imperative-handle] add comments

316ccc6

[server-item-renderer-imperative-handle] docs(changeset): Encapsulate…

c6cb744

… ServerItemRenderer to only allow explicitly declared imperative APIS

[server-item-renderer-imperative-handle] undo exported type

e467550

[server-item-renderer-imperative-handle] fix issue with rerenders

4c538ce

handeyeco force-pushed the server-item-renderer-imperative-handle branch from 516c513 to 4c538ce Compare August 27, 2026 16:44

handeyeco mentioned this pull request Sep 3, 2026

Switch from state to ref for keypad context #4133

Open

[server-item-renderer-imperative-handle] Merge branch 'ref-keypad-con…

7321cbd

…text' into server-item-renderer-imperative-handle

	focus: () => instance().focus(),
	focus: () => innerRef.current?.focus(),

Conversation

handeyeco commented Aug 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary:

Uh oh!

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

Uh oh!

npm Snapshot: Published

Uh oh!

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

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jeremywiebe left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

handeyeco Aug 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

handeyeco commented Aug 27, 2026

Uh oh!

handeyeco commented Aug 27, 2026

Uh oh!

handeyeco commented Aug 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

handeyeco commented Aug 26, 2026 •

edited

Loading

github-actions Bot commented Aug 26, 2026 •

edited

Loading

github-actions Bot commented Aug 26, 2026 •

edited

Loading

handeyeco Aug 27, 2026 •

edited

Loading