Skip to content

Commit

Permalink
Deprecate metadata in renderer, hint, and Group widget data schemas (
Browse files Browse the repository at this point in the history
…#2242)

We aren't using `metadata`. As near as I can tell, [it was added in 2014][1] so content
creators could add "tags" to content, but I don't know why it was added or if it was
ever used.

We are seeing Perseus JSON parser errors in webapp because `metadata` is null in some
exercises. This PR fixes the errors by changing the type of `metadata` to `any`, marking
it deprecated, and removing references to it.

[1]: https://phabricator.khanacademy.org/D13929

Issue: none

## Test plan:

- Install the build of Perseus from this PR
- Visit https://www.khanacademy.dev/internal-courses/test-everything/test-everything-1/te-group/a/group-article
- You should be able to view and edit the article without errors.

Author: benchristel

Reviewers: handeyeco, benchristel, jeremywiebe

Required Reviewers:

Approved By: handeyeco

Checks: ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ⏹️  [cancelled] Publish npm snapshot (ubuntu-latest, 20.x), ⏹️  [cancelled] Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ⏹️  [cancelled] Cypress (ubuntu-latest, 20.x), ⏹️  [cancelled] Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ⏹️  [cancelled] Check builds for changes in size (ubuntu-latest, 20.x), ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ⏹️  [cancelled] Check builds for changes in size (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ⏹️  [cancelled] Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x)

Pull Request URL: #2242
  • Loading branch information
benchristel authored Feb 18, 2025
1 parent 62ed407 commit e187c6b
Show file tree
Hide file tree
Showing 10 changed files with 28 additions and 38 deletions.
7 changes: 7 additions & 0 deletions .changeset/tall-llamas-crash.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@khanacademy/perseus": minor
"@khanacademy/perseus-core": minor
"@khanacademy/perseus-editor": minor
---

Deprecate the `metadata` field in renderer, hint, and Group widget data schemas.
12 changes: 8 additions & 4 deletions packages/perseus-core/src/data-schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -244,10 +244,14 @@ export type PerseusRenderer = {
* field.
*/
widgets: PerseusWidgetsMap;
// Used in the PerseusGradedGroup widget. A list of "tags" that are keys
// that represent other content in the system. Not rendered to the user.
// NOTE: perseus_data.go says this is required even though it isn't necessary.
metadata?: ReadonlyArray<string>;
/**
* Formerly used in the PerseusGradedGroup widget. A list of "tags" that
* are keys that represent other content in the system. Not rendered to
* the user. NOTE: perseus_data.go says this is required even though it
* isn't necessary.
* @deprecated
*/
metadata?: any;
/**
* A dictionary of {[imageUrl]: PerseusImageDetail}.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ describe("parseAndMigratePerseusArticle", () => {
content: "",
widgets: {},
images: {},
metadata: undefined,
}),
);
});
Expand All @@ -80,8 +79,8 @@ describe("parseAndMigratePerseusArticle", () => {
);
expect(result).toEqual(
success([
{content: "one", widgets: {}, images: {}, metadata: undefined},
{content: "two", widgets: {}, images: {}, metadata: undefined},
{content: "one", widgets: {}, images: {}},
{content: "two", widgets: {}, images: {}},
]),
);
});
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import {
array,
any,
boolean,
object,
optional,
Expand All @@ -17,6 +17,7 @@ export const parseHint: Parser<Hint> = object({
replace: optional(boolean),
content: string,
widgets: defaulted(parseWidgetsMap, () => ({})),
metadata: optional(array(string)),
images: parseImages,
// deprecated
metadata: any,
});
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {array, object, optional, string} from "../general-purpose-parsers";
import {any, object, string} from "../general-purpose-parsers";
import {defaulted} from "../general-purpose-parsers/defaulted";

import {parseImages} from "./images-map";
Expand All @@ -20,8 +20,9 @@ export const parsePerseusRenderer: Parser<PerseusRenderer> = defaulted(
(rawVal, ctx) => parseWidgetsMap(rawVal, ctx),
() => ({}),
),
metadata: optional(array(string)),
images: parseImages,
// deprecated
metadata: any,
}),
// Default value
() => ({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,6 @@ describe("parseWidgetsMap", () => {
options: {
content: "",
widgets: {},
metadata: [],
images: {},
},
},
Expand Down
5 changes: 1 addition & 4 deletions packages/perseus-core/src/widgets/group/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,13 @@ import type {WidgetLogic} from "../logic-export.types";

export type GroupDefaultWidgetOptions = Pick<
PerseusGroupWidgetOptions,
"content" | "widgets" | "images" | "metadata"
"content" | "widgets" | "images"
>;

const defaultWidgetOptions: GroupDefaultWidgetOptions = {
content: "",
widgets: {},
images: {},
// `undefined` instead of `null` so that getDefaultProps works for
// `the GroupMetadataEditor`
metadata: undefined,
};

const groupWidgetLogic: WidgetLogic = {
Expand Down
1 change: 0 additions & 1 deletion packages/perseus-editor/src/__tests__/traversal.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,6 @@ const sampleGroupUpgraded = {
alignment: "default",
},
},
metadata: undefined,
},
version: {
major: 0,
Expand Down
21 changes: 1 addition & 20 deletions packages/perseus-editor/src/widgets/group-editor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ class GroupEditor extends React.Component<Props> {
content: PropTypes.string,
widgets: PropTypes.object,
images: PropTypes.object,
metadata: PropTypes.any,
apiOptions: ApiOptions.propTypes,
};

Expand All @@ -30,17 +29,6 @@ class GroupEditor extends React.Component<Props> {

editor = React.createRef<Editor>();

_renderMetadataEditor: () => React.ReactElement = () => {
const GroupMetadataEditor = this.props.apiOptions.GroupMetadataEditor;
return (
<GroupMetadataEditor
value={this.props.metadata}
// @ts-expect-error - TS2554 - Expected 3 arguments, but got 1.
onChange={this.change("metadata")}
/>
);
};

change: (arg1: any, arg2: any, arg3: any) => any = (...args) => {
return Changeable.change.apply(this, args);
};
Expand All @@ -50,19 +38,12 @@ class GroupEditor extends React.Component<Props> {
};

serialize: () => any = () => {
return _.extend({}, this.editor.current?.serialize(), {
metadata: this.props.metadata,
});
return _.extend({}, this.editor.current?.serialize());
};

render(): React.ReactNode {
return (
<div className="perseus-group-editor">
<div>
{/* the metadata editor; used for tags on
khanacademy.org */}
{this._renderMetadataEditor()}
</div>
<Editor
ref={this.editor}
content={this.props.content}
Expand Down
4 changes: 3 additions & 1 deletion packages/perseus/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,9 @@ export type APIOptions = Readonly<{
keypadHeight?: number,
focusedElement?: HTMLElement,
) => unknown;
/**
* @deprecated - metadata is no longer used by the Group widget
*/
GroupMetadataEditor?: React.ComponentType<StubTagEditorType>;
showAlignmentOptions?: boolean;
/**
Expand Down Expand Up @@ -425,7 +428,6 @@ export interface PerseusDependenciesV2 {
*/
export type APIOptionsWithDefaults = Readonly<
APIOptions & {
GroupMetadataEditor: NonNullable<APIOptions["GroupMetadataEditor"]>;
baseElements: NonNullable<APIOptions["baseElements"]>;
canScrollPage: NonNullable<APIOptions["canScrollPage"]>;
crossOutEnabled: NonNullable<APIOptions["crossOutEnabled"]>;
Expand Down

0 comments on commit e187c6b

Please sign in to comment.