Skip to content

Commit 45635f7

Browse files
authored
Remove readonly types from data-schema.ts (#2398)
## Summary: See the [slack discussion] for more details. TL;DR: - The `readonly` modifier in TypeScript simply removes mutating operations like `Array.push` and `obj.foo = 1` property assignment from a type. - As such, it's best to think of it as an application of the [Interface Segregation Principle]. - That implies that `readonly` types are most appropriate in function parameters. There, they act as a promise that the function won't mutate its arguments. - They may also be appropriate in return types **if** the function returns a frozen object or array, because frozen objects actually disallow mutation at runtime. - In general, readonly types are **not** appropriate in the return types of functions, because they unnecessarily restrict the caller from mutating an object to which no one else holds a reference. - That means readonly types are probably **not** appropriate for shared data types which may be used in _either_ parameters or return values. - We weren't using readonly types consistently in `data-schema.ts` to begin with. We used them for arrays, but not for tuples or objects. - For the sake of consistency, and to allow our `data-schema` types to be reused easily in different contexts, this PR replaces `ReadonlyArray` with `Array` in `data-schema.ts` and fixes the type errors that cascade out of that. [slack discussion]: https://khanacademy.slack.com/archives/C01AZ9H8TTQ/p1744325813358959 [Interface Segregation Principle]: https://en.wikipedia.org/wiki/Interface_segregation_principle Issue: LEMS-XXXX ## Test plan: `pnpm tsc` Author: benchristel Reviewers: jeremywiebe, benchristel, handeyeco Required Reviewers: Approved By: jeremywiebe Checks: ✅ 8 checks were successful Pull Request URL: #2398
1 parent 67b3c67 commit 45635f7

39 files changed

+340
-335
lines changed

.changeset/bright-guests-march.md

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@khanacademy/perseus-core": major
3+
---
4+
5+
Remove ReadonlyArray types from `@khanacademy/perseus-core` in favor of mutable arrays. Users should define separate readonly types if desired.

packages/perseus-core/src/data-schema.ts

+64-66
Large diffs are not rendered by default.

packages/perseus-core/src/index.ts

+1
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ export type {
66
InteractiveMarkerType,
77
Relationship,
88
Alignment,
9+
RecursiveReadonly,
910
} from "./types";
1011
export type {
1112
KeypadKey,

packages/perseus-core/src/keypad.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ export type KeypadType = "FRACTION" | "EXPRESSION";
115115

116116
export type KeypadConfiguration = {
117117
keypadType: KeypadType;
118-
extraKeys?: ReadonlyArray<KeypadKey>;
118+
extraKeys?: KeypadKey[];
119119
times?: boolean;
120120
scientific?: boolean;
121121
};

packages/perseus-core/src/parse-perseus-json/perseus-parsers/legacy-button-sets.ts

+3-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import {array, enumeration} from "../general-purpose-parsers";
22
import {defaulted} from "../general-purpose-parsers/defaulted";
33

4+
import type {LegacyButtonSets} from "../../data-schema";
5+
46
export const parseLegacyButtonSet = enumeration(
57
"basic",
68
"basic+div",
@@ -17,5 +19,5 @@ export const parseLegacyButtonSets = defaulted(
1719
// NOTE(benchristel): I copied the default buttonSets from
1820
// expression.tsx. See the parse-perseus-json/README.md for
1921
// an explanation of why we want to duplicate the default here.
20-
() => ["basic", "trig", "prealgebra", "logarithms"] as const,
22+
(): LegacyButtonSets => ["basic", "trig", "prealgebra", "logarithms"],
2123
);

packages/perseus-core/src/types.ts

+6-2
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ export type KEScore = {
2626
// Base marker, with the props that are set by the editor.
2727
export type MarkerType = {
2828
// The list of correct answers expected for the marker.
29-
answers: ReadonlyArray<string>;
29+
answers: string[];
3030
// The marker title or description.
3131
label: string;
3232
// The marker coordinates on the question image as percent of image size.
@@ -37,7 +37,7 @@ export type MarkerType = {
3737
// Additional props that are set when user interacts with the marker.
3838
export type InteractiveMarkerType = MarkerType & {
3939
// The user selected list of answers, used to grade the question.
40-
selected?: ReadonlyArray<string>;
40+
selected?: string[];
4141
// Reveal the correctness state of the user selected answers for the marker.
4242
showCorrectness?: "correct" | "incorrect";
4343
focused?: boolean;
@@ -55,3 +55,7 @@ export type Alignment =
5555
| "float-left"
5656
| "float-right"
5757
| "full-width";
58+
59+
export type RecursiveReadonly<T> = {
60+
readonly [K in keyof T]: RecursiveReadonly<T[K]>;
61+
};

packages/perseus-core/src/utils/random-util.ts

+2-4
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,9 @@ export function shuffle<T>(
2929
array: ReadonlyArray<T>,
3030
randomSeed: number | RNG,
3131
ensurePermuted = false,
32-
): ReadonlyArray<T> {
32+
): T[] {
3333
// Always return a copy of the input array
34-
const shuffled = _.clone(array);
34+
const shuffled = [...array];
3535

3636
// Handle edge cases (input array is empty or uniform)
3737
if (
@@ -57,9 +57,7 @@ export function shuffle<T>(
5757
const newEnd = Math.floor(random() * top);
5858
const temp = shuffled[newEnd];
5959

60-
// @ts-expect-error - TS2542 - Index signature in type 'readonly T[]' only permits reading.
6160
shuffled[newEnd] = shuffled[top - 1];
62-
// @ts-expect-error - TS2542 - Index signature in type 'readonly T[]' only permits reading.
6361
shuffled[top - 1] = temp;
6462
}
6563
} while (ensurePermuted && _.isEqual(array, shuffled));

packages/perseus-core/src/validation.types.ts

+27-27
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ export type UserInputStatus = "correct" | "incorrect" | "incomplete";
8282
export type PerseusCategorizerRubric = {
8383
// The correct answers where index relates to the items and value relates
8484
// to the category. e.g. [0, 1, 0, 1, 2]
85-
values: ReadonlyArray<number>;
85+
values: number[];
8686
} & PerseusCategorizerValidationData;
8787

8888
export type PerseusCategorizerUserInput = {
@@ -91,7 +91,7 @@ export type PerseusCategorizerUserInput = {
9191

9292
export type PerseusCategorizerValidationData = {
9393
// Translatable text; a list of items to categorize. e.g. ["banana", "yellow", "apple", "purple", "shirt"]
94-
items: ReadonlyArray<string>;
94+
items: string[];
9595
};
9696

9797
export type PerseusCSProgramUserInput = {
@@ -100,16 +100,16 @@ export type PerseusCSProgramUserInput = {
100100
};
101101

102102
export type PerseusDropdownRubric = {
103-
choices: ReadonlyArray<PerseusDropdownChoice>;
103+
choices: Array<PerseusDropdownChoice>;
104104
};
105105

106106
export type PerseusDropdownUserInput = {
107107
value: number;
108108
};
109109

110110
export type PerseusExpressionRubric = {
111-
answerForms: ReadonlyArray<PerseusExpressionAnswerForm>;
112-
functions: ReadonlyArray<string>;
111+
answerForms: Array<PerseusExpressionAnswerForm>;
112+
functions: string[];
113113
};
114114

115115
export type PerseusExpressionUserInput = string;
@@ -162,29 +162,29 @@ export type PerseusInteractiveGraphRubric = {
162162
export type PerseusInteractiveGraphUserInput = PerseusGraphType;
163163

164164
export type PerseusLabelImageRubric = {
165-
markers: ReadonlyArray<{
166-
answers: ReadonlyArray<string>;
165+
markers: Array<{
166+
answers: string[];
167167
label: string;
168168
}>;
169169
};
170170

171171
export type PerseusLabelImageUserInput = {
172-
markers: ReadonlyArray<{
173-
selected?: ReadonlyArray<string>;
172+
markers: Array<{
173+
selected?: string[];
174174
label: string;
175175
}>;
176176
};
177177

178178
export type PerseusMatcherRubric = {
179179
// Translatable Text; Static concepts to show in the left column. e.g. ["Fruit", "Color", "Clothes"]
180-
left: ReadonlyArray<string>;
180+
left: string[];
181181
// Translatable Markup; Values that represent the concepts to be correlated with the concepts. e.g. ["Red", "Shirt", "Banana"]
182-
right: ReadonlyArray<string>;
182+
right: string[];
183183
};
184184

185185
export type PerseusMatcherUserInput = {
186-
left: ReadonlyArray<string>;
187-
right: ReadonlyArray<string>;
186+
left: string[];
187+
right: string[];
188188
};
189189

190190
export type PerseusMatrixRubric = {
@@ -201,7 +201,7 @@ export type PerseusMatrixUserInput = {
201201
export type PerseusNumberLineRubric = {
202202
correctRel: string | null | undefined;
203203
correctX: number;
204-
range: ReadonlyArray<number>;
204+
range: number[];
205205
initialX: number | null | undefined;
206206
isInequality: boolean;
207207
};
@@ -211,12 +211,12 @@ export type PerseusNumberLineUserInput = {
211211
numLinePosition: number;
212212
rel: Relationship | "eq";
213213
numDivisions: number;
214-
divisionRange: ReadonlyArray<number>;
214+
divisionRange: number[];
215215
};
216216

217217
export type PerseusNumericInputRubric = {
218218
// A list of all the possible correct and incorrect answers
219-
answers: ReadonlyArray<PerseusNumericInputAnswer>;
219+
answers: PerseusNumericInputAnswer[];
220220
// A coefficient style number allows the student to use - for -1 and an empty string to mean 1.
221221
coefficient: boolean;
222222
};
@@ -228,46 +228,46 @@ export type PerseusNumericInputUserInput = {
228228
export type PerseusOrdererRubric = PerseusOrdererWidgetOptions;
229229

230230
export type PerseusOrdererUserInput = {
231-
current: ReadonlyArray<string>;
231+
current: string[];
232232
};
233233

234234
export type PerseusPlotterRubric = {
235235
// The Y values that represent the correct answer expected
236-
correct: ReadonlyArray<number>;
236+
correct: number[];
237237
} & PerseusPlotterValidationData;
238238

239239
export type PerseusPlotterValidationData = {
240240
// The Y values the graph should start with
241-
starting: ReadonlyArray<number>;
241+
starting: number[];
242242
};
243243

244-
export type PerseusPlotterUserInput = ReadonlyArray<number>;
244+
export type PerseusPlotterUserInput = number[];
245245

246246
export type PerseusRadioRubric = {
247247
// The choices provided to the user.
248-
choices: ReadonlyArray<PerseusRadioChoice>;
248+
choices: PerseusRadioChoice[];
249249
};
250250

251251
export type PerseusRadioUserInput = {
252-
choicesSelected: ReadonlyArray<boolean>;
252+
choicesSelected: boolean[];
253253
};
254254

255255
export type PerseusSorterRubric = {
256256
// Translatable Text; The correct answer (in the correct order). The user will see the cards in a randomized order.
257-
correct: ReadonlyArray<string>;
257+
correct: string[];
258258
};
259259

260260
export type PerseusSorterUserInput = {
261-
options: ReadonlyArray<string>;
261+
options: string[];
262262
changed: boolean;
263263
};
264264

265265
export type PerseusTableRubric = {
266266
// Translatable Text; A 2-dimensional array of text to populate the table with
267-
answers: ReadonlyArray<ReadonlyArray<string>>;
267+
answers: string[][];
268268
};
269269

270-
export type PerseusTableUserInput = ReadonlyArray<ReadonlyArray<string>>;
270+
export type PerseusTableUserInput = string[][];
271271

272272
export interface RubricRegistry {
273273
categorizer: PerseusCategorizerRubric;
@@ -352,7 +352,7 @@ export type UserInputMap = MakeWidgetMap<UserInputRegistry>;
352352
/**
353353
* deprecated prefer using UserInputMap
354354
*/
355-
export type UserInputArray = ReadonlyArray<
355+
export type UserInputArray = Array<
356356
UserInputArray | UserInput | null | undefined
357357
>;
358358

packages/perseus-core/src/widgets/expression/derive-extra-keys.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,12 @@ function deriveExtraKeys(
1616
widgetOptions: PerseusExpressionWidgetOptions,
1717
): KeypadConfiguration["extraKeys"] {
1818
if (widgetOptions.extraKeys) {
19-
return widgetOptions.extraKeys as ReadonlyArray<KeypadKey>;
19+
return widgetOptions.extraKeys;
2020
}
2121

2222
// If there are no extra symbols available, we include Pi anyway, so
2323
// that the "extra symbols" button doesn't appear empty.
24-
const defaultKeys: ReadonlyArray<KeypadKey> = ["PI"];
24+
const defaultKeys: KeypadKey[] = ["PI"];
2525

2626
if (widgetOptions.answerForms == null) {
2727
return defaultKeys;

packages/perseus-core/src/widgets/matcher/matcher-util.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,8 @@ export const shuffleMatcher = (
3838

3939
// TODO(LEMS-2841): Can shorten to shuffleMatcher after above function removed
4040
function shuffleMatcherWithRandom(data: MatcherShuffleInfo): {
41-
left: ReadonlyArray<string>;
42-
right: ReadonlyArray<string>;
41+
left: string[];
42+
right: string[];
4343
} {
4444
// Use the same random() function to shuffle both columns sequentially
4545
let left;

packages/perseus-editor/src/widgets/interactive-graph-editor/start-coords/start-coords-point.tsx

+2-2
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,8 @@ import CoordinatePairInput from "../../../components/coordinate-pair-input";
1010
import type {Coord} from "@khanacademy/perseus";
1111

1212
type Props = {
13-
startCoords: ReadonlyArray<Coord>;
14-
onChange: (startCoords: ReadonlyArray<Coord>) => void;
13+
startCoords: Coord[];
14+
onChange: (startCoords: Coord[]) => void;
1515
};
1616

1717
const StartCoordsPoint = (props: Props) => {

packages/perseus-editor/src/widgets/label-image-editor.tsx

+1-1
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ import type {
2525

2626
type Props = {
2727
// List of answer choices to label question image with.
28-
choices: ReadonlyArray<string>;
28+
choices: string[];
2929
// The question image properties.
3030
imageAlt: string;
3131
imageUrl: string;

packages/perseus-editor/src/widgets/label-image/marker.tsx

-2
Original file line numberDiff line numberDiff line change
@@ -162,8 +162,6 @@ export default class Marker extends React.Component<Props, State> {
162162

163163
<OptionGroup
164164
onSelected={this.handleSelectAnswer}
165-
// TODO(WB-1096): make selectedValues immutable in wonder-blocks
166-
// @ts-expect-error - TS2769 - No overload matches this call.
167165
selectedValues={answers}
168166
>
169167
{choices.map((choice) => (

packages/perseus-editor/src/widgets/label-image/question-markers.tsx

+1-1
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import type {PerseusLabelImageWidgetOptions} from "@khanacademy/perseus-core";
1515

1616
type Props = {
1717
// The list of possible answers in a specific order.
18-
choices: ReadonlyArray<string>;
18+
choices: string[];
1919
// The question image properties.
2020
imageUrl: string;
2121
imageWidth: number;

packages/perseus-editor/src/widgets/radio/editor.tsx

+1-1
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ class ChoiceEditor extends React.Component<ChoiceEditorProps> {
101101
type RadioEditorProps = {
102102
apiOptions: APIOptions;
103103
countChoices: boolean;
104-
choices: ReadonlyArray<PerseusRadioChoice>;
104+
choices: PerseusRadioChoice[];
105105
displayCount: number;
106106
randomize: boolean;
107107
hasNoneOfTheAbove: boolean;

packages/perseus-score/src/widgets/categorizer/score-categorizer.test.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ describe("scoreCategorizer", () => {
1111

1212
const userInput = {
1313
values: [1, 3],
14-
} as const;
14+
};
1515
const score = scoreCategorizer(userInput, rubric);
1616

1717
expect(score).toHaveBeenAnsweredCorrectly();
@@ -25,7 +25,7 @@ describe("scoreCategorizer", () => {
2525

2626
const userInput = {
2727
values: [2, 3],
28-
} as const;
28+
};
2929
const score = scoreCategorizer(userInput, rubric);
3030

3131
expect(score).toHaveBeenAnsweredIncorrectly();

packages/perseus-score/src/widgets/categorizer/validate-categorizer.test.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ describe("validateCategorizer", () => {
1010

1111
const userInput = {
1212
values: [2],
13-
} as const;
13+
};
1414
const score = validateCategorizer(userInput, validationData);
1515

1616
expect(score).toHaveInvalidInput("INVALID_SELECTION_ERROR");
@@ -23,7 +23,7 @@ describe("validateCategorizer", () => {
2323

2424
const userInput = {
2525
values: [2, 4],
26-
} as const;
26+
};
2727
const score = validateCategorizer(userInput, validationData);
2828

2929
expect(score).toBeNull();

packages/perseus-score/src/widgets/interactive-graph/score-interactive-graph.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -168,8 +168,9 @@ function scoreInteractiveGraph(
168168
// eq() comparison but _.isEqual(0, -0) is false, so we'll use
169169
// eq() anyway. The sort should be fine because it'll stringify
170170
// it and -0 converted to a string is "0"
171+
// TODO(benchristel): once we drop support for Safari 15, use
172+
// toSorted here to avoid mutating the input arrays!
171173
guess?.sort();
172-
// @ts-expect-error - TS2339 - Property 'sort' does not exist on type 'readonly Coord[]'.
173174
correct.sort();
174175
if (approximateDeepEqual(guess, correct)) {
175176
return {

0 commit comments

Comments
 (0)