diff --git a/.changeset/rude-mangos-boil.md b/.changeset/rude-mangos-boil.md new file mode 100644 index 0000000000..55ef40376d --- /dev/null +++ b/.changeset/rude-mangos-boil.md @@ -0,0 +1,6 @@ +--- +"@khanacademy/perseus": patch +"@khanacademy/perseus-editor": patch +--- + +Internal: improve type safety of interactive graph editor diff --git a/packages/perseus-editor/src/widgets/interactive-graph-editor/interactive-graph-editor.tsx b/packages/perseus-editor/src/widgets/interactive-graph-editor/interactive-graph-editor.tsx index b7eafc091e..2b99be6e56 100644 --- a/packages/perseus-editor/src/widgets/interactive-graph-editor/interactive-graph-editor.tsx +++ b/packages/perseus-editor/src/widgets/interactive-graph-editor/interactive-graph-editor.tsx @@ -1,8 +1,8 @@ import {vector as kvector} from "@khanacademy/kmath"; import { components, - interactiveSizes, InteractiveGraphWidget, + interactiveSizes, SizingUtils, Util, } from "@khanacademy/perseus"; @@ -11,8 +11,10 @@ import {OptionItem, SingleSelect} from "@khanacademy/wonder-blocks-dropdown"; import {Checkbox} from "@khanacademy/wonder-blocks-form"; import {spacing} from "@khanacademy/wonder-blocks-tokens"; import {LabelSmall} from "@khanacademy/wonder-blocks-typography"; +import {UnreachableCaseError} from "@khanacademy/wonder-stuff-core"; import {StyleSheet} from "aphrodite"; import * as React from "react"; +import invariant from "tiny-invariant"; import _ from "underscore"; import LabeledRow from "../../components/graph-locked-figures/labeled-row"; @@ -28,10 +30,11 @@ import {shouldShowStartCoordsUI} from "../../components/util"; import {parsePointCount} from "../../util/points"; import type { - PerseusImageBackground, - PerseusInteractiveGraphWidgetOptions, APIOptionsWithDefaults, LockedFigure, + PerseusImageBackground, + PerseusInteractiveGraphWidgetOptions, + PerseusGraphType, } from "@khanacademy/perseus"; import type {PropsFor} from "@khanacademy/wonder-blocks-core"; @@ -56,6 +59,8 @@ const POLYGON_SIDES = _.map(_.range(3, 13), function (value) { }); type Range = [min: number, max: number]; +type PerseusGraphTypePolygon = Extract; +type PerseusGraphTypeAngle = Extract; export type Props = { apiOptions: APIOptionsWithDefaults; @@ -120,7 +125,8 @@ export type Props = { * on the state of the interactive graph previewed at the bottom of the * editor page. */ - correct: any; // TODO(jeremy) + // TODO(LEMS-2344): make the type of `correct` more specific + correct: PerseusGraphType; /** * The locked figures to display in the graph area. * Locked figures are graph elements (points, lines, line segmeents, @@ -176,14 +182,6 @@ class InteractiveGraphEditor extends React.Component { }, }; - changeMatchType = (newValue) => { - const correct = { - ...this.props.correct, - match: newValue, - }; - this.props.onChange({correct: correct}); - }; - changeStartCoords = (coords) => { if (!this.props.graph?.type) { return; @@ -306,17 +304,16 @@ class InteractiveGraphEditor extends React.Component { fullGraphAriaLabel: this.props.fullGraphAriaLabel, fullGraphAriaDescription: this.props.fullGraphAriaDescription, trackInteraction: function () {}, - onChange: (newProps: InteractiveGraphProps) => { + onChange: ({graph: newGraph}: InteractiveGraphProps) => { let correct = this.props.correct; - // @ts-expect-error - TS2532 - Object is possibly 'undefined'. - if (correct.type === newProps.graph.type) { - correct = { - ...correct, - ...newProps.graph, - }; + // TODO(benchristel): can we improve the type of onChange + // so this invariant isn't necessary? + invariant(newGraph != null); + if (correct.type === newGraph.type) { + correct = mergeGraphs(correct, newGraph); } else { // Clear options from previous graph - correct = newProps.graph; + correct = newGraph; } this.props.onChange({ correct: correct, @@ -411,19 +408,27 @@ class InteractiveGraphEditor extends React.Component { } placeholder="" onChange={(newValue) => { - const graph = { - ...this.props.correct, + invariant( + this.props.graph?.type === "polygon", + ); + const updates = { numSides: parsePointCount(newValue), coords: null, // reset the snap for UNLIMITED, which // only supports "grid" // From: D6578 snapTo: "grid", - }; + } as const; this.props.onChange({ - correct: graph, - graph: graph, + correct: { + ...this.props.correct, + ...updates, + }, + graph: { + ...this.props.graph, + ...updates, + }, }); }} style={styles.singleSelectShort} @@ -446,15 +451,29 @@ class InteractiveGraphEditor extends React.Component { // Never uses placeholder, always has value placeholder="" onChange={(newValue) => { - const graph = { - ...this.props.correct, - snapTo: newValue, + invariant( + this.props.correct.type === "polygon", + `Expected correct answer type to be polygon, but got ${this.props.correct.type}`, + ); + invariant( + this.props.graph?.type === "polygon", + `Expected graph type to be polygon, but got ${this.props.graph?.type}`, + ); + + const updates = { + snapTo: newValue as PerseusGraphTypePolygon["snapTo"], coords: null, - }; + } as const; this.props.onChange({ - correct: graph, - graph: graph, + correct: { + ...this.props.correct, + ...updates, + }, + graph: { + ...this.props.graph, + ...updates, + }, }); }} style={styles.singleSelectShort} @@ -500,6 +519,11 @@ class InteractiveGraphEditor extends React.Component { } onChange={() => { if (this.props.graph?.type === "polygon") { + invariant( + this.props.correct.type === + "polygon", + `Expected graph type to be polygon, but got ${this.props.correct.type}`, + ); this.props.onChange({ correct: { ...this.props.correct, @@ -531,7 +555,10 @@ class InteractiveGraphEditor extends React.Component { !!this.props.correct?.showSides } onChange={() => { - if (this.props.graph?.type === "polygon") { + if ( + this.props.graph?.type === "polygon" && + this.props.correct.type === "polygon" + ) { this.props.onChange({ correct: { ...this.props.correct, @@ -605,7 +632,24 @@ class InteractiveGraphEditor extends React.Component { { + invariant( + this.props.correct.type === "polygon", + `Expected graph type to be polygon, but got ${this.props.correct.type}`, + ); + const correct = { + ...this.props.correct, + // TODO(benchristel): this cast is necessary + // because "exact" is not actually a valid + // value for `match`; a value of undefined + // means exact matching. The code happens + // to work because "exact" falls through + // to the correct else branch in + // InteractiveGraph.validate() + match: newValue as PerseusGraphTypePolygon["match"], + }; + this.props.onChange({correct}); + }} // Never uses placeholder, always has value placeholder="" style={styles.singleSelectShort} @@ -668,7 +712,21 @@ class InteractiveGraphEditor extends React.Component { { + this.props.onChange({ + correct: { + ...this.props.correct, + // TODO(benchristel): this cast is necessary + // because "exact" is not actually a valid + // value for `match`; a value of undefined + // means exact matching. The code happens + // to work because "exact" falls through + // to the correct else branch in + // InteractiveGraph.validate() + match: newValue as PerseusGraphTypeAngle["match"], + }, + }); + }} // Never uses placeholder, always has value placeholder="" style={styles.singleSelectShort} @@ -718,6 +776,54 @@ class InteractiveGraphEditor extends React.Component { } } +// Merges two graphs that have the same `type`. Properties defined in `b` +// overwrite properties of the same name in `a`. Throws an exception if the +// types are different or not recognized. +function mergeGraphs( + a: PerseusGraphType, + b: PerseusGraphType, +): PerseusGraphType { + if (a.type !== b.type) { + throw new Error( + `Cannot merge graphs with different types (${a.type} and ${b.type})`, + ); + } + switch (a.type) { + case "angle": + invariant(b.type === "angle"); + return {...a, ...b}; + case "circle": + invariant(b.type === "circle"); + return {...a, ...b}; + case "linear": + invariant(b.type === "linear"); + return {...a, ...b}; + case "linear-system": + invariant(b.type === "linear-system"); + return {...a, ...b}; + case "point": + invariant(b.type === "point"); + return {...a, ...b}; + case "polygon": + invariant(b.type === "polygon"); + return {...a, ...b}; + case "quadratic": + invariant(b.type === "quadratic"); + return {...a, ...b}; + case "ray": + invariant(b.type === "ray"); + return {...a, ...b}; + case "segment": + invariant(b.type === "segment"); + return {...a, ...b}; + case "sinusoid": + invariant(b.type === "sinusoid"); + return {...a, ...b}; + default: + throw new UnreachableCaseError(a); + } +} + const styles = StyleSheet.create({ singleSelectShort: { // Non-standard spacing, but it's the smallest we can go diff --git a/packages/perseus/src/perseus-types.ts b/packages/perseus/src/perseus-types.ts index f171c6652d..34c7097093 100644 --- a/packages/perseus/src/perseus-types.ts +++ b/packages/perseus/src/perseus-types.ts @@ -674,6 +674,7 @@ export type PerseusInteractiveGraphWidgetOptions = { // The type of graph graph: PerseusGraphType; // The correct kind of graph, if being used to select function type + // TODO(LEMS-2344): make the type of `correct` more specific correct: PerseusGraphType; // Shapes (points, chords, etc) displayed on the graph that cannot // be moved by the user. @@ -816,7 +817,7 @@ export type PerseusGraphTypeAngle = { // How to match the answer. If missing, defaults to exact matching. match?: "congruent"; // must have 3 coords - ie [Coord, Coord, Coord] - coords?: [Coord, Coord, Coord]; + coords?: [Coord, Coord, Coord] | null; // The initial coordinates the graph renders with. startCoords?: [Coord, Coord, Coord]; }; @@ -835,7 +836,7 @@ export type PerseusGraphTypeCircle = { export type PerseusGraphTypeLinear = { type: "linear"; // expects 2 coords - coords?: CollinearTuple; + coords?: CollinearTuple | null; // The initial coordinates the graph renders with. startCoords?: CollinearTuple; } & PerseusGraphTypeCommon; @@ -843,7 +844,7 @@ export type PerseusGraphTypeLinear = { export type PerseusGraphTypeLinearSystem = { type: "linear-system"; // expects 2 sets of 2 coords - coords?: CollinearTuple[]; + coords?: CollinearTuple[] | null; // The initial coordinates the graph renders with. startCoords?: CollinearTuple[]; } & PerseusGraphTypeCommon; @@ -852,7 +853,7 @@ export type PerseusGraphTypePoint = { type: "point"; // The number of points if a "point" type. default: 1. "unlimited" if no limit numPoints?: number | "unlimited"; - coords?: ReadonlyArray; + coords?: ReadonlyArray | null; // The initial coordinates the graph renders with. startCoords?: ReadonlyArray; } & PerseusGraphTypeCommon; @@ -869,7 +870,7 @@ export type PerseusGraphTypePolygon = { snapTo?: "grid" | "angles" | "sides"; // How to match the answer. If missing, defaults to exact matching. match?: "similar" | "congruent" | "approx"; - coords?: ReadonlyArray; + coords?: ReadonlyArray | null; // The initial coordinates the graph renders with. startCoords?: ReadonlyArray; } & PerseusGraphTypeCommon; @@ -877,7 +878,7 @@ export type PerseusGraphTypePolygon = { export type PerseusGraphTypeQuadratic = { type: "quadratic"; // expects a list of 3 coords - coords?: [Coord, Coord, Coord]; + coords?: [Coord, Coord, Coord] | null; // The initial coordinates the graph renders with. startCoords?: [Coord, Coord, Coord]; } & PerseusGraphTypeCommon; @@ -887,7 +888,7 @@ export type PerseusGraphTypeSegment = { // The number of segments if a "segment" type. default: 1. Max: 6 numSegments?: number; // Expects a list of Coord tuples. Length should match the `numSegments` value. - coords?: CollinearTuple[]; + coords?: CollinearTuple[] | null; // The initial coordinates the graph renders with. startCoords?: CollinearTuple[]; } & PerseusGraphTypeCommon; @@ -895,7 +896,7 @@ export type PerseusGraphTypeSegment = { export type PerseusGraphTypeSinusoid = { type: "sinusoid"; // Expects a list of 2 Coords - coords?: ReadonlyArray; + coords?: ReadonlyArray | null; // The initial coordinates the graph renders with. startCoords?: ReadonlyArray; } & PerseusGraphTypeCommon; @@ -903,7 +904,7 @@ export type PerseusGraphTypeSinusoid = { export type PerseusGraphTypeRay = { type: "ray"; // Expects a list of 2 Coords - coords?: CollinearTuple; + coords?: CollinearTuple | null; // The initial coordinates the graph renders with. startCoords?: CollinearTuple; } & PerseusGraphTypeCommon; diff --git a/packages/perseus/src/widgets/interactive-graph.tsx b/packages/perseus/src/widgets/interactive-graph.tsx index ebbc4df794..a68df25620 100644 --- a/packages/perseus/src/widgets/interactive-graph.tsx +++ b/packages/perseus/src/widgets/interactive-graph.tsx @@ -119,6 +119,7 @@ const makeInvalidTypeError = ( type RenderProps = PerseusInteractiveGraphWidgetOptions; // There's no transform function in exports export type Rubric = { + // TODO(LEMS-2344): make the type of `correct` more specific correct: PerseusGraphType; graph: PerseusGraphType; }; diff --git a/packages/perseus/src/widgets/interactive-graphs/stateful-mafs-graph.tsx b/packages/perseus/src/widgets/interactive-graphs/stateful-mafs-graph.tsx index 72e90bc144..3542b44214 100644 --- a/packages/perseus/src/widgets/interactive-graphs/stateful-mafs-graph.tsx +++ b/packages/perseus/src/widgets/interactive-graphs/stateful-mafs-graph.tsx @@ -21,6 +21,7 @@ export type StatefulMafsGraphProps = { box: [number, number]; backgroundImage?: InteractiveGraphProps["backgroundImage"]; graph: PerseusGraphType; + // TODO(LEMS-2344): make the type of `correct` more specific correct: PerseusGraphType; lockedFigures?: InteractiveGraphProps["lockedFigures"]; range: InteractiveGraphProps["range"];