Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 3 commits into from
Dec 3, 2024
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ddbf028
Fix types so that we can remove a bunch of TS suppressions
jeremywiebe Nov 30, 2024
339d50e
More TS fixes
jeremywiebe Nov 30, 2024
5c63df7
Changeset
jeremywiebe Nov 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/early-phones-remain.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@khanacademy/perseus": patch
"@khanacademy/perseus-editor": patch
---

Type fixes
24 changes: 7 additions & 17 deletions packages/perseus-editor/src/article-editor.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
7 changes: 1 addition & 6 deletions packages/perseus-editor/src/components/__stories__/color-select.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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}
/>
);
},
Expand Down
5 changes: 3 additions & 2 deletions packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/color-select.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand All @@ -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=""
>
Expand Down
4 changes: 2 additions & 2 deletions ...perseus-editor/src/widgets/interactive-graph-editor/locked-figures/line-stroke-select.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand All @@ -20,7 +20,7 @@ const LineStrokeSelect = (props: Props) => {
<Strut size={spacing.xxxSmall_4} />
<SingleSelect
selectedValue={selectedValue}
onChange={onChange}
onChange={onChange as any}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Anyway we can avoid the usage of as any here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not easily. The issue is that SingleSelect's onChange provides the newly selected value as a string, but this component wants to raise the onChange with the the new style strongly typed to StyleOptions ("solid" | "dashed"). There might be a magic TypeScript way to morph the string to a strongly typed TypeScript union, but I haven't figured out how yet.

// Placeholder is required, but never gets used.
placeholder=""
>
Expand Down
20 changes: 6 additions & 14 deletions ...us-editor/src/widgets/interactive-graph-editor/locked-figures/locked-ellipse-settings.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ const LockedEllipseSettings = (props: Props) => {
}

function handleLabelChange(
updatedLabel: LockedLabelType,
updatedLabel: Partial<LockedLabelType>,
labelIndex: number,
) {
if (!labels) {
Expand Down Expand Up @@ -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} />

Expand Down Expand Up @@ -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 */}
Expand Down Expand Up @@ -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);
}}
Expand Down
10 changes: 5 additions & 5 deletions ...rseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-figure-select.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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}>
Expand Down
3 changes: 1 addition & 2 deletions ...eus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-figures-section.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 && (
Expand Down
14 changes: 5 additions & 9 deletions ...s-editor/src/widgets/interactive-graph-editor/locked-figures/locked-function-settings.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ const LockedFunctionSettings = (props: Props) => {
}

function handleLabelChange(
updatedLabel: LockedLabelType,
updatedLabel: Partial<LockedLabelType>,
labelIndex: number,
) {
if (!labels) {
Expand Down Expand Up @@ -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} />

Expand Down Expand Up @@ -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);
}}
Expand Down
13 changes: 4 additions & 9 deletions ...seus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-label-settings.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@
*/
import {
lockedFigureColors,
type LockedFigure,
type LockedFigureColor,
type LockedLabelType,
components,
} from "@khanacademy/perseus";
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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} />
Expand Down
15 changes: 5 additions & 10 deletions ...rseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-line-settings.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ const LockedLineSettings = (props: Props) => {
}

function handleLabelChange(
updatedLabel: LockedLabelType,
updatedLabel: Partial<LockedLabelType>,
labelIndex: number,
) {
if (!labels) {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
}}
Expand Down
11 changes: 4 additions & 7 deletions ...seus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-point-settings.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ const LockedPointSettings = (props: Props) => {
}

function handleLabelChange(
updatedLabel: LockedLabelType,
updatedLabel: Partial<LockedLabelType>,
labelIndex: number,
) {
if (!labels) {
Expand Down Expand Up @@ -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);
}}
Expand Down
Loading