remove use of findDOMNode in number-input

[handeyeco]

Summary:

When running tests, I was tired of seeing:

Warning: findDOMNode is deprecated and will be removed in the next major release. Instead, add a ref directly to the element you want to reference. Learn more about using refs safely here: https://reactjs.org/link/strict-mode-find-node

So I thought I'd start chipping away at them.

Test plan:

Nothing should change, just an implementation detail.

[khan-actions-bot]

Gerald

Required Reviewers

• @Khan/perseus for changes to .changeset/chilly-carrots-wink.md, packages/perseus/src/components/number-input.tsx

---

Don't want to be involved in this pull request? Comment #removeme and we won't notify you of further changes.

[github-actions]

npm Snapshot: Published

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

Example:

yarn add @khanacademy/perseus@PR1915

If you are working in Khan Academy's webapp, you can run:

./dev/tools/bump_perseus_version.sh -t PR1915

[github-actions]

Size Change: -121 B (-0.01%)

Total Size: 1.29 MB

Filename	Size	Change
packages/perseus/dist/es/index.js	421 kB	-121 B (-0.03%)

ℹ️ View Unchanged

Filename	Size
packages/kas/dist/es/index.js	39 kB
packages/keypad-context/dist/es/index.js	760 B
packages/kmath/dist/es/index.js	4.27 kB
packages/math-input/dist/es/index.js	77.9 kB
packages/math-input/dist/es/strings.js	1.79 kB
packages/perseus-core/dist/es/index.js	1.48 kB
packages/perseus-editor/dist/es/index.js	698 kB
packages/perseus-linter/dist/es/index.js	22.2 kB
packages/perseus/dist/es/strings.js	3.68 kB
packages/pure-markdown/dist/es/index.js	3.66 kB
packages/simple-markdown/dist/es/index.js	12.5 kB

_{compressed-size-action}

[jeremywiebe]

Nice!

[jeremywiebe]

I like this migration to use a real Ref. One concern is that this function could get called by something after dismounting and we start throwing errors. Perhaps that's fine as an unmounted component throwing errors shouldn't tear down the React tree, right?

[handeyeco]

Yeah, I was a little worried about doing this, but I think it's roughly the same from the learner's perspective if we throw an error intentionally vs getting an error from trying to access a property on an unset value.

For example, the before:

        // @ts-expect-error - TS2531 - Object is possibly 'null'
        return ReactDOM.findDOMNode(this.refs.input).value.toString();

So here we'd get an error when we try to access .value (because I believe findDOMNode would return null). Now we'd still throw an error, but we'd be doing it intentionally and can provide an error message.

[nishasy]

I don't think I've seen this approach in Perseus? Can we not just do a this.inputRef.current?.focus(); and leave it at that rather than using a _getInput() function?

[handeyeco]

That would require changing the logic and the return types of all the methods that weren't properly checking the ref before. I was worried that would spiral out of control if I then had to handle undefined where number-input was consumed. We do it regularly in Webapp:

        const itemRenderer = itemRendererRef.current;
        if (itemRenderer == null) {
            // NOTE(kevinb): this should never happen since the itemRenderer
            // and the exercise chrome should be unmounted at the same time.
            throw new KAError(
                "cannot check answer on null item renderer",
                Errors.Internal,
            );
        }
        const score = itemRenderer.scoreInput();