remove staticRender #421

handeyeco · 2023-03-23T18:38:02Z

Summary:

I ran across staticRender in ApiOptions while working on Expression. I couldn't figure out what it was for and, after digging in a bit, realized it was meant to be depreciated years ago.

I couldn't find anything that was setting staticRender to true (or anything that could set it).

See this rambling: https://khanacademy.slack.com/archives/C01AZ9H8TTQ/p1679519079246219

A highlight from 2016:

At this point, it doesn't feel like it's worth changing, since we'll hopefully remove staticRender as soon as we're using the customKeypad everywhere.

Test plan:

Nothing should change in webapp/mobile

changeset-bot · 2023-03-23T18:38:05Z

🦋 Changeset detected

Latest commit: ad36c5a

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages

Name	Type
@khanacademy/perseus	Major
@khanacademy/perseus-editor	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

khan-actions-bot · 2023-03-23T18:38:25Z

Gerald

Required Reviewers

@Khan/perseus for changes to .changeset/fresh-rivers-accept.md, .changeset/sweet-days-reflect.md, packages/perseus/src/hint-renderer.jsx, packages/perseus/src/perseus-api.jsx, packages/perseus/src/types.js, packages/perseus/src/widgets/expression.jsx, packages/perseus/src/widgets/input-number.jsx, packages/perseus/src/widgets/matrix.jsx, packages/perseus/src/widgets/number-line.jsx, packages/perseus/src/widgets/numeric-input.jsx, packages/perseus/src/widgets/simulator.jsx, packages/perseus/src/widgets/table.jsx, packages/perseus/src/widgets/transformer.jsx, packages/perseus/src/widgets/unit.jsx, packages/perseus/src/widgets/__stories__/unit.stories.jsx, packages/perseus/src/widgets/__tests__/expression_test.jsx, packages/perseus/src/widgets/__tests__/input-number_test.jsx, packages/perseus/src/widgets/__tests__/numeric-input_test.jsx, packages/perseus/src/widgets/__tests__/unit_test.jsx, packages/perseus/src/widgets/radio/choice.jsx, packages/perseus/src/widgets/__tests__/__snapshots__/input-number_test.jsx.snap, packages/perseus/src/widgets/__tests__/__snapshots__/unit_test.jsx.snap

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

codecov · 2023-03-23T18:40:39Z

Codecov Report

❗ No coverage uploaded for pull request base (main@1fe3786). Click here to learn what that means.
The diff coverage is 0.00%.

❗ Current head 4b79600 differs from pull request most recent head ad36c5a. Consider uploading reports for the commit ad36c5a to get more accurate results

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #421   +/-   ##
=======================================
  Coverage        ?   28.52%           
=======================================
  Files           ?      304           
  Lines           ?    25887           
  Branches        ?     6240           
=======================================
  Hits            ?     7384           
  Misses          ?    18503           
  Partials        ?        0

Impacted Files	Coverage Δ
packages/perseus/src/hint-renderer.jsx	`9.09% <ø> (ø)`
packages/perseus/src/perseus-api.jsx	`44.44% <ø> (ø)`
packages/perseus/src/widgets/expression.jsx	`3.12% <0.00%> (ø)`
packages/perseus/src/widgets/input-number.jsx	`5.94% <0.00%> (ø)`
packages/perseus/src/widgets/matrix.jsx	`5.96% <ø> (ø)`
packages/perseus/src/widgets/number-line.jsx	`4.60% <ø> (ø)`
packages/perseus/src/widgets/numeric-input.jsx	`3.70% <ø> (ø)`
packages/perseus/src/widgets/radio/choice.jsx	`11.11% <0.00%> (ø)`
packages/perseus/src/widgets/simulator.jsx	`4.76% <ø> (ø)`
packages/perseus/src/widgets/table.jsx	`10.41% <ø> (ø)`
... and 2 more

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1fe3786...ad36c5a. Read the comment docs.

github-actions · 2023-03-23T18:41:27Z

Size Change: +280 B (0%)

Total Size: 635 kB

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

ℹ️ View Unchanged

Filename	Size
`packages/kas/dist/es/index.js`	38.4 kB
`packages/kmath/dist/es/index.js`	4.18 kB
`packages/math-input/dist/es/index.js`	60.2 kB
`packages/perseus-editor/dist/es/index.js`	112 kB
`packages/perseus-error/dist/es/index.js`	825 B
`packages/perseus-linter/dist/es/index.js`	18.6 kB
`packages/pure-markdown/dist/es/index.js`	3.74 kB
`packages/simple-markdown/dist/es/index.js`	12.9 kB

_{compressed-size-action}

jeremywiebe

I love this change. I think the unit widget might have been semi-broken and now completely broken with this refactoring. Requesting changes to add a unit test with RTL or a Story to validate it actually renders.

jeremywiebe · 2023-03-23T19:03:16Z

