Use WB View instead of fake-react-native-web View #3055
Conversation
🗄️ Schema Change: No Changes ✅ |
🛠️ Item Splitting: No Changes ✅ |
|
Size Change: -379 B (-0.08%) Total Size: 498 kB
ℹ️ View Unchanged
|
ivyolamit
force-pushed
the
LEMS-3361/refactor-fake-react-native-web
branch
2 times, most recently
from
0b3c9bb to
928a055
Compare
npm Snapshot: PublishedGood news!! We've packaged up the latest commit from this PR (fdd906c) and published it to npm. You Example: pnpm add @khanacademy/perseus@PR3055If you are working in Khan Academy's frontend, you can run the below command. ./dev/tools/bump_perseus_version.ts -t PR3055If you are working in Khan Academy's webapp, you can run the below command. ./dev/tools/bump_perseus_version.js -t PR3055 |
jeremywiebe
left a comment
jeremywiebe
left a comment
There was a problem hiding this comment.
Thanks for cleaning this super-old code up!!
| @@ -1,13 +1,13 @@ | |||
| /* eslint-disable max-lines */ | |||
| /* eslint-disable @khanacademy/ts-no-error-suppressions */ | |||
There was a problem hiding this comment.
Question: can we remove this line now or are there still some @ts-expect-error's in here? :)
There was a problem hiding this comment.
there are still a number of ts-expect-error here. Let me see if I can clean up some of them.
There was a problem hiding this comment.
ok i'm able to remove it, asked help from claude:
Summary of Fixes
- Class Property Definite Assignment (lines 64-73)
- Added ! (definite assignment assertion) to properties that are assigned in componentDidMount:
- recordTouchStartOutside!
- blurOnTouchEndOutside!
- blurOnClickOutside!
- _container!
- _containerBounds!
- tabIndex Type Fix (line 999)
- Changed tabIndex={"0"} to tabIndex={0} (string → number)
- Tuple Type for Spread Operator (line 443)
- Added explicit tuple type: const points: [number, number][] = [...]
- This allows the spread operator ...point to work correctly with document.elementFromPoint()
- Element Type Fixes (lines 471-523)
- Changed ReadonlyArray<null | HTMLElement> to Array<Element | null> for leafElements and nonLeafElements
- Added proper type for hitNode: Element | null = null
- Used optional chaining element?.getAttribute() to handle potential null values
Result
- ✅ 0 TypeScript errors in the math-input package
- ✅ All ts-expect-error suppressions removed from the file
- ✅ Code is now fully type-safe and cleaner!
github-actions
bot
added
item-splitting-change
and removed
item-splitting-change
labels
| y += dy; | ||
|
|
||
| const points = [ | ||
| const points: [number, number][] = [ |
| blurOnTouchEndOutside: (arg1: any) => void; | ||
| // @ts-expect-error - TS2564 - Property 'blurOnClickOutside' has no initializer and is not definitely assigned in the constructor. | ||
| blurOnClickOutside: (arg1: any) => void; | ||
| recordTouchStartOutside!: (arg1: any) => void; |
There was a problem hiding this comment.
I think this is an old pattern that could be changed. Generally, I'm pretty hesitant to use ! as a solution. It creates risk for us in the future if we refactor when/where we initialize these fields because it suppresses some of TypeScript's type checking.
These functions are assigned in componentDidMount. Could we just make them arrow functions on the class and move the initialization out of componentDidMount? We could pair tomorrow or next week if that's not clear. :)
ivyolamit
force-pushed
the
LEMS-3361/use-wb-tab-in-expression
branch
from
6ea88ca to
bfa3a3b
Compare
ivyolamit
force-pushed
the
LEMS-3361/refactor-fake-react-native-web
branch
from
5a7da89 to
927b1c7
Compare
…ew instead of fake-react-native-web View
…e-react-native-web View
ivyolamit
force-pushed
the
LEMS-3361/refactor-fake-react-native-web
branch
from
927b1c7 to
fdd906c
Compare
Summary:
Use WB View instead of fake-react-native-web View
This refactor is on top of the base PR WB Tabs change in #2952
Issue: LEMS-3361
Test plan:
Co-Authored by Claude Code