fix(LEMS-3701): update editorPage to sync json state #3196

mark-fitzgerald · 2026-01-27T20:16:57Z

Should we also assert that "Initial content" is NOT in the document?

mark-fitzgerald · 2026-01-27T20:23:14Z

Should there be a test for valid user input when the parent updates the value?

mark-fitzgerald · 2026-01-27T20:22:27Z

I am missing something - how is this invalid user input?

It's incomplete, its missing the " and the closing bracket
{"content": "User is typing
instead of:
{"content": "User is typing"}

ivyolamit · 2026-01-27T20:05:55Z

We can remove code in line 2, if UNSAFE tag is removed

/* eslint-disable react/no-unsafe */

I wish ESLint would tell us if there's "disable" statements that are unused. 🤷‍♂️

ivyolamit · 2026-01-27T20:08:30Z

Do you know why this was marked UNSAFE?

Update, I asked Claude about this:

UNSAFE_componentWillReceiveProps = unsafe (has issues with concurrent rendering)

"componentWillReceiveProps was problematic because it was called before every render triggered by a prop change, making it difficult to distinguish between essential updates and potentially causing infinite loops or unpredictable behavior, especially with asynchronous operations. "

😓

jeremywiebe · 2026-01-27T20:17:35Z

Given that this function is used to update state, could we just use the static getDerivedStateFromProps instead? I'm not sure it's a huge difference, but it's a static method (which is nice as it is then a pure function.

Thank you for the suggestion!
I looked into it and it doesn't seem like it would work for our use case 😞
The problem we were having was that the JsonEditor wasn't syncing when the parent sent updated props (whether on load or later), so we need ongoing prop synchronization

With componentDidUpdate, we're checking if the value prop from the parent has changed, not just on initial load but throughout the component's lifetime

mark-fitzgerald · 2026-01-27T19:52:13Z

This reads much better! Thank you!!

jeremywiebe · 2026-01-27T20:19:01Z

TIL: Cool, I hadn't seen this React function before.

-Original file line number
+Diff line change
@@ -0,0 +1,6 @@
+    ---
+    "@khanacademy/perseus-editor": patch
+    ---
+    JsonEditor: Replaced deprecated lifecycle method with componentDidUpdate to sync internal state when props change
+    EditorPage: Added getSnapshotBeforeUpdate to capture editor state before switching to JSON mode

-Original file line number
+Diff line change
@@ -1,5 +1,4 @@
     /* eslint-disable @typescript-eslint/no-invalid-this */
-    /* eslint-disable react/no-unsafe */
     import * as React from "react";
     import _ from "underscore";
@@ Expand Down Expand Up / @@ -42,18 +41,28 @@ class JsonEditor extends React.Component<Props, State> { @@
             };
         }
-        UNSAFE_componentWillReceiveProps(nextProps) {
-            const shouldReplaceContent =
-                !this.state.valid ||
-                !_.isEqual(
-                    nextProps.value,
-                    JSON.parse(
-                        this.state.currentValue ? this.state.currentValue : "",
-                    ),
-                );
+        componentDidUpdate(prevProps: Props) {
+            if (!_.isEqual(prevProps.value, this.props.value)) {
+                const shouldReplaceContent =
+                    !this.state.valid ||
+                    !_.isEqual(this.props.value, this.getCurrentValueAsJson());
+                if (shouldReplaceContent) {
+                    this.setState({
+                        currentValue: JSON.stringify(this.props.value, null, 4),
+                        valid: true,
+                    });
+                }
+            }
+        }
-            if (shouldReplaceContent) {
-                this.setState(this.getInitialState());
+        getCurrentValueAsJson() {
+            try {
+                return this.state.currentValue
+                    ? JSON.parse(this.state.currentValue)
+                    : {};
+            } catch {
+                return null;
             }
         }
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up / @@ -226,4 +226,57 @@ describe("EditorPage", () => { @@
             const widgetSelect = screen.getByTestId("editor__widget-select");
             expect(widgetSelect).toBeDisabled();
         });
+        it("should sync json state when props change in JSON mode", async () => {
+            // Arrange
+            const initialQuestion: PerseusRenderer = {
+                content: "Initial content",
+                images: {},
+                widgets: {},
+            };
+            const updatedQuestion: PerseusRenderer = {
+                content: "Updated content from parent",
+                images: {},
+                widgets: {},
+            };
+            const onChangeMock = jest.fn();
+            const {rerender} = render(
+                <EditorPage
+                    dependencies={testDependenciesV2}
+                    question={initialQuestion}
+                    onChange={onChangeMock}
+                    onPreviewDeviceChange={() => {}}
+                    previewDevice="desktop"
+                    previewURL=""
+                    itemId="itemId"
+                    developerMode={true}
+                    jsonMode={true}
+                    widgetsAreOpen={true}
+                />,
+            );
+            // Act
+            rerender(
+                <EditorPage
+                    dependencies={testDependenciesV2}
+                    question={updatedQuestion}
+                    onChange={onChangeMock}
+                    onPreviewDeviceChange={() => {}}
+                    previewDevice="desktop"
+                    previewURL=""
+                    itemId="itemId"
+                    developerMode={true}
+                    jsonMode={true}
+                    widgetsAreOpen={true}
+                />,
+            );
+            // Assert
+            expect(
+                screen.getByDisplayValue(/Updated content from parent/),
+            ).toBeInTheDocument();
+        });
     });

