diff --git a/src/components/Editor/Project/ScratchContainer.jsx b/src/components/Editor/Project/ScratchContainer.jsx index 7c5748843..db4a434f7 100644 --- a/src/components/Editor/Project/ScratchContainer.jsx +++ b/src/components/Editor/Project/ScratchContainer.jsx @@ -4,6 +4,10 @@ import { ClickScrollPlugin, OverlayScrollbars } from "overlayscrollbars"; import { OverlayScrollbarsComponent } from "overlayscrollbars-react"; import { applyScratchProjectIdentifierUpdate } from "../../../redux/EditorSlice"; import { runStartedEvent } from "../../../events/WebComponentCustomEvents"; +import { + cancelPendingRunEventDebounce, + scheduleRunEventCycle, +} from "../../WebComponentProject/runEventCodeSnapshot"; import { subscribeToScratchProjectIdentifierUpdates, postMessageToScratchIframe, @@ -45,6 +49,8 @@ export default function ScratchContainer() { nonce: null, hadAccessToken: false, }); + const projectIdentifierRef = useRef(projectIdentifier); + projectIdentifierRef.current = projectIdentifier; useEffect(() => { return subscribeToScratchProjectIdentifierUpdates( @@ -74,12 +80,22 @@ export default function ScratchContainer() { if (event.origin !== allowedOrigin) return; if (event.data?.type !== "scratch-gui-project-run-started") return; - document.dispatchEvent(runStartedEvent({})); + scheduleRunEventCycle( + projectIdentifierRef.current, + null, + { bypassSnapshot: true }, + { + onRunStarted: () => { + document.dispatchEvent(runStartedEvent({})); + }, + }, + ); }; window.addEventListener("message", handleScratchRunStarted); return () => { window.removeEventListener("message", handleScratchRunStarted); + cancelPendingRunEventDebounce(); }; }, []); diff --git a/src/components/Editor/Project/ScratchContainer.test.js b/src/components/Editor/Project/ScratchContainer.test.js index 77c410abf..5563c7647 100644 --- a/src/components/Editor/Project/ScratchContainer.test.js +++ b/src/components/Editor/Project/ScratchContainer.test.js @@ -7,6 +7,7 @@ import * as scratchIframeUtils from "../../../utils/scratchIframe"; import webComponentStore from "../../../redux/stores/WebComponentStore"; import { setUser } from "../../../redux/WebComponentAuthSlice"; import { resetStore } from "../../../redux/RootSlice"; +import { resetRunEventCodeSnapshot } from "../../WebComponentProject/runEventCodeSnapshot"; jest.mock("../../../utils/scratchIframe", () => ({ ...jest.requireActual("../../../utils/scratchIframe"), @@ -51,14 +52,17 @@ describe("ScratchContainer", () => { scratchApiEndpoint: "https://api.example.com/v1", }; - const buildStore = ({ authReducer } = {}) => + const buildStore = ({ authReducer, editorOverrides } = {}) => configureStore({ reducer: { editor: EditorReducer, ...(authReducer ? { auth: authReducer } : {}), }, preloadedState: { - editor: defaultEditorState, + editor: { + ...defaultEditorState, + ...editorOverrides, + }, }, }); @@ -174,14 +178,23 @@ describe("ScratchContainer", () => { let runStartedHandler; beforeEach(() => { + jest.useFakeTimers(); + resetRunEventCodeSnapshot(); runStartedHandler = jest.fn(); document.addEventListener("editor-runStarted", runStartedHandler); }); afterEach(() => { + jest.useRealTimers(); document.removeEventListener("editor-runStarted", runStartedHandler); }); + const flushScratchRunDebounce = () => { + act(() => { + jest.advanceTimersByTime(250); + }); + }; + test("dispatches editor-runStarted when scratch-gui-project-run-started is received", () => { renderScratchContainer(); @@ -191,6 +204,8 @@ describe("ScratchContainer", () => { }); }); + flushScratchRunDebounce(); + expect(runStartedHandler).toHaveBeenCalledTimes(1); expect(runStartedHandler.mock.calls[0][0].detail).toEqual({}); }); @@ -211,6 +226,157 @@ describe("ScratchContainer", () => { expect(runStartedHandler).not.toHaveBeenCalled(); }); + + test("does not dispatch editor-runStarted when unmounted before debounce fires", () => { + const store = buildStore(); + const { unmount } = render( + + + , + ); + + act(() => { + dispatchMessage({ type: "scratch-gui-project-run-started" }); + }); + + unmount(); + flushScratchRunDebounce(); + + expect(runStartedHandler).not.toHaveBeenCalled(); + }); + + test("dispatches editor-runStarted when project identifier changes during debounce", () => { + const store = buildStore(); + const view = render( + + + , + ); + + act(() => { + dispatchMessage({ type: "scratch-gui-project-run-started" }); + }); + + const updatedStore = buildStore({ + editorOverrides: { + project: { + identifier: "project-456", + project_type: "code_editor_scratch", + }, + scratchIframeProjectIdentifier: "project-456", + }, + }); + + view.rerender( + + + , + ); + + flushScratchRunDebounce(); + + expect(runStartedHandler).toHaveBeenCalledTimes(1); + }); + + test("collapses rapid scratch runs into one debounced dispatch", () => { + renderScratchContainer(); + + act(() => { + dispatchMessage({ + type: "scratch-gui-project-run-started", + }); + }); + + act(() => { + dispatchMessage({ + type: "scratch-gui-project-run-started", + }); + }); + + expect(runStartedHandler).toHaveBeenCalledTimes(0); + + flushScratchRunDebounce(); + + expect(runStartedHandler).toHaveBeenCalledTimes(1); + }); + + test("allows separate scratch bursts after debounce quiet period", () => { + renderScratchContainer(); + + act(() => { + dispatchMessage({ + type: "scratch-gui-project-run-started", + }); + }); + + flushScratchRunDebounce(); + + act(() => { + jest.advanceTimersByTime(250); + }); + + act(() => { + dispatchMessage({ + type: "scratch-gui-project-run-started", + }); + }); + + flushScratchRunDebounce(); + + expect(runStartedHandler).toHaveBeenCalledTimes(2); + }); + + test("collapses rapid read-only scratch runs into one debounced dispatch", () => { + renderScratchContainer( + buildStore({ editorOverrides: { readOnly: true } }), + ); + + act(() => { + dispatchMessage({ + type: "scratch-gui-project-run-started", + }); + }); + + act(() => { + dispatchMessage({ + type: "scratch-gui-project-run-started", + }); + }); + + expect(runStartedHandler).toHaveBeenCalledTimes(0); + + flushScratchRunDebounce(); + + expect(runStartedHandler).toHaveBeenCalledTimes(1); + }); + + test("allows read-only scratch runs after the debounce window", () => { + renderScratchContainer( + buildStore({ editorOverrides: { readOnly: true } }), + ); + + act(() => { + dispatchMessage({ + type: "scratch-gui-project-run-started", + }); + }); + + flushScratchRunDebounce(); + + act(() => { + jest.advanceTimersByTime(250); + }); + + act(() => { + dispatchMessage({ + type: "scratch-gui-project-run-started", + }); + }); + + flushScratchRunDebounce(); + + expect(runStartedHandler).toHaveBeenCalledTimes(2); + }); }); test("updates the parent project identifier without reloading the iframe project_id", () => { diff --git a/src/components/Editor/Runners/HtmlRunner/HtmlRunner.jsx b/src/components/Editor/Runners/HtmlRunner/HtmlRunner.jsx index 6cee298da..ed4c541bf 100644 --- a/src/components/Editor/Runners/HtmlRunner/HtmlRunner.jsx +++ b/src/components/Editor/Runners/HtmlRunner/HtmlRunner.jsx @@ -88,10 +88,9 @@ function HtmlRunner() { const [runningFile, setRunningFile] = useState(previewFile); const previewFileRef = useRef(previewFile); - const showModal = () => { + const showModal = useCallback(() => { dispatch(showErrorModal()); - eventListener(); - }; + }, [dispatch]); const { externalLink, @@ -100,34 +99,48 @@ function HtmlRunner() { handleExternalLinkError, } = useExternalLinkState(showModal); - const eventListener = () => { - window.addEventListener("message", (event) => { - if (event.data?.type === MSG_HTML_PREVIEW_EVENT) { - if (event.data?.msg === "ERROR: External link") { - handleExternalLinkError(showModal); - } else if (event.data?.msg === "Allowed external link") { - handleAllowedExternalLink(event.data.payload.linkTo); - } else if (event.data?.msg === "RELOAD") { - const nextPreviewFile = `${event.data.payload.linkTo}.html`; - setExternalLink(null); - - if (nextPreviewFile === previewFileRef.current) { - reloadAfterPreviewChange.current = false; - dispatch(triggerCodeRun()); - } else { - reloadAfterPreviewChange.current = true; - setPreviewFile(nextPreviewFile); - } + const handleAllowedExternalLinkRef = useRef(handleAllowedExternalLink); + const handleExternalLinkErrorRef = useRef(handleExternalLinkError); + handleAllowedExternalLinkRef.current = handleAllowedExternalLink; + handleExternalLinkErrorRef.current = handleExternalLinkError; + + useEffect(() => { + const handleIframeMessage = (event) => { + const { data } = event; + + if (data?.type === MSG_HTML_PREVIEW_READY) { + setRendererReady(true); + return; + } + + if (data?.type !== MSG_HTML_PREVIEW_EVENT) { + return; + } + + if (data.msg === "ERROR: External link") { + handleExternalLinkErrorRef.current(); + } else if (data.msg === "Allowed external link") { + handleAllowedExternalLinkRef.current(data.payload.linkTo); + } else if (data.msg === "RELOAD") { + const nextPreviewFile = `${data.payload.linkTo}.html`; + setExternalLink(null); + + if (nextPreviewFile === previewFileRef.current) { + reloadAfterPreviewChange.current = false; + dispatch(triggerCodeRun()); + } else { + reloadAfterPreviewChange.current = true; + setPreviewFile(nextPreviewFile); } } - }); - }; + }; - useEffect(() => { - eventListener(); + window.addEventListener("message", handleIframeMessage); dispatch(loadingRunner("html")); dispatch(setLoadedRunner("html")); - }, []); + + return () => window.removeEventListener("message", handleIframeMessage); + }, [dispatch, setExternalLink]); let timeout; @@ -172,21 +185,6 @@ function HtmlRunner() { } }, [runningFile]); - useEffect(() => { - window.addEventListener("message", listener); - return () => window.removeEventListener("message", listener); - }); - - const listener = useCallback( - (event) => { - const message = event.data; - if (message?.type === MSG_HTML_PREVIEW_READY) { - setRendererReady(true); - } - }, - [setRendererReady], - ); - const runCode = () => { setRunningFile(previewFile); diff --git a/src/components/Editor/Runners/HtmlRunner/HtmlRunner.test.js b/src/components/Editor/Runners/HtmlRunner/HtmlRunner.test.js index 1da74758b..f5aec0e94 100644 --- a/src/components/Editor/Runners/HtmlRunner/HtmlRunner.test.js +++ b/src/components/Editor/Runners/HtmlRunner/HtmlRunner.test.js @@ -16,6 +16,7 @@ import { MemoryRouter } from "react-router-dom"; import { matchMedia, setMedia } from "mock-match-media"; import { MOBILE_BREAKPOINT } from "../../../../utils/mediaQueryBreakpoints"; import { + MSG_HTML_PREVIEW_EVENT, MSG_HTML_PREVIEW_READY, MSG_HTML_PROJECT_UPDATE, } from "../../../../utils/iframeUtils"; @@ -444,3 +445,75 @@ describe("When run is triggered", () => { }); }); }); + +describe("iframe message listener", () => { + const htmlRunnerState = { + project: { + components: [indexPage], + }, + focussedFileIndices: [0], + openFiles: [["index.html"]], + codeHasBeenRun: true, + errorModalShowing: false, + }; + + const renderHtmlRunner = (store) => + render( + + +
+ +
+
+
, + ); + + test("does not handle preview messages after unmount", () => { + const mockStore = configureStore([]); + const store = mockStore({ editor: htmlRunnerState }); + const { unmount } = renderHtmlRunner(store); + const actionsAfterMount = store.getActions().length; + + unmount(); + + act(() => { + window.dispatchEvent( + new MessageEvent("message", { + data: { + type: MSG_HTML_PREVIEW_EVENT, + msg: "RELOAD", + payload: { linkTo: "index" }, + }, + }), + ); + }); + + expect(store.getActions().length).toBe(actionsAfterMount); + }); + + test("dispatches triggerCodeRun once per RELOAD message after remount", () => { + const mockStore = configureStore([]); + const store = mockStore({ editor: htmlRunnerState }); + const first = renderHtmlRunner(store); + first.unmount(); + + renderHtmlRunner(store); + + act(() => { + window.dispatchEvent( + new MessageEvent("message", { + data: { + type: MSG_HTML_PREVIEW_EVENT, + msg: "RELOAD", + payload: { linkTo: "index" }, + }, + }), + ); + }); + + const runActions = store + .getActions() + .filter((action) => action.type === triggerCodeRun().type); + expect(runActions).toHaveLength(1); + }); +}); diff --git a/src/components/WebComponentProject/WebComponentProject.jsx b/src/components/WebComponentProject/WebComponentProject.jsx index dcebad3ce..582f6e4a4 100644 --- a/src/components/WebComponentProject/WebComponentProject.jsx +++ b/src/components/WebComponentProject/WebComponentProject.jsx @@ -1,4 +1,4 @@ -import React, { useEffect, useRef, useState } from "react"; +import React, { useEffect, useRef } from "react"; import { useDispatch, useSelector } from "react-redux"; import { useMediaQuery } from "react-responsive"; import { marked } from "marked"; @@ -25,6 +25,18 @@ import { runStartedEvent, stepChangedEvent, } from "../../events/WebComponentCustomEvents"; +import { + endRunEventCycle, + handleRunEndedForEventCycle, + scheduleRunEventCycle, +} from "./runEventCodeSnapshot"; +import { + getPrevCodeRunTriggered, + setPrevCodeRunTriggered, + syncRunEventTrackingProject, +} from "./runEventTrackingState"; + +export { resetCodeRunEventTracking } from "./runEventTrackingState"; const WebComponentProject = ({ withProjectbar = false, @@ -50,10 +62,13 @@ const WebComponentProject = ({ (state) => state.editor.codeRunTriggered, ); - const error = useSelector((state) => state.editor.error); + const error = useSelector((state) => state.editor.error) ?? ""; const errorDetails = useSelector((state) => state.editor.errorDetails); const friendlyError = useSelector((state) => state.editor.friendlyError); - const codeHasBeenRun = useSelector((state) => state.editor.codeHasBeenRun); + const projectComponents = useSelector( + (state) => state.editor.project.components, + ); + const readOnly = useSelector((state) => state.editor.readOnly); const projectInstructions = useSelector( (state) => state.editor.project.instructions, ); @@ -64,11 +79,33 @@ const WebComponentProject = ({ (state) => state.instructions.permitOverride, ); const isMobile = useMediaQuery({ query: MOBILE_MEDIA_QUERY }); - const [codeHasRun, setCodeHasRun] = useState(codeHasBeenRun); - const prevCodeRunTriggeredRef = useRef(false); const dispatch = useDispatch(); const renderer = new marked.Renderer(); + const buildRunCompletedPayloadRef = useRef(() => ({})); + buildRunCompletedPayloadRef.current = () => { + const mz_criteria = Sk.sense_hat + ? Sk.sense_hat.mz_criteria + : { ...defaultMZCriteria }; + + return outputOnly + ? { + errorDetails, + step: currentStepPosition, + projectIdentifier, + projectType, + } + : { + isErrorFree: error === "", + step: currentStepPosition, + errorDetails, + friendlyErrorShown: Boolean(friendlyError?.html), + projectIdentifier, + projectType, + ...mz_criteria, + }; + }; + useEffect(() => { dispatch(setIsSplitView(outputSplitView)); dispatch(setWebComponent(true)); @@ -77,7 +114,6 @@ const WebComponentProject = ({ }, [editableInstructions, outputSplitView, outputOnly, dispatch]); useEffect(() => { - setCodeHasRun(false); const timeout = setTimeout(() => { document.dispatchEvent(codeChangedEvent({ step: currentStepPosition })); }, 2000); @@ -122,47 +158,51 @@ const WebComponentProject = ({ }, [dispatch, projectInstructions, permitInstructionsOverride]); useEffect(() => { - if (codeRunTriggered && !prevCodeRunTriggeredRef.current) { - document.dispatchEvent( - runStartedEvent({ - step: currentStepPosition, - projectIdentifier, - projectType, - }), + syncRunEventTrackingProject(projectIdentifier, codeRunTriggered); + }, [projectIdentifier, codeRunTriggered]); + + useEffect(() => { + const wasTriggered = getPrevCodeRunTriggered(); + + if (codeRunTriggered && !wasTriggered) { + scheduleRunEventCycle( + projectIdentifier, + projectComponents, + { bypassSnapshot: readOnly }, + { + onRunStarted: () => { + document.dispatchEvent( + runStartedEvent({ + step: currentStepPosition, + projectIdentifier, + projectType, + }), + ); + }, + onRunCompletedIfRunAlreadyEnded: () => { + document.dispatchEvent( + runCompletedEvent(buildRunCompletedPayloadRef.current()), + ); + }, + }, ); - setCodeHasRun(true); } - prevCodeRunTriggeredRef.current = codeRunTriggered; - }, [codeRunTriggered, currentStepPosition, projectIdentifier, projectType]); - useEffect(() => { - if (!codeRunTriggered && codeHasRun) { - const mz_criteria = Sk.sense_hat - ? Sk.sense_hat.mz_criteria - : { ...defaultMZCriteria }; - - const payload = outputOnly - ? { - errorDetails, - step: currentStepPosition, - projectIdentifier, - projectType, - } - : { - isErrorFree: error === "", - step: currentStepPosition, - errorDetails, - friendlyErrorShown: Boolean(friendlyError?.html), - projectIdentifier, - projectType, - ...mz_criteria, - }; - - document.dispatchEvent(runCompletedEvent(payload)); + if (!codeRunTriggered && wasTriggered) { + handleRunEndedForEventCycle({ + onRunCompleted: () => { + document.dispatchEvent( + runCompletedEvent(buildRunCompletedPayloadRef.current()), + ); + }, + }); + + endRunEventCycle(); } + + setPrevCodeRunTriggered(codeRunTriggered); }, [ codeRunTriggered, - codeHasRun, outputOnly, error, errorDetails, @@ -170,6 +210,8 @@ const WebComponentProject = ({ currentStepPosition, projectIdentifier, projectType, + readOnly, + projectComponents, ]); useEffect(() => { diff --git a/src/components/WebComponentProject/WebComponentProject.test.js b/src/components/WebComponentProject/WebComponentProject.test.js index 3f8f8132d..a529b09cf 100644 --- a/src/components/WebComponentProject/WebComponentProject.test.js +++ b/src/components/WebComponentProject/WebComponentProject.test.js @@ -2,7 +2,10 @@ import React from "react"; import { act, render, screen } from "@testing-library/react"; import { Provider } from "react-redux"; import configureStore from "redux-mock-store"; -import WebComponentProject from "./WebComponentProject"; +import WebComponentProject, { + resetCodeRunEventTracking, +} from "./WebComponentProject"; +import { RUN_EVENT_DEBOUNCE_MS } from "./runEventCodeSnapshot"; const codeChangedHandler = jest.fn(); const runStartedHandler = jest.fn(); @@ -18,6 +21,16 @@ beforeAll(() => { jest.useFakeTimers(); +const flushRunEventDebounce = () => { + act(() => { + jest.advanceTimersByTime(RUN_EVENT_DEBOUNCE_MS); + }); +}; + +beforeEach(() => { + resetCodeRunEventTracking(); +}); + let store; const renderWebComponentProject = ({ @@ -78,6 +91,7 @@ describe("When state set", () => { instructions: "My amazing instructions", codeRunTriggered: true, }); + flushRunEventDebounce(); }); test("Renders", () => { @@ -246,12 +260,67 @@ describe("When overriding instructions is not permitted", () => { }); describe("When code run finishes", () => { - test("Triggers runCompletedEvent", () => { - renderWebComponentProject({ - projectType: "python", + const renderRunCompletion = (editorOverrides = {}, props = {}) => { + const middlewares = []; + const mockStore = configureStore(middlewares); + const baseEditor = { + project: { + identifier: "test-project", + project_type: "python", + components: [ + { name: "main", extension: "py", content: "print('hello')" }, + ], + image_list: [], + }, + loading: "success", + openFiles: [], + focussedFileIndices: [], + codeRunTriggered: true, codeHasBeenRun: true, + error: "", + errorDetails: undefined, + friendlyError: undefined, + ...editorOverrides, + }; + const initialState = { + editor: baseEditor, + instructions: { + currentStepPosition: 3, + permitOverride: true, + }, + auth: {}, + }; + store = mockStore(initialState); + + const view = render( + + + , + ); + + store = mockStore({ + ...initialState, + editor: { ...baseEditor, codeRunTriggered: false }, }); - expect(runCompletedHandler).toHaveBeenCalled(); + + view.rerender( + + + , + ); + + flushRunEventDebounce(); + + return view; + }; + + beforeEach(() => { + runCompletedHandler.mockClear(); + }); + + test("Triggers runCompletedEvent", () => { + renderRunCompletion(); + expect(runCompletedHandler).toHaveBeenCalledTimes(1); expect(runCompletedHandler.mock.lastCall[0].detail).toEqual( expect.objectContaining({ isErrorFree: true, @@ -264,24 +333,114 @@ describe("When code run finishes", () => { ); }); + test("reports isErrorFree when error state is undefined", () => { + renderRunCompletion({ error: undefined }); + + expect(runCompletedHandler).toHaveBeenCalledTimes(1); + expect(runCompletedHandler.mock.lastCall[0].detail).toEqual( + expect.objectContaining({ + isErrorFree: true, + }), + ); + }); + test("includes error details and friendly error state when a run failed", () => { + renderRunCompletion({ + error: "NameError: name 'kettle' is not defined on line 5 of main.py", + errorDetails: { + type: "NameError", + file: "main.py", + line: 5, + description: "name 'kettle' is not defined", + }, + friendlyError: { + html: "

Friendly error

", + }, + }); + + expect(runCompletedHandler).toHaveBeenCalledTimes(1); + expect(runCompletedHandler.mock.lastCall[0].detail).toEqual( + expect.objectContaining({ + isErrorFree: false, + errorDetails: { + type: "NameError", + file: "main.py", + line: 5, + description: "name 'kettle' is not defined", + }, + friendlyErrorShown: true, + projectIdentifier: "test-project", + projectType: "python", + }), + ); + }); + + test("Triggers runCompletedEvent with error details when outputOnly is true", () => { + renderRunCompletion({}, { outputOnly: true }); + expect(runCompletedHandler).toHaveBeenCalledTimes(1); + expect(runCompletedHandler.mock.lastCall[0].detail).toEqual( + expect.objectContaining({ + errorDetails: undefined, + step: 3, + projectIdentifier: "test-project", + projectType: "python", + }), + ); + }); + + test("does not trigger runCompletedEvent again when error state updates after the run ends", () => { const middlewares = []; const mockStore = configureStore(middlewares); - const initialState = { + const editor = { + project: { + identifier: "test-project", + project_type: "python", + components: [ + { name: "main", extension: "py", content: "print('hello')" }, + ], + image_list: [], + }, + loading: "success", + openFiles: [], + focussedFileIndices: [], + codeRunTriggered: true, + codeHasBeenRun: true, + error: "", + errorDetails: undefined, + friendlyError: undefined, + }; + const instructions = { + currentStepPosition: 3, + permitOverride: true, + }; + + store = mockStore({ editor, instructions, auth: {} }); + + const view = render( + + + , + ); + + store = mockStore({ + editor: { ...editor, codeRunTriggered: false }, + instructions, + auth: {}, + }); + view.rerender( + + + , + ); + + flushRunEventDebounce(); + + expect(runCompletedHandler).toHaveBeenCalledTimes(1); + + store = mockStore({ editor: { - project: { - identifier: "test-project", - project_type: "python", - components: [ - { name: "main", extension: "py", content: "print('hello')" }, - ], - image_list: [], - }, - loading: "success", - openFiles: [], - focussedFileIndices: [], + ...editor, codeRunTriggered: false, - codeHasBeenRun: true, error: "NameError: name 'kettle' is not defined on line 5 of main.py", errorDetails: { type: "NameError", @@ -293,21 +452,87 @@ describe("When code run finishes", () => { html: "

Friendly error

", }, }, - instructions: { - currentStepPosition: 3, - permitOverride: true, - }, + instructions, auth: {}, + }); + view.rerender( + + + , + ); + + expect(runCompletedHandler).toHaveBeenCalledTimes(1); + }); + + test("emits runCompleted with latest error state when run ends during debounce", () => { + const middlewares = []; + const mockStore = configureStore(middlewares); + const editor = { + project: { + identifier: "test-project", + project_type: "python", + components: [ + { name: "main", extension: "py", content: "print('hello')" }, + ], + image_list: [], + }, + loading: "success", + openFiles: [], + focussedFileIndices: [], + codeRunTriggered: true, + codeHasBeenRun: true, + error: "", + errorDetails: undefined, + friendlyError: undefined, + }; + const instructions = { + currentStepPosition: 3, + permitOverride: true, }; - store = mockStore(initialState); - render( + store = mockStore({ editor, instructions, auth: {} }); + + const view = render( + + + , + ); + + store = mockStore({ + editor: { ...editor, codeRunTriggered: false }, + instructions, + auth: {}, + }); + view.rerender( + + + , + ); + + store = mockStore({ + editor: { + ...editor, + codeRunTriggered: false, + error: "NameError: name 'kettle' is not defined on line 5 of main.py", + errorDetails: { + type: "NameError", + file: "main.py", + line: 5, + description: "name 'kettle' is not defined", + }, + }, + instructions, + auth: {}, + }); + view.rerender( , ); - expect(runCompletedHandler).toHaveBeenCalled(); + flushRunEventDebounce(); + + expect(runCompletedHandler).toHaveBeenCalledTimes(1); expect(runCompletedHandler.mock.lastCall[0].detail).toEqual( expect.objectContaining({ isErrorFree: false, @@ -317,28 +542,315 @@ describe("When code run finishes", () => { line: 5, description: "name 'kettle' is not defined", }, - friendlyErrorShown: true, - projectIdentifier: "test-project", - projectType: "python", }), ); }); +}); - test("Triggers runCompletedEvent with error details when outputOnly is true", () => { - renderWebComponentProject({ - projectType: "python", +describe("When code is unchanged between runs", () => { + const middlewares = []; + const mockStoreFactory = configureStore(middlewares); + + const baseEditor = { + project: { + identifier: "test-project", + project_type: "python", + components: [ + { name: "main", extension: "py", content: "print('hello')" }, + ], + image_list: [], + }, + loading: "success", + openFiles: [], + focussedFileIndices: [], + error: "", + errorDetails: undefined, + friendlyError: undefined, + }; + + const renderRunCycle = (editorOverrides = {}) => { + const editor = { ...baseEditor, ...editorOverrides }; + const instructions = { + currentStepPosition: 3, + permitOverride: true, + }; + + store = mockStoreFactory({ editor, instructions, auth: {} }); + + const view = render( + + + , + ); + + return { view, editor, instructions }; + }; + + beforeEach(() => { + runStartedHandler.mockClear(); + runCompletedHandler.mockClear(); + }); + + test("does not emit run events on repeated runs with the same code", () => { + const { view, editor, instructions } = renderRunCycle({ + codeRunTriggered: true, + }); + flushRunEventDebounce(); + + expect(runStartedHandler).toHaveBeenCalledTimes(1); + + store = mockStoreFactory({ + editor: { ...editor, codeRunTriggered: false, codeHasBeenRun: true }, + instructions, + auth: {}, + }); + view.rerender( + + + , + ); + + expect(runCompletedHandler).toHaveBeenCalledTimes(1); + + store = mockStoreFactory({ + editor: { ...editor, codeRunTriggered: true, codeHasBeenRun: true }, + instructions, + auth: {}, + }); + view.rerender( + + + , + ); + flushRunEventDebounce(); + + expect(runStartedHandler).toHaveBeenCalledTimes(1); + + store = mockStoreFactory({ + editor: { ...editor, codeRunTriggered: false }, + instructions, + auth: {}, + }); + view.rerender( + + + , + ); + + expect(runCompletedHandler).toHaveBeenCalledTimes(1); + }); + + test("emits run events again after code changes", () => { + const { view, editor, instructions } = renderRunCycle({ + codeRunTriggered: true, + }); + flushRunEventDebounce(); + + store = mockStoreFactory({ + editor: { ...editor, codeRunTriggered: false, codeHasBeenRun: true }, + instructions, + auth: {}, + }); + view.rerender( + + + , + ); + + flushRunEventDebounce(); + + const updatedEditor = { + ...editor, + project: { + ...editor.project, + components: [ + { name: "main", extension: "py", content: "print('world')" }, + ], + }, + codeRunTriggered: true, codeHasBeenRun: true, - props: { outputOnly: true }, + }; + + store = mockStoreFactory({ + editor: updatedEditor, + instructions, + auth: {}, }); - expect(runCompletedHandler).toHaveBeenCalled(); - expect(runCompletedHandler.mock.lastCall[0].detail).toEqual( - expect.objectContaining({ - errorDetails: undefined, - step: 3, - projectIdentifier: "test-project", - projectType: "python", - }), + view.rerender( + + + , + ); + flushRunEventDebounce(); + + expect(runStartedHandler).toHaveBeenCalledTimes(2); + + store = mockStoreFactory({ + editor: { ...updatedEditor, codeRunTriggered: false }, + instructions, + auth: {}, + }); + view.rerender( + + + , ); + + expect(runCompletedHandler).toHaveBeenCalledTimes(2); + }); + + test("emits run events on repeated runs when read only", () => { + const { view, editor, instructions } = renderRunCycle({ + codeRunTriggered: true, + readOnly: true, + }); + flushRunEventDebounce(); + + expect(runStartedHandler).toHaveBeenCalledTimes(1); + + store = mockStoreFactory({ + editor: { ...editor, codeRunTriggered: false, codeHasBeenRun: true }, + instructions, + auth: {}, + }); + view.rerender( + + + , + ); + + expect(runCompletedHandler).toHaveBeenCalledTimes(1); + + flushRunEventDebounce(); + + store = mockStoreFactory({ + editor: { + ...editor, + codeRunTriggered: true, + codeHasBeenRun: true, + readOnly: true, + }, + instructions, + auth: {}, + }); + view.rerender( + + + , + ); + flushRunEventDebounce(); + + expect(runStartedHandler).toHaveBeenCalledTimes(2); + + store = mockStoreFactory({ + editor: { ...editor, codeRunTriggered: false, readOnly: true }, + instructions, + auth: {}, + }); + view.rerender( + + + , + ); + + expect(runCompletedHandler).toHaveBeenCalledTimes(2); + }); + + test("collapses rapid read-only reruns into one debounced dispatch", () => { + const { view, editor, instructions } = renderRunCycle({ + codeRunTriggered: true, + readOnly: true, + }); + flushRunEventDebounce(); + + expect(runStartedHandler).toHaveBeenCalledTimes(1); + + store = mockStoreFactory({ + editor: { ...editor, codeRunTriggered: false, codeHasBeenRun: true }, + instructions, + auth: {}, + }); + view.rerender( + + + , + ); + + store = mockStoreFactory({ + editor: { + ...editor, + codeRunTriggered: true, + codeHasBeenRun: true, + readOnly: true, + }, + instructions, + auth: {}, + }); + view.rerender( + + + , + ); + + expect(runStartedHandler).toHaveBeenCalledTimes(1); + + flushRunEventDebounce(); + + expect(runStartedHandler).toHaveBeenCalledTimes(2); + }); +}); + +describe("When remounted during a run", () => { + beforeEach(() => { + runStartedHandler.mockClear(); + }); + + test("does not trigger runStarted again", () => { + const middlewares = []; + const mockStore = configureStore(middlewares); + const initialState = { + editor: { + project: { + identifier: "test-project", + project_type: "python", + components: [ + { name: "main", extension: "py", content: "print('hello')" }, + ], + image_list: [], + }, + loading: "success", + openFiles: [], + focussedFileIndices: [], + codeRunTriggered: true, + codeHasBeenRun: true, + }, + instructions: { + currentStepPosition: 3, + permitOverride: true, + }, + auth: {}, + }; + const store = mockStore(initialState); + + const view = render( + + + , + ); + + flushRunEventDebounce(); + + expect(runStartedHandler).toHaveBeenCalledTimes(1); + + view.unmount(); + + render( + + + , + ); + + expect(runStartedHandler).toHaveBeenCalledTimes(1); }); }); diff --git a/src/components/WebComponentProject/buildProjectCodeSnapshot.js b/src/components/WebComponentProject/buildProjectCodeSnapshot.js new file mode 100644 index 000000000..b564b5f74 --- /dev/null +++ b/src/components/WebComponentProject/buildProjectCodeSnapshot.js @@ -0,0 +1,13 @@ +export const buildProjectCodeSnapshot = (components = []) => { + if (!Array.isArray(components)) { + return ""; + } + + return components + .map( + ({ name, extension, content }) => + `${name}.${extension}\0${content ?? ""}`, + ) + .sort() + .join("\n"); +}; diff --git a/src/components/WebComponentProject/buildProjectCodeSnapshot.test.js b/src/components/WebComponentProject/buildProjectCodeSnapshot.test.js new file mode 100644 index 000000000..ed0ac080a --- /dev/null +++ b/src/components/WebComponentProject/buildProjectCodeSnapshot.test.js @@ -0,0 +1,29 @@ +import { buildProjectCodeSnapshot } from "./buildProjectCodeSnapshot"; + +describe("buildProjectCodeSnapshot", () => { + test("builds a stable snapshot from project components", () => { + const components = [ + { name: "main", extension: "py", content: "print('hello')" }, + { name: "utils", extension: "py", content: "def foo(): pass" }, + ]; + + expect(buildProjectCodeSnapshot(components)).toBe( + buildProjectCodeSnapshot([...components].reverse()), + ); + }); + + test("returns an empty string for non-array input", () => { + expect(buildProjectCodeSnapshot(null)).toBe(""); + }); + + test("changes when component content changes", () => { + const before = buildProjectCodeSnapshot([ + { name: "main", extension: "py", content: "print(1)" }, + ]); + const after = buildProjectCodeSnapshot([ + { name: "main", extension: "py", content: "print(2)" }, + ]); + + expect(before).not.toBe(after); + }); +}); diff --git a/src/components/WebComponentProject/runEventCodeSnapshot.js b/src/components/WebComponentProject/runEventCodeSnapshot.js new file mode 100644 index 000000000..9f78cb792 --- /dev/null +++ b/src/components/WebComponentProject/runEventCodeSnapshot.js @@ -0,0 +1,129 @@ +import { buildProjectCodeSnapshot } from "./buildProjectCodeSnapshot"; + +export const RUN_EVENT_DEBOUNCE_MS = 250; + +let lastEmittedCodeSnapshot = null; +let trackedProjectIdentifier = null; +let emitRunEventsForCurrentCycle = false; +let debounceTimer = null; +let runEndedWhileDebouncing = false; +let pendingCallbacks = null; + +const syncTrackedProject = (projectIdentifier) => { + if (trackedProjectIdentifier !== projectIdentifier) { + lastEmittedCodeSnapshot = null; + trackedProjectIdentifier = projectIdentifier ?? null; + } +}; + +const clearDebounceTimer = () => { + if (debounceTimer) { + clearTimeout(debounceTimer); + debounceTimer = null; + } +}; + +const shouldAllowRunEvent = (components, bypassSnapshot) => { + if (bypassSnapshot) { + return true; + } + + const snapshot = buildProjectCodeSnapshot(components); + + if (snapshot === lastEmittedCodeSnapshot) { + return false; + } + + lastEmittedCodeSnapshot = snapshot; + return true; +}; + +const flushDebouncedRunEvent = () => { + debounceTimer = null; + const callbacks = pendingCallbacks; + pendingCallbacks = null; + + if (!callbacks) { + return; + } + + const { + projectIdentifier, + components, + bypassSnapshot, + onRunStarted, + onRunCompletedIfRunAlreadyEnded, + } = callbacks; + + syncTrackedProject(projectIdentifier); + + if (!shouldAllowRunEvent(components, bypassSnapshot)) { + emitRunEventsForCurrentCycle = false; + runEndedWhileDebouncing = false; + return; + } + + emitRunEventsForCurrentCycle = true; + onRunStarted?.(); + + if (runEndedWhileDebouncing) { + onRunCompletedIfRunAlreadyEnded?.(); + emitRunEventsForCurrentCycle = false; + runEndedWhileDebouncing = false; + } +}; + +export const scheduleRunEventCycle = ( + projectIdentifier, + components, + { bypassSnapshot = false } = {}, + { onRunStarted, onRunCompletedIfRunAlreadyEnded } = {}, +) => { + syncTrackedProject(projectIdentifier); + runEndedWhileDebouncing = false; + pendingCallbacks = { + projectIdentifier, + components, + bypassSnapshot, + onRunStarted, + onRunCompletedIfRunAlreadyEnded, + }; + + clearDebounceTimer(); + debounceTimer = setTimeout(flushDebouncedRunEvent, RUN_EVENT_DEBOUNCE_MS); +}; + +export const handleRunEndedForEventCycle = ({ onRunCompleted }) => { + if (debounceTimer) { + runEndedWhileDebouncing = true; + return; + } + + if (emitRunEventsForCurrentCycle) { + onRunCompleted?.(); + } +}; + +export const shouldEmitRunCompletedEvent = () => emitRunEventsForCurrentCycle; + +export const endRunEventCycle = () => { + if (debounceTimer) { + return; + } + + emitRunEventsForCurrentCycle = false; + runEndedWhileDebouncing = false; +}; + +export const cancelPendingRunEventDebounce = () => { + clearDebounceTimer(); + pendingCallbacks = null; + runEndedWhileDebouncing = false; +}; + +export const resetRunEventCodeSnapshot = () => { + cancelPendingRunEventDebounce(); + lastEmittedCodeSnapshot = null; + trackedProjectIdentifier = null; + emitRunEventsForCurrentCycle = false; +}; diff --git a/src/components/WebComponentProject/runEventCodeSnapshot.test.js b/src/components/WebComponentProject/runEventCodeSnapshot.test.js new file mode 100644 index 000000000..3a6fb1e11 --- /dev/null +++ b/src/components/WebComponentProject/runEventCodeSnapshot.test.js @@ -0,0 +1,165 @@ +import { + cancelPendingRunEventDebounce, + endRunEventCycle, + handleRunEndedForEventCycle, + resetRunEventCodeSnapshot, + RUN_EVENT_DEBOUNCE_MS, + scheduleRunEventCycle, + shouldEmitRunCompletedEvent, +} from "./runEventCodeSnapshot"; + +const components = [ + { name: "main", extension: "py", content: "print('hello')" }, +]; + +const flushDebounce = () => { + jest.advanceTimersByTime(RUN_EVENT_DEBOUNCE_MS); +}; + +describe("runEventCodeSnapshot", () => { + beforeEach(() => { + jest.useFakeTimers(); + resetRunEventCodeSnapshot(); + }); + + afterEach(() => { + jest.useRealTimers(); + }); + + test("allows the first run for a project after debounce", () => { + const onRunStarted = jest.fn(); + + scheduleRunEventCycle("project-a", components, {}, { onRunStarted }); + flushDebounce(); + + expect(onRunStarted).toHaveBeenCalledTimes(1); + expect(shouldEmitRunCompletedEvent()).toBe(true); + }); + + test("suppresses repeated runs with unchanged code", () => { + const onRunStarted = jest.fn(); + + scheduleRunEventCycle("project-a", components, {}, { onRunStarted }); + flushDebounce(); + endRunEventCycle(); + + scheduleRunEventCycle("project-a", components, {}, { onRunStarted }); + flushDebounce(); + + expect(onRunStarted).toHaveBeenCalledTimes(1); + expect(shouldEmitRunCompletedEvent()).toBe(false); + }); + + test("allows a run after code changes", () => { + const onRunStarted = jest.fn(); + + scheduleRunEventCycle("project-a", components, {}, { onRunStarted }); + flushDebounce(); + endRunEventCycle(); + + const updatedComponents = [ + { name: "main", extension: "py", content: "print('world')" }, + ]; + + scheduleRunEventCycle("project-a", updatedComponents, {}, { onRunStarted }); + flushDebounce(); + + expect(onRunStarted).toHaveBeenCalledTimes(2); + expect(shouldEmitRunCompletedEvent()).toBe(true); + }); + + test("collapses a rapid burst into one run event", () => { + const onRunStarted = jest.fn(); + const onRunCompletedIfRunAlreadyEnded = jest.fn(); + + scheduleRunEventCycle( + "project-a", + components, + { bypassSnapshot: true }, + { onRunStarted, onRunCompletedIfRunAlreadyEnded }, + ); + scheduleRunEventCycle( + "project-a", + components, + { bypassSnapshot: true }, + { onRunStarted, onRunCompletedIfRunAlreadyEnded }, + ); + handleRunEndedForEventCycle({ + onRunCompleted: onRunCompletedIfRunAlreadyEnded, + }); + flushDebounce(); + + expect(onRunStarted).toHaveBeenCalledTimes(1); + expect(onRunCompletedIfRunAlreadyEnded).toHaveBeenCalledTimes(1); + expect(shouldEmitRunCompletedEvent()).toBe(false); + }); + + test("emits run completed on run end after debounced start", () => { + const onRunStarted = jest.fn(); + const onRunCompleted = jest.fn(); + + scheduleRunEventCycle("project-a", components, {}, { onRunStarted }); + flushDebounce(); + + handleRunEndedForEventCycle({ onRunCompleted: onRunCompleted }); + + expect(onRunStarted).toHaveBeenCalledTimes(1); + expect(onRunCompleted).toHaveBeenCalledTimes(1); + }); + + test("resets snapshot when the project identifier changes", () => { + const onRunStarted = jest.fn(); + + scheduleRunEventCycle("project-a", components, {}, { onRunStarted }); + flushDebounce(); + endRunEventCycle(); + + scheduleRunEventCycle("project-b", components, {}, { onRunStarted }); + flushDebounce(); + + expect(onRunStarted).toHaveBeenCalledTimes(2); + expect(shouldEmitRunCompletedEvent()).toBe(true); + }); + + test("allows separate bursts after debounce quiet period", () => { + const onRunStarted = jest.fn(); + + scheduleRunEventCycle( + "project-a", + components, + { bypassSnapshot: true }, + { onRunStarted }, + ); + flushDebounce(); + endRunEventCycle(); + + jest.advanceTimersByTime(RUN_EVENT_DEBOUNCE_MS); + + scheduleRunEventCycle( + "project-a", + components, + { bypassSnapshot: true }, + { onRunStarted }, + ); + flushDebounce(); + + expect(onRunStarted).toHaveBeenCalledTimes(2); + expect(shouldEmitRunCompletedEvent()).toBe(true); + }); + + test("does not fire pending callbacks after debounce is cancelled", () => { + const onRunStarted = jest.fn(); + + scheduleRunEventCycle( + "project-a", + components, + { bypassSnapshot: true }, + { onRunStarted }, + ); + cancelPendingRunEventDebounce(); + flushDebounce(); + + expect(onRunStarted).not.toHaveBeenCalled(); + expect(shouldEmitRunCompletedEvent()).toBe(false); + }); +}); diff --git a/src/components/WebComponentProject/runEventTrackingState.js b/src/components/WebComponentProject/runEventTrackingState.js new file mode 100644 index 000000000..acab2a663 --- /dev/null +++ b/src/components/WebComponentProject/runEventTrackingState.js @@ -0,0 +1,30 @@ +import { resetRunEventCodeSnapshot } from "./runEventCodeSnapshot"; + +let prevCodeRunTriggered = false; +let trackedProjectIdentifier = null; + +export const getPrevCodeRunTriggered = () => prevCodeRunTriggered; + +export const setPrevCodeRunTriggered = (value) => { + prevCodeRunTriggered = value; +}; + +export const resetCodeRunEventTracking = () => { + prevCodeRunTriggered = false; + trackedProjectIdentifier = null; + resetRunEventCodeSnapshot(); +}; + +export const syncRunEventTrackingProject = ( + projectIdentifier, + codeRunTriggered, +) => { + if ( + trackedProjectIdentifier !== null && + trackedProjectIdentifier !== projectIdentifier + ) { + prevCodeRunTriggered = codeRunTriggered; + } + + trackedProjectIdentifier = projectIdentifier ?? null; +}; diff --git a/src/components/WebComponentProject/runEventTrackingState.test.js b/src/components/WebComponentProject/runEventTrackingState.test.js new file mode 100644 index 000000000..c18bca2a4 --- /dev/null +++ b/src/components/WebComponentProject/runEventTrackingState.test.js @@ -0,0 +1,36 @@ +import { + getPrevCodeRunTriggered, + resetCodeRunEventTracking, + setPrevCodeRunTriggered, + syncRunEventTrackingProject, +} from "./runEventTrackingState"; + +describe("runEventTrackingState", () => { + beforeEach(() => { + resetCodeRunEventTracking(); + }); + + test("tracks the previous codeRunTriggered value across calls", () => { + expect(getPrevCodeRunTriggered()).toBe(false); + setPrevCodeRunTriggered(true); + expect(getPrevCodeRunTriggered()).toBe(true); + }); + + test("resets tracking state", () => { + setPrevCodeRunTriggered(true); + syncRunEventTrackingProject("project-a", true); + + resetCodeRunEventTracking(); + + expect(getPrevCodeRunTriggered()).toBe(false); + }); + + test("resets prev state when the project identifier changes", () => { + syncRunEventTrackingProject("project-a", true); + setPrevCodeRunTriggered(true); + + syncRunEventTrackingProject("project-b", false); + + expect(getPrevCodeRunTriggered()).toBe(false); + }); +}); diff --git a/src/web-component.js b/src/web-component.js index 537a4d267..aa9e90281 100644 --- a/src/web-component.js +++ b/src/web-component.js @@ -10,6 +10,7 @@ import camelCase from "camelcase"; import { stopCodeRun, stopDraw, triggerCodeRun } from "./redux/EditorSlice"; import { BrowserRouter } from "react-router-dom"; import { resetStore } from "./redux/RootSlice"; +import { resetCodeRunEventTracking } from "./components/WebComponentProject/runEventTrackingState"; import dedupeDesignSystemWarnings from "./utils/dedupeDesignSystemWarnings"; import { setUser } from "./redux/WebComponentAuthSlice"; import { projectHasChangedSinceInitialLoad } from "./utils/projectHelpers"; @@ -45,6 +46,7 @@ class WebComponent extends HTMLElement { console.log("Unmounted web-component..."); this.root.unmount(); } + resetCodeRunEventTracking(); store.dispatch(resetStore()); }