From 5e8d8468bb307ab8168be8e15d30af2be61ffd4a Mon Sep 17 00:00:00 2001 From: Jeremy Wiebe Date: Tue, 3 Dec 2024 13:10:58 -0800 Subject: [PATCH] Fix types so that we can remove a bunch of TS suppressions (#1938) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary: I was running unit tests and came across some TS suppressions that were fairly easy to resolve. Issue: "none" ## Test plan: `yarn lint` `yarn typecheck` `yarn test` Author: jeremywiebe Reviewers: catandthemachines, jeremywiebe Required Reviewers: Approved By: catandthemachines Checks: ✅ gerald, ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ✅ gerald Pull Request URL: https://github.com/Khan/perseus/pull/1938 --- .changeset/early-phones-remain.md | 6 +++++ .../perseus-editor/src/article-editor.tsx | 24 ++++++------------- .../__stories__/color-select.stories.tsx | 7 +----- .../locked-figures/color-select.tsx | 5 ++-- .../locked-figures/line-stroke-select.tsx | 4 ++-- .../locked-ellipse-settings.tsx | 20 +++++----------- .../locked-figures/locked-figure-select.tsx | 10 ++++---- .../locked-figures/locked-figures-section.tsx | 3 +-- .../locked-function-settings.tsx | 14 ++++------- .../locked-figures/locked-label-settings.tsx | 13 ++++------ .../locked-figures/locked-line-settings.tsx | 15 ++++-------- .../locked-figures/locked-point-settings.tsx | 11 ++++----- .../locked-polygon-settings.tsx | 20 +++++----------- .../locked-figures/locked-vector-settings.tsx | 14 ++++------- packages/perseus/src/renderer.tsx | 6 ++--- packages/perseus/src/types.ts | 2 +- .../widgets/__testutils__/renderQuestion.tsx | 14 +++++------ .../radio/__stories__/radio.stories.tsx | 4 +--- 18 files changed, 71 insertions(+), 121 deletions(-) create mode 100644 .changeset/early-phones-remain.md diff --git a/.changeset/early-phones-remain.md b/.changeset/early-phones-remain.md new file mode 100644 index 0000000000..66ef76e75b --- /dev/null +++ b/.changeset/early-phones-remain.md @@ -0,0 +1,6 @@ +--- +"@khanacademy/perseus": patch +"@khanacademy/perseus-editor": patch +--- + +Type fixes diff --git a/packages/perseus-editor/src/article-editor.tsx b/packages/perseus-editor/src/article-editor.tsx index 6dad830adc..3fecafed7b 100644 --- a/packages/perseus-editor/src/article-editor.tsx +++ b/packages/perseus-editor/src/article-editor.tsx @@ -208,12 +208,8 @@ export default class ArticleEditor extends React.Component { {...section} apiOptions={apiOptions} imageUploader={imageUploader} - // TODO(LEMS-2656): remove TS suppression - onChange={ - _.partial( - this._handleEditorChange, - i, - ) as any + onChange={(newProps) => + this._handleEditorChange(i, newProps) } placeholder="Type your section text here..." ref={"editor" + i} @@ -304,9 +300,8 @@ export default class ArticleEditor extends React.Component { i, newProps, ) => { - const sections = _.clone(this._sections()); - // @ts-expect-error - TS2542 - Index signature in type 'readonly RendererProps[]' only permits reading. - sections[i] = _.extend({}, sections[i], newProps); + const sections = [...this._sections()]; + sections[i] = {...sections[i], ...newProps}; this.props.onChange({json: sections}); }; @@ -314,11 +309,9 @@ export default class ArticleEditor extends React.Component { if (i === 0) { return; } - const sections = _.clone(this._sections()); + const sections = [...this._sections()]; const section = sections[i]; - // @ts-expect-error - TS2551 - Property 'splice' does not exist on type 'readonly RendererProps[]'. Did you mean 'slice'? sections.splice(i, 1); - // @ts-expect-error - TS2551 - Property 'splice' does not exist on type 'readonly RendererProps[]'. Did you mean 'slice'? sections.splice(i - 1, 0, section); this.props.onChange({ json: sections, @@ -326,14 +319,12 @@ export default class ArticleEditor extends React.Component { } _handleMoveSectionLater(i: number) { - const sections = _.clone(this._sections()); + const sections = [...this._sections()]; if (i + 1 === sections.length) { return; } const section = sections[i]; - // @ts-expect-error - TS2551 - Property 'splice' does not exist on type 'readonly RendererProps[]'. Did you mean 'slice'? sections.splice(i, 1); - // @ts-expect-error - TS2551 - Property 'splice' does not exist on type 'readonly RendererProps[]'. Did you mean 'slice'? sections.splice(i + 1, 0, section); this.props.onChange({ json: sections, @@ -364,8 +355,7 @@ export default class ArticleEditor extends React.Component { } _handleRemoveSection(i: number) { - const sections = _.clone(this._sections()); - // @ts-expect-error - TS2551 - Property 'splice' does not exist on type 'readonly RendererProps[]'. Did you mean 'slice'? + const sections = [...this._sections()]; sections.splice(i, 1); this.props.onChange({ json: sections, diff --git a/packages/perseus-editor/src/components/__stories__/color-select.stories.tsx b/packages/perseus-editor/src/components/__stories__/color-select.stories.tsx index 24c2cebbeb..5a0bf94c06 100644 --- a/packages/perseus-editor/src/components/__stories__/color-select.stories.tsx +++ b/packages/perseus-editor/src/components/__stories__/color-select.stories.tsx @@ -29,15 +29,10 @@ export const Controlled = { const [selectedValue, setSelectedValue] = React.useState(defaultColor); - const handleColorChange = (color: LockedFigureColor) => { - setSelectedValue(color); - }; - return ( ); }, diff --git a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/color-select.tsx b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/color-select.tsx index df8cafec96..2485b79286 100644 --- a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/color-select.tsx +++ b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/color-select.tsx @@ -17,7 +17,7 @@ const possibleColors = Object.keys(lockedFigureColors) as LockedFigureColor[]; type Props = { selectedValue: LockedFigureColor; style?: StyleType; - onChange: (newValue: string) => void; + onChange: (newColor: LockedFigureColor) => void; }; const ColorSelect = (props: Props) => { @@ -30,7 +30,8 @@ const ColorSelect = (props: Props) => { diff --git a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/line-stroke-select.tsx b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/line-stroke-select.tsx index f0051f63ab..b5c93d01a3 100644 --- a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/line-stroke-select.tsx +++ b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/line-stroke-select.tsx @@ -8,7 +8,7 @@ import * as React from "react"; type StyleOptions = "solid" | "dashed"; type Props = { selectedValue: StyleOptions; - onChange: (newValue: string) => void; + onChange: (newValue: StyleOptions) => void; }; const LineStrokeSelect = (props: Props) => { @@ -20,7 +20,7 @@ const LineStrokeSelect = (props: Props) => { diff --git a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-ellipse-settings.tsx b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-ellipse-settings.tsx index 77b6e410ad..f9bb17c479 100644 --- a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-ellipse-settings.tsx +++ b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-ellipse-settings.tsx @@ -126,7 +126,7 @@ const LockedEllipseSettings = (props: Props) => { } function handleLabelChange( - updatedLabel: LockedLabelType, + updatedLabel: Partial, labelIndex: number, ) { if (!labels) { @@ -206,8 +206,7 @@ const LockedEllipseSettings = (props: Props) => { {/* Color */} @@ -242,11 +241,7 @@ const LockedEllipseSettings = (props: Props) => { {/* Stroke style */} - onChangeProps({strokeStyle: value})) as any - } + onChange={(value) => onChangeProps({strokeStyle: value})} /> {/* Aria label */} @@ -278,12 +273,9 @@ const LockedEllipseSettings = (props: Props) => { { - handleLabelChange(newLabel, labelIndex); - }) as any - } + onChangeProps={(newLabel) => { + handleLabelChange(newLabel, labelIndex); + }} onRemove={() => { handleLabelRemove(labelIndex); }} diff --git a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-figure-select.tsx b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-figure-select.tsx index 71debfccf9..bbf6deed4a 100644 --- a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-figure-select.tsx +++ b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-figure-select.tsx @@ -11,29 +11,29 @@ import {spacing, color} from "@khanacademy/wonder-blocks-tokens"; import {StyleSheet} from "aphrodite"; import * as React from "react"; +import type {LockedFigureType} from "@khanacademy/perseus"; + type Props = { // Whether to show the locked labels in the locked figure settings. // TODO(LEMS-2274): Remove this prop once the label flag is // sfully rolled out. showLabelsFlag?: boolean; id: string; - onChange: (value: string) => void; + onChange: (value: LockedFigureType) => void; }; const LockedFigureSelect = (props: Props) => { const {id, onChange} = props; - const figureTypes = [ + const figureTypes: ReadonlyArray = [ "point", "line", "vector", "ellipse", "polygon", "function", + ...(props.showLabelsFlag ? ["label" as const] : []), ]; - if (props.showLabelsFlag) { - figureTypes.push("label"); - } return ( diff --git a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-figures-section.tsx b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-figures-section.tsx index 18d6468b06..6cbba741c2 100644 --- a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-figures-section.tsx +++ b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-figures-section.tsx @@ -191,8 +191,7 @@ const LockedFiguresSection = (props: Props) => { {showExpandButton && ( diff --git a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-function-settings.tsx b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-function-settings.tsx index a303ec87e2..7c62cf23d0 100644 --- a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-function-settings.tsx +++ b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-function-settings.tsx @@ -161,7 +161,7 @@ const LockedFunctionSettings = (props: Props) => { } function handleLabelChange( - updatedLabel: LockedLabelType, + updatedLabel: Partial, labelIndex: number, ) { if (!labels) { @@ -205,8 +205,7 @@ const LockedFunctionSettings = (props: Props) => { {/* Line color settings */} @@ -356,12 +355,9 @@ const LockedFunctionSettings = (props: Props) => { key={labelIndex} {...label} expanded={true} - // TODO(LEMS-2656): remove TS suppression - onChangeProps={ - ((newLabel: LockedLabelType) => { - handleLabelChange(newLabel, labelIndex); - }) as any - } + onChangeProps={(newLabel) => { + handleLabelChange(newLabel, labelIndex); + }} onRemove={() => { handleLabelRemove(labelIndex); }} diff --git a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-label-settings.tsx b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-label-settings.tsx index 346b6af5ad..60308011e1 100644 --- a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-label-settings.tsx +++ b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-label-settings.tsx @@ -7,8 +7,6 @@ */ import { lockedFigureColors, - type LockedFigure, - type LockedFigureColor, type LockedLabelType, components, } from "@khanacademy/perseus"; @@ -36,7 +34,7 @@ export type Props = LockedLabelType & { /** * Called when the props (coord, color, etc.) are updated. */ - onChangeProps: (newProps: Partial) => void; + onChangeProps: (newProps: Partial) => void; // Movement props. Used for standalone label actions. // Not used within other locked figure settings. @@ -150,12 +148,9 @@ export default function LockedLabelSettings(props: Props) { { - onChangeProps({color: newColor}); - }) as any - } + onChange={(newColor) => { + onChangeProps({color: newColor}); + }} style={styles.spaceUnder} /> diff --git a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-line-settings.tsx b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-line-settings.tsx index 4aaece85cf..454974644e 100644 --- a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-line-settings.tsx +++ b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-line-settings.tsx @@ -162,7 +162,7 @@ const LockedLineSettings = (props: Props) => { } function handleLabelChange( - updatedLabel: LockedLabelType, + updatedLabel: Partial, labelIndex: number, ) { if (!labels) { @@ -224,15 +224,13 @@ const LockedLineSettings = (props: Props) => { {/* Line color settings */} {/* Line style settings */} onChangeProps({lineStyle: value})) as any @@ -300,12 +298,9 @@ const LockedLineSettings = (props: Props) => { { - handleLabelChange(newLabel, labelIndex); - }) as any - } + onChangeProps={(newLabel) => { + handleLabelChange(newLabel, labelIndex); + }} onRemove={() => { handleLabelRemove(labelIndex); }} diff --git a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-point-settings.tsx b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-point-settings.tsx index b82dc3f938..8a25c756a3 100644 --- a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-point-settings.tsx +++ b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-point-settings.tsx @@ -164,7 +164,7 @@ const LockedPointSettings = (props: Props) => { } function handleLabelChange( - updatedLabel: LockedLabelType, + updatedLabel: Partial, labelIndex: number, ) { if (!labels) { @@ -275,12 +275,9 @@ const LockedPointSettings = (props: Props) => { styles.lockedPointLabelContainer } expanded={true} - // TODO(LEMS-2656): remove TS suppression - onChangeProps={ - ((newLabel: LockedLabelType) => { - handleLabelChange(newLabel, labelIndex); - }) as any - } + onChangeProps={(newLabel) => { + handleLabelChange(newLabel, labelIndex); + }} onRemove={() => { handleLabelRemove(labelIndex); }} diff --git a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-polygon-settings.tsx b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-polygon-settings.tsx index e56c9075f5..fbd3eb5789 100644 --- a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-polygon-settings.tsx +++ b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-polygon-settings.tsx @@ -142,7 +142,7 @@ const LockedPolygonSettings = (props: Props) => { } function handleLabelChange( - updatedLabel: LockedLabelType, + updatedLabel: Partial, labelIndex: number, ) { if (!labels) { @@ -189,8 +189,7 @@ const LockedPolygonSettings = (props: Props) => { {/* Color */} @@ -225,11 +224,7 @@ const LockedPolygonSettings = (props: Props) => { {/* Stroke style */} - onChangeProps({strokeStyle: value})) as any - } + onChange={(value) => onChangeProps({strokeStyle: value})} /> {/* Show vertices switch */} @@ -371,12 +366,9 @@ const LockedPolygonSettings = (props: Props) => { { - handleLabelChange(newLabel, labelIndex); - }) as any - } + onChangeProps={(newLabel) => { + handleLabelChange(newLabel, labelIndex); + }} onRemove={() => { handleLabelRemove(labelIndex); }} diff --git a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-vector-settings.tsx b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-vector-settings.tsx index 098dd27c9c..9cba61984b 100644 --- a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-vector-settings.tsx +++ b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-vector-settings.tsx @@ -117,7 +117,7 @@ const LockedVectorSettings = (props: Props) => { } function handleLabelChange( - updatedLabel: LockedLabelType, + updatedLabel: Partial, labelIndex: number, ) { if (!labels) { @@ -159,8 +159,7 @@ const LockedVectorSettings = (props: Props) => { {/* Line color settings */} @@ -237,12 +236,9 @@ const LockedVectorSettings = (props: Props) => { { - handleLabelChange(newLabel, labelIndex); - }) as any - } + onChangeProps={(newLabel) => { + handleLabelChange(newLabel, labelIndex); + }} onRemove={() => { handleLabelRemove(labelIndex); }} diff --git a/packages/perseus/src/renderer.tsx b/packages/perseus/src/renderer.tsx index ff930e7711..fd89f39834 100644 --- a/packages/perseus/src/renderer.tsx +++ b/packages/perseus/src/renderer.tsx @@ -1652,8 +1652,8 @@ class Renderer setInputValue: ( path: FocusPath, newValue: string, - focus?: () => unknown, - ) => void = (path, newValue, focus) => { + cb?: () => void, + ) => void = (path, newValue, cb) => { // @ts-expect-error - TS2345 - Argument of type 'FocusPath' is not assignable to parameter of type 'List'. const widgetId = _.first(path); // @ts-expect-error - TS2345 - Argument of type 'FocusPath' is not assignable to parameter of type 'List'. @@ -1661,7 +1661,7 @@ class Renderer const widget = this.getWidgetInstance(widgetId); // Widget handles parsing of the interWidgetPath. - widget?.setInputValue?.(interWidgetPath, newValue, focus); + widget?.setInputValue?.(interWidgetPath, newValue, cb); }; /** diff --git a/packages/perseus/src/types.ts b/packages/perseus/src/types.ts index eb7f3dd0a6..41e4514cdd 100644 --- a/packages/perseus/src/types.ts +++ b/packages/perseus/src/types.ts @@ -156,7 +156,7 @@ export type ChangeHandler = ( // Interactive Graph callback (see legacy: interactive-graph.tsx) graph?: PerseusGraphType; }, - callback?: () => unknown | null | undefined, + callback?: () => void, silent?: boolean, ) => unknown; diff --git a/packages/perseus/src/widgets/__testutils__/renderQuestion.tsx b/packages/perseus/src/widgets/__testutils__/renderQuestion.tsx index baaa469c4c..40a62a67e0 100644 --- a/packages/perseus/src/widgets/__testutils__/renderQuestion.tsx +++ b/packages/perseus/src/widgets/__testutils__/renderQuestion.tsx @@ -22,17 +22,16 @@ import type {PropsFor} from "@khanacademy/wonder-blocks-core"; type RenderResult = ReturnType; +type ExtraProps = Omit, "strings">; + export const renderQuestion = ( question: PerseusRenderer, apiOptions: APIOptions = Object.freeze({}), - extraProps?: Omit, "strings">, + extraProps?: ExtraProps, ): { container: HTMLElement; renderer: Perseus.Renderer; - rerender: ( - question: PerseusRenderer, - extraProps?: Omit, "strings">, - ) => void; + rerender: (question: PerseusRenderer, extraProps?: ExtraProps) => void; unmount: RenderResult["unmount"]; } => { setDependencies(testDependencies); @@ -59,7 +58,7 @@ export const renderQuestion = ( } const renderAgain = ( question: PerseusRenderer, - extraProps: undefined | React.ComponentProps, + extraProps?: ExtraProps, ) => { rerender( @@ -81,8 +80,7 @@ export const renderQuestion = ( } }; - // TODO(LEMS-2656): remove TS suppression - return {container, renderer, rerender: renderAgain as any, unmount}; + return {container, renderer, rerender: renderAgain, unmount}; }; const Renderer = React.forwardRef< diff --git a/packages/perseus/src/widgets/radio/__stories__/radio.stories.tsx b/packages/perseus/src/widgets/radio/__stories__/radio.stories.tsx index ead3e91f55..bc1a4a36b3 100644 --- a/packages/perseus/src/widgets/radio/__stories__/radio.stories.tsx +++ b/packages/perseus/src/widgets/radio/__stories__/radio.stories.tsx @@ -41,8 +41,6 @@ export default { }, }, }, - // TODO(LEMS-2656): remove TS suppression - // @ts-expect-error: Type 'Args' is not assignable to type 'StoryArgs'. render: (args: StoryArgs) => ( ), -} satisfies Meta; +} satisfies Meta; const applyStoryArgs = (args: StoryArgs): PerseusRenderer => { const q = {