-Original file line number
+Diff line change
@@ Expand Up / @@ -122,7 +122,21 @@ class EditorPage extends React.Component<Props, State> { @@
             this.updateRenderer();
         }
-        componentDidUpdate() {
+        getSnapshotBeforeUpdate(prevProps: Props, prevState: State) {
+            if (!prevProps.jsonMode && this.props.jsonMode) {
+                return {
+                    ...(this.itemEditor.current?.serialize({
+                        keepDeletedWidgets: true,
+                    }) ?? {}),
+                    hints: this.hintsEditor.current?.serialize({
+                        keepDeletedWidgets: true,
+                    }),
+                };
+            }
+            return null;
+        }
+        componentDidUpdate(previousProps: Props, prevState: State, snapshot: any) {
             // NOTE: It is required to delay the preview update until after the
             // current frame, to allow for ItemEditor to render its widgets.
             // This then enables to serialize the widgets properties correctly,
@@ Expand All / @@ -133,12 +147,47 @@ class EditorPage extends React.Component<Props, State> { @@
             setTimeout(() => {
                 this.updateRenderer();
             });
+            // Use serialized snapshot from before unmount
+            if (snapshot) {
+                this.setState({json: snapshot});
+                return;
+            }
+            if (
+                !_.isEqual(previousProps.question, this.props.question) ||
+                !_.isEqual(previousProps.answerArea, this.props.answerArea) ||
+                !_.isEqual(previousProps.hints, this.props.hints)
+            ) {
+                this.syncJsonStateFromProps();
+            }
         }
         componentWillUnmount() {
             this._isMounted = false;
         }
+        /**
+         * Updates JSON state when props change from the parent.
+         *
+         * `state.json` is initialized once in the constructor. If the
+         * Frontend sends fresh data while the editor is already mounted,
+         * we need to update state.json to reflect those changes.
+         */
+        syncJsonStateFromProps() {
+            if (!this.props.question) {
+                return;
+            }
+            this.setState({
+                json: {
+                    question: this.props.question,
+                    answerArea: this.props.answerArea,
+                    hints: this.props.hints as Hint[],
+                },
+            });
+        }
         toggleJsonMode: () => void = () => {
             this.setState(
                 {
@@ Expand Down @@

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(LEMS-3701): update editorPage to sync json state #3196

Uh oh!

Diff view

Diff view

There are no files selected for viewing

mark-fitzgerald Jan 27, 2026 •

edited

Loading

Uh oh!

mark-fitzgerald Jan 27, 2026

Uh oh!

mark-fitzgerald Jan 27, 2026

Uh oh!

anakaren-rojas Jan 27, 2026

Uh oh!

ivyolamit Jan 27, 2026

Uh oh!

jeremywiebe Jan 27, 2026

Uh oh!

ivyolamit Jan 27, 2026

Uh oh!

ivyolamit Jan 27, 2026

Uh oh!

anakaren-rojas Jan 28, 2026

Uh oh!

jeremywiebe Jan 27, 2026

Uh oh!

anakaren-rojas Jan 27, 2026

Uh oh!

mark-fitzgerald Jan 27, 2026

Uh oh!

jeremywiebe Jan 27, 2026

Uh oh!

Uh oh!

-Original file line number
+Diff line change
@@ -0,0 +1,211 @@
+    import {render, screen} from "@testing-library/react";
+    import {userEvent as userEventLib} from "@testing-library/user-event";
+    import * as React from "react";
+    import JsonEditor from "./json-editor";
+    import type {UserEvent} from "@testing-library/user-event";
+    describe("JsonEditor", () => {
+        let userEvent: UserEvent;
+        beforeEach(() => {
+            userEvent = userEventLib.setup({
+                advanceTimers: jest.advanceTimersByTime,
+            });
+        });
+        it("should render with initial value", () => {
+            // Arrange
+            const initialValue = {
+                content: "Test content",
+                widgets: {},
+            };
+            // Act
+            render(
+                <JsonEditor
+                    multiLine={true}
+                    value={initialValue}
+                    onChange={() => {}}
+                    editingDisabled={false}
+                />,
+            );
+            // Assert
+            expect(screen.getByDisplayValue(/Test content/)).toBeInTheDocument();
+        });
+        it("should update when value prop changes", () => {
+            // Arrange
+            const initialValue = {
+                content: "Initial content",
+                widgets: {},
+            };
+            const updatedValue = {
+                content: "Updated content",
+                widgets: {},
+            };
+            const {rerender} = render(
+                <JsonEditor
+                    multiLine={true}
+                    value={initialValue}
+                    onChange={() => {}}
+                    editingDisabled={false}
+                />,
+            );
+            // Act
+            rerender(
+                <JsonEditor
+                    multiLine={true}
+                    value={updatedValue}
+                    onChange={() => {}}
+                    editingDisabled={false}
+                />,
+            );
+            // Assert
+            expect(
+                screen.queryByDisplayValue(/Initial content/),
+            ).not.toBeInTheDocument();
+            expect(screen.getByDisplayValue(/Updated content/)).toBeInTheDocument();
+        });
+        it("should call onChange when valid JSON is entered", async () => {
+            // Arrange
+            const onChangeMock = jest.fn();
+            const initialValue = {content: "test"};
+            render(
+                <JsonEditor
+                    multiLine={true}
+                    value={initialValue}
+                    onChange={onChangeMock}
+                    editingDisabled={false}
+                />,
+            );
+            const textarea = screen.getByRole("textbox");
+            // Act
+            await userEvent.clear(textarea);
+            textarea.focus();
+            await userEvent.paste('{"content": "new content"}');
+            // Assert
+            expect(onChangeMock).toHaveBeenCalledWith({content: "new content"});
+        });
+        it("should not call onChange for invalid JSON", async () => {
+            // Arrange
+            const onChangeMock = jest.fn();
+            const initialValue = {content: "test"};
+            render(
+                <JsonEditor
+                    multiLine={true}
+                    value={initialValue}
+                    onChange={onChangeMock}
+                    editingDisabled={false}
+                />,
+            );
+            const textarea = screen.getByRole("textbox");
+            // Act
+            await userEvent.clear(textarea);
+            textarea.focus();
+            await userEvent.paste("{invalid json");
+            // Assert
+            expect(onChangeMock).not.toHaveBeenCalled();
+        });
+        it("should be disabled when editingDisabled is true", () => {
+            // Arrange
+            const initialValue = {content: "test"};
+            // Act
+            render(
+                <JsonEditor
+                    multiLine={true}
+                    value={initialValue}
+                    onChange={() => {}}
+                    editingDisabled={true}
+                />,
+            );
+            // Assert
+            expect(screen.getByRole("textbox")).toBeDisabled();
+        });
+        it("should replace valid user input when parent updates value", async () => {
+            // Arrange
+            const initialValue = {content: "Initial"};
+            const updatedValue = {content: "Updated from parent"};
+            const {rerender} = render(
+                <JsonEditor
+                    multiLine={true}
+                    value={initialValue}
+                    onChange={() => {}}
+                    editingDisabled={false}
+                />,
+            );
+            const textarea = screen.getByRole("textbox") as HTMLTextAreaElement;
+            // Act
+            await userEvent.clear(textarea);
+            textarea.focus();
+            await userEvent.paste('{"content": "Valid user input"}');
+            rerender(
+                <JsonEditor
+                    multiLine={true}
+                    value={updatedValue}
+                    onChange={() => {}}
+                    editingDisabled={false}
+                />,
+            );
+            // Assert
+            expect(textarea.value).toContain("Updated from parent");
+        });
+        it("should replace invalid user input when parent updates value", async () => {
+            // Arrange
+            const initialValue = {content: "Initial"};
+            const updatedValue = {content: "Updated from parent"};
+            const {rerender} = render(
+                <JsonEditor
+                    multiLine={true}
+                    value={initialValue}
+                    onChange={() => {}}
+                    editingDisabled={false}
+                />,
+            );
+            const textarea = screen.getByRole("textbox") as HTMLTextAreaElement;
+            // Act
+            await userEvent.clear(textarea);
+            textarea.focus();
+            await userEvent.paste('{"content": "User is typing');
+            rerender(
+                <JsonEditor
+                    multiLine={true}
+                    value={updatedValue}
+                    onChange={() => {}}
+                    editingDisabled={false}
+                />,
+            );
+            // Assert
+            expect(textarea.value).toContain("Updated from parent");
+        });
+    });

fix(LEMS-3701): update editorPage to sync json state #3196

Are you sure you want to change the base?

Uh oh!

fix(LEMS-3701): update editorPage to sync json state #3196

Uh oh!

Uh oh!

Diff view

Diff view

There are no files selected for viewing

mark-fitzgerald Jan 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mark-fitzgerald Jan 27, 2026 •

edited

Loading