Fix types so that we can remove a bunch of TS suppressions (#1938)

## 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: #1938
Khan · Dec 3, 2024 · 5e8d846 · 5e8d846
1 parent 763a4ba
commit 5e8d846
Show file tree

Hide file tree

Showing 18 changed files with 71 additions and 121 deletions.
diff --git a/.changeset/early-phones-remain.md 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
@@ -208,12 +208,8 @@ export default class ArticleEditor extends React.Component<Props, State> {
                                     {...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,36 +300,31 @@ export default class ArticleEditor extends React.Component<Props, State> {
         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});
     };
 
     _handleMoveSectionEarlier(i: number) {
         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,
         });
     }
 
     _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<Props, State> {
     }
 
     _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
@@ -29,15 +29,10 @@ export const Controlled = {
         const [selectedValue, setSelectedValue] =
             React.useState<LockedFigureColor>(defaultColor);
 
-        const handleColorChange = (color: LockedFigureColor) => {
-            setSelectedValue(color);
-        };
-
         return (
             <ColorSelect
                 selectedValue={selectedValue}
-                // TODO(LEMS-2656): remove TS suppression
-                onChange={handleColorChange as any}
+                onChange={setSelectedValue}
             />
         );
     },

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
@@ -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) => {
                 <Strut size={spacing.xxSmall_6} />
                 <SingleSelect
                     selectedValue={selectedValue}
-                    onChange={onChange}
+                    // TODO(LEMS-2656): remove TS suppression
+                    onChange={onChange as any}
                     // Placeholder is required, but never gets used.
                     placeholder=""
                 >

diff --git a/...perseus-editor/src/widgets/interactive-graph-editor/locked-figures/line-stroke-select.tsx b/...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) => {
             <Strut size={spacing.xxxSmall_4} />
             <SingleSelect
                 selectedValue={selectedValue}
-                onChange={onChange}
+                onChange={onChange as any}
                 // Placeholder is required, but never gets used.
                 placeholder=""
             >

diff --git a/...us-editor/src/widgets/interactive-graph-editor/locked-figures/locked-ellipse-settings.tsx b/...us-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<LockedLabelType>,
         labelIndex: number,
     ) {
         if (!labels) {
@@ -206,8 +206,7 @@ const LockedEllipseSettings = (props: Props) => {
                 {/* Color */}
                 <ColorSelect
                     selectedValue={color}
-                    // TODO(LEMS-2656): remove TS suppression
-                    onChange={handleColorChange as any}
+                    onChange={handleColorChange}
                 />
                 <Strut size={spacing.medium_16} />
 
@@ -242,11 +241,7 @@ const LockedEllipseSettings = (props: Props) => {
             {/* Stroke style */}
             <LineStrokeSelect
                 selectedValue={strokeStyle}
-                // TODO(LEMS-2656): remove TS suppression
-                onChange={
-                    ((value: "solid" | "dashed") =>
-                        onChangeProps({strokeStyle: value})) as any
-                }
+                onChange={(value) => onChangeProps({strokeStyle: value})}
             />
 
             {/* Aria label */}
@@ -278,12 +273,9 @@ const LockedEllipseSettings = (props: Props) => {
                         <LockedLabelSettings
                             {...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/...rseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-figure-select.tsx b/...rseus-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<LockedFigureType> = [
         "point",
         "line",
         "vector",
         "ellipse",
         "polygon",
         "function",
+        ...(props.showLabelsFlag ? ["label" as const] : []),
     ];
-    if (props.showLabelsFlag) {
-        figureTypes.push("label");
-    }
 
     return (
         <View style={styles.container}>

diff --git a/...eus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-figures-section.tsx b/...eus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-figures-section.tsx
@@ -191,8 +191,7 @@ const LockedFiguresSection = (props: Props) => {
                         <LockedFigureSelect
                             showLabelsFlag={props.showLabelsFlag}
                             id={`${uniqueId}-select`}
-                            // TODO(LEMS-2656): remove TS suppression
-                            onChange={addLockedFigure as any}
+                            onChange={addLockedFigure}
                         />
                         <Strut size={spacing.small_12} />
                         {showExpandButton && (

diff --git a/...s-editor/src/widgets/interactive-graph-editor/locked-figures/locked-function-settings.tsx b/...s-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<LockedLabelType>,
         labelIndex: number,
     ) {
         if (!labels) {
@@ -205,8 +205,7 @@ const LockedFunctionSettings = (props: Props) => {
                 {/* Line color settings */}
                 <ColorSelect
                     selectedValue={lineColor}
-                    // TODO(LEMS-2656): remove TS suppression
-                    onChange={handleColorChange as any}
+                    onChange={handleColorChange}
                 />
                 <Strut size={spacing.small_12} />
 
@@ -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/...seus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-label-settings.tsx b/...seus-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<LockedFigure>) => void;
+    onChangeProps: (newProps: Partial<LockedLabelType>) => 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) {
             <View style={styles.row}>
                 <ColorSelect
                     selectedValue={color}
-                    // TODO(LEMS-2656): remove TS suppression
-                    onChange={
-                        ((newColor: LockedFigureColor) => {
-                            onChangeProps({color: newColor});
-                        }) as any
-                    }
+                    onChange={(newColor) => {
+                        onChangeProps({color: newColor});
+                    }}
                     style={styles.spaceUnder}
                 />
                 <Strut size={spacing.medium_16} />

diff --git a/...rseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-line-settings.tsx b/...rseus-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<LockedLabelType>,
         labelIndex: number,
     ) {
         if (!labels) {
@@ -224,15 +224,13 @@ const LockedLineSettings = (props: Props) => {
                 {/* Line color settings */}
                 <ColorSelect
                     selectedValue={lineColor}
-                    // TODO(LEMS-2656): remove TS suppression
-                    onChange={handleColorChange as any}
+                    onChange={handleColorChange}
                 />
                 <Strut size={spacing.small_12} />
 
                 {/* Line style settings */}
                 <LineStrokeSelect
                     selectedValue={lineStyle}
-                    // TODO(LEMS-2656): remove TS suppression
                     onChange={
                         ((value: "solid" | "dashed") =>
                             onChangeProps({lineStyle: value})) as any
@@ -300,12 +298,9 @@ const LockedLineSettings = (props: Props) => {
                         <LockedLabelSettings
                             {...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/...seus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-point-settings.tsx b/...seus-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<LockedLabelType>,
         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);
                             }}