packages/perseus/src/widgets/input-number.jsx

-    _getInputType: () => "tex" | "text" = () => {
-        if (this.props.apiOptions.staticRender) {
-            return "tex";
-        }
-        return "text";
-    };
-


Oh, I'm really happy to see this go!!!

jeremywiebe · 2023-03-23T19:04:35Z

packages/perseus/src/widgets/radio/choice.jsx

-                            apiOptions.staticRender ||
-                            apiOptions.readOnly


These two options always felt suspiciously similar anyways!

jeremywiebe · 2023-03-23T19:08:08Z

packages/perseus/src/widgets/unit.jsx

-                onBlur={this.handleBlur}
-            />
-        ) : (
+        const input = (


🤔 I think this is now a cycle. The input on line 182 seems to refer to itself (line 181). Can you maybe do a super-simple unit test or story to see if this component actually renders?

Would you call it a Unit test? Because it's the Unit widget? 😅

#423

It was working in Storybook as-is, but I see what you mean. Removed that cycle, added a unit test, and cleaned up the component while I was there.

Would you call it a Unit test?

Well played! 😂

## Summary: Follow-up to Jeremy's feedback in #421 - Add unit test to make sure Unit widget renders - Reorganize Unit to conform to the React ordering linting thing Author: handeyeco Reviewers: kevinbarabash, jeremywiebe Required Reviewers: Approved By: kevinbarabash, jeremywiebe Checks: ✅ codecov/project, ❌ Publish npm snapshot (ubuntu-latest, 16.x), ✅ Lint, Flow, and Test (ubuntu-latest, 16.x), ✅ Extract i18n strings (ubuntu-latest, 16.x), ✅ Cypress Coverage (ubuntu-latest, 16.x), ✅ Check for .changeset file (ubuntu-latest, 16.x), ✅ Check builds for changes in size (ubuntu-latest, 16.x), ✅ gerald, ✅ Jest Coverage (ubuntu-latest, 16.x) Pull Request URL: #423

github-actions · 2023-03-24T14:29:52Z

npm Snapshot: Published

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

Example:

yarn add @khanacademy/perseus@PR421

remove staticRender

6fe3a0b

handeyeco self-assigned this Mar 23, 2023

khan-actions-bot requested a review from a team March 23, 2023 18:38

handeyeco added 4 commits March 23, 2023 13:42

add comment back

9100f4c

changeset

15fbe9b

simplify number input classes

2a392db

remove brackets

4b79600

jeremywiebe requested changes Mar 23, 2023

View reviewed changes

handeyeco mentioned this pull request Mar 23, 2023

add test for Unit widget #423

Merged

jeremywiebe approved these changes Mar 23, 2023

View reviewed changes

handeyeco merged commit 8ca2a1a into main Mar 24, 2023

handeyeco deleted the remove-static-render branch March 24, 2023 14:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

remove staticRender #421

remove staticRender #421

handeyeco commented Mar 23, 2023 •

edited

changeset-bot bot commented Mar 23, 2023 •

edited

khan-actions-bot commented Mar 23, 2023 •

edited

codecov bot commented Mar 23, 2023 •

edited

github-actions bot commented Mar 23, 2023 •

edited

jeremywiebe left a comment •

edited

jeremywiebe Mar 23, 2023

jeremywiebe Mar 23, 2023

jeremywiebe Mar 23, 2023

handeyeco Mar 23, 2023

jeremywiebe Mar 23, 2023 •

edited

github-actions bot commented Mar 24, 2023

remove staticRender #421

remove staticRender #421

Conversation

handeyeco commented Mar 23, 2023 • edited

Summary:

Test plan:

changeset-bot bot commented Mar 23, 2023 • edited

🦋 Changeset detected

khan-actions-bot commented Mar 23, 2023 • edited

Gerald

codecov bot commented Mar 23, 2023 • edited

Codecov Report

github-actions bot commented Mar 23, 2023 • edited

jeremywiebe left a comment • edited

Choose a reason for hiding this comment

jeremywiebe Mar 23, 2023

Choose a reason for hiding this comment

jeremywiebe Mar 23, 2023

Choose a reason for hiding this comment

jeremywiebe Mar 23, 2023

Choose a reason for hiding this comment

handeyeco Mar 23, 2023

Choose a reason for hiding this comment

jeremywiebe Mar 23, 2023 • edited

Choose a reason for hiding this comment

github-actions bot commented Mar 24, 2023

npm Snapshot: Published

handeyeco commented Mar 23, 2023 •

edited

changeset-bot bot commented Mar 23, 2023 •

edited

khan-actions-bot commented Mar 23, 2023 •

edited

codecov bot commented Mar 23, 2023 •

edited

github-actions bot commented Mar 23, 2023 •

edited

jeremywiebe left a comment •

edited

jeremywiebe Mar 23, 2023 •

edited