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 cc0bd596c9..c7a260a260 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"; @@ -27,10 +29,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"; @@ -55,6 +58,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; @@ -119,7 +124,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, @@ -168,14 +174,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; @@ -294,17 +292,16 @@ class InteractiveGraphEditor extends React.Component { showTooltips: this.props.showTooltips, lockedFigures: this.props.lockedFigures, 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, @@ -387,19 +384,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} @@ -422,15 +427,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} @@ -476,6 +495,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, @@ -507,7 +531,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, @@ -581,7 +608,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} @@ -644,7 +688,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} @@ -694,6 +752,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 baa1aacc51..347c9d1828 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. @@ -812,7 +813,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]; }; @@ -831,7 +832,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; @@ -839,7 +840,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; @@ -848,7 +849,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; @@ -865,7 +866,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; @@ -873,7 +874,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; @@ -883,7 +884,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; @@ -891,7 +892,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; @@ -899,7 +900,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 82644f8c6d..370a97c780 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"];