[DRAFT]: Remove unused code and usages of findDOMNode #2769

SonicScrewdriver · 2025-08-06T20:08:35Z

Summary:

This [DRAFT] PR is currently a proof of concept. I'm testing the results upstream, but please feel free to leave any comments or concerns about my approach.

The main goals of this PR are to:

Remove as many usages of findDOMNode as possible
Clean up string refs wherever related to these usages of findDOMNode
Improve any typing to be more specific, in an effort to slowly clean up our need for overrides

Changes Made

SimpleKeypadInput: Converted to forwardRef functional component with useImperativeHandle to expose DOM node directly
Table, Matrix, NumberLine widgets: Replaced string refs and findDOMNode calls with modern callback/object ref patterns
Type improvements: Updated input ref types and removed unnecessary type cast in NumericInput

Issue: LEMS-XXXX

Test plan:

Tests pass
Manual testing upstream
Upcoming QA pass

github-actions · 2025-08-06T20:10:16Z

🗄️ Schema Change: No Changes ✅

github-actions · 2025-08-06T20:10:47Z

Size Change: +3 B (0%)

Total Size: 496 kB

Filename	Size	Change
`packages/perseus/dist/es/index.js`	207 kB	+3 B (0%)

ℹ️ View Unchanged

Filename	Size
`packages/kas/dist/es/index.js`	20.7 kB
`packages/keypad-context/dist/es/index.js`	1 kB
`packages/kmath/dist/es/index.js`	5.98 kB
`packages/math-input/dist/es/index.js`	98.6 kB
`packages/math-input/dist/es/strings.js`	1.61 kB
`packages/perseus-core/dist/es/index.item-splitting.js`	12.9 kB
`packages/perseus-core/dist/es/index.js`	21.5 kB
`packages/perseus-editor/dist/es/index.js`	94.1 kB
`packages/perseus-linter/dist/es/index.js`	7.07 kB
`packages/perseus-score/dist/es/index.js`	9.34 kB
`packages/perseus-utils/dist/es/index.js`	403 B
`packages/perseus/dist/es/strings.js`	7.56 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 · 2025-08-06T20:10:47Z

🛠️ Item Splitting: No Changes ✅

github-actions · 2025-08-06T20:11:20Z

npm Snapshot: Published

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

Example:

pnpm add @khanacademy/perseus@PR2769

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

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

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

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

SonicScrewdriver · 2025-08-06T20:13:28Z

packages/perseus/src/widgets/matrix/matrix.tsx

@@ -462,7 +461,6 @@ class Matrix extends React.Component<Props, State> implements Widget {
                                            <SimpleKeypadInput
                                                {...inputProps}
                                                style={style}
-                                                scrollable={true}


This is a nonsense property that doesn't exist, so I've removed it! We simply didn't catch it before as SimpleKeypadInput wasn't very specific about props

SonicScrewdriver · 2025-08-06T20:14:14Z

packages/perseus/src/widgets/number-line/number-line.tsx

@@ -247,6 +246,8 @@ class NumberLine extends React.Component<Props, State> implements Widget {
    static contextType = PerseusI18nContext;
    declare context: React.ContextType<typeof PerseusI18nContext>;

+    tickControlRef: HTMLInputElement | null = null;


There's only ever one input in NumberLine, so I figured I'd simplify this accordingly.

SonicScrewdriver · 2025-08-06T20:16:12Z

packages/perseus/src/widgets/number-line/number-line.tsx

        }
    };

+    // There's only one input path for the tick control, but the renderer
+    // expects this method to be implemented.
    getInputPaths: () => ReadonlyArray<ReadonlyArray<string>> = () => {


Since these methods are part of our our main renderer logic, I didn't want to mess with them too much.

I think improving this will require a more holistic and modern approach to our focus handling across Perseus (perhaps a context?).

…d usages of findDOMNode

SonicScrewdriver · 2025-08-06T20:42:24Z

packages/perseus/src/widgets/matrix/matrix.tsx

-        // eslint-disable-next-line react/no-string-refs
-        const curInput = this.refs[getRefForPath(getInputPath(row, col))];
+        const inputID = getRefForPath(getInputPath(row, col));
+        const curInput = this.answerRefs[inputID];
        // @ts-expect-error - TS2339 - Property 'getStringValue' does not exist on type 'ReactInstance'.
        const curValueString = curInput.getStringValue();


I can't remove these ts-expect errors as the SimpleKeypadInput never supported these methods, although our TextInput does. (getStringValue, getSelectionStart, getSelectionEnd).

This means that none of this keyboard logic was ever working for our mobile users. However, they were also very unlikely to hit this as it requires hitting specific keys on their mobile device's keyboard.

…s onRender (testing)

SonicScrewdriver · 2025-08-07T15:41:39Z

packages/perseus/src/widgets/table/table.tsx

-        } else {
-            // @ts-expect-error - TS2531 - Object is possibly 'null'. | TS2339 - Property 'focus' does not exist on type 'Element | Text'.
-            ReactDOM.findDOMNode(inputComponent).focus();
-        }


This is the section of code that was able to be cleaned up by converting SimpleKeypadInput into the forward ref functional component

SonicScrewdriver · 2025-08-07T20:33:49Z

packages/perseus/src/widgets/number-line/number-line.tsx

-                        ref="tick-ctrl"
+                        ref={(ref) => {
+                            this.tickControlRef = ref;
+                        }}


Ooo I can simplify this

In my testing and investigations, I simply cannot find any case where this fallback is called.

SonicScrewdriver · 2025-08-08T17:59:43Z

packages/perseus/src/renderer.tsx

-            if (interWidgetPath.length === 0) {
-                // @ts-expect-error - TS2345 - Argument of type 'Widget | null | undefined' is not assignable to parameter of type 'ReactInstance | null | undefined'.
-                return ReactDOM.findDOMNode(widget);
-            }


We only ever seem to hit line 1536 when we are in a Graded Group, but the length is never zero. I've been unable to find any cases where we ever actually need this fallback. I tested every widget in storybook in both desktop/mobile views. The only place we ever satisfy this condition is when we're explicitly forcing the situation in tests.

SonicScrewdriver self-assigned this Aug 6, 2025

SonicScrewdriver added the olc-5.0.b27e5 label Aug 6, 2025

SonicScrewdriver changed the title ~~docs(changeset): Remove unused code and usages of findDOMNode~~ [DRAFT - Testing Spiciness]: Remove unused code and usages of findDOMNode Aug 6, 2025

github-actions bot added schema-change Attached to PRs when we detect Perseus Schema changes in it item-splitting-change and removed schema-change Attached to PRs when we detect Perseus Schema changes in it labels Aug 6, 2025

github-actions bot removed the item-splitting-change label Aug 6, 2025

SonicScrewdriver commented Aug 6, 2025

View reviewed changes

SonicScrewdriver added 4 commits August 6, 2025 13:34

[cleaning-the-perseus-finddom] Initial cleanup

462b977

[cleaning-the-perseus-finddom] SSSSOMEBODY STOP ME

0531062

[cleaning-the-perseus-finddom] docs(changeset): Remove unused code an…

504bb38

…d usages of findDOMNode

[cleaning-the-perseus-finddom] Remove cast

10b363c

SonicScrewdriver force-pushed the cleaning-the-perseus-finddom branch from 6010d1f to 10b363c Compare August 6, 2025 20:34

SonicScrewdriver commented Aug 6, 2025

View reviewed changes

[cleaning-the-perseus-finddom] Make sure we're still calling the prop…

ab60dc1

…s onRender (testing)

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 Aug 7, 2025

SonicScrewdriver commented Aug 7, 2025

View reviewed changes

SonicScrewdriver changed the title ~~[DRAFT - Testing Spiciness]: Remove unused code and usages of findDOMNode~~ [DRAFT]: Remove unused code and usages of findDOMNode Aug 7, 2025

github-actions bot added item-splitting-change and removed item-splitting-change labels Aug 7, 2025

SonicScrewdriver commented Aug 7, 2025

View reviewed changes

SonicScrewdriver added 2 commits August 7, 2025 14:49

[cleaning-the-perseus-finddom] Remove unused fallback

79b40f8

In my testing and investigations, I simply cannot find any case where this fallback is called.

[cleaning-the-perseus-finddom] No need for react dom

e9a8587

SonicScrewdriver commented Aug 8, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[DRAFT]: Remove unused code and usages of findDOMNode #2769

[DRAFT]: Remove unused code and usages of findDOMNode #2769

Uh oh!

SonicScrewdriver commented Aug 6, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Aug 6, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Aug 6, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Aug 6, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Aug 6, 2025 •

edited

Loading

Uh oh!

SonicScrewdriver Aug 6, 2025

Uh oh!

SonicScrewdriver Aug 6, 2025

Uh oh!

SonicScrewdriver Aug 6, 2025

Uh oh!

SonicScrewdriver Aug 6, 2025 •

edited

Loading

Uh oh!

SonicScrewdriver Aug 7, 2025

Uh oh!

SonicScrewdriver Aug 7, 2025

Uh oh!

SonicScrewdriver Aug 8, 2025

Uh oh!

Uh oh!

[DRAFT]: Remove unused code and usages of findDOMNode #2769

Are you sure you want to change the base?

[DRAFT]: Remove unused code and usages of findDOMNode #2769

Uh oh!

Conversation

SonicScrewdriver commented Aug 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary:

Changes Made

Test plan:

Uh oh!

github-actions bot commented Aug 6, 2025 • 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 Aug 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Aug 6, 2025 • 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 Aug 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

npm Snapshot: Published

Uh oh!

SonicScrewdriver Aug 6, 2025

Choose a reason for hiding this comment

Uh oh!

SonicScrewdriver Aug 6, 2025

Choose a reason for hiding this comment

Uh oh!

SonicScrewdriver Aug 6, 2025

Choose a reason for hiding this comment

Uh oh!

SonicScrewdriver Aug 6, 2025 • 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!

SonicScrewdriver Aug 7, 2025

Choose a reason for hiding this comment

Uh oh!

SonicScrewdriver Aug 7, 2025

Choose a reason for hiding this comment

Uh oh!

SonicScrewdriver Aug 8, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

SonicScrewdriver commented Aug 6, 2025 •

edited

Loading

github-actions bot commented Aug 6, 2025 •

edited

Loading

github-actions bot commented Aug 6, 2025 •

edited

Loading

github-actions bot commented Aug 6, 2025 •

edited

Loading

github-actions bot commented Aug 6, 2025 •

edited

Loading

SonicScrewdriver Aug 6, 2025 •

edited

Loading