From cc7332f5479235514f1d925b79788e35c6243628 Mon Sep 17 00:00:00 2001 From: Duane Nykamp Date: Sat, 11 Jul 2026 23:53:33 -0500 Subject: [PATCH 1/3] Report mixed DoenetML versions to the containing page MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implements #39. Each document carries its own DoenetML `version`, and every embedded viewer downloads and parses the multi-MB standalone bundle for its document's version — an assignment mixing versions multiplies that cost by the number of distinct versions. The viewer does not normalize versions itself (that would change how older documents behave), and a console warning alone is invisible to the site's visitors, so the condition is reported to the containing page through a new `reportWarningsCallback` prop (typed `ActivityViewerWarning[]`, fired once per source analysis); the page decides how to display it. A console.warn is still emitted for developers. Co-Authored-By: Claude Fable 5 --- README.md | 33 +++++++ src/Activity/activityState.ts | 21 +++++ src/activity-viewer.tsx | 78 ++++++++++++++- src/index.ts | 1 + src/test/collectDoenetmlVersions.test.ts | 52 ++++++++++ .../ActivityViewer.mixedVersions.cy.tsx | 94 +++++++++++++++++++ 6 files changed, 277 insertions(+), 2 deletions(-) create mode 100644 src/test/collectDoenetmlVersions.test.ts create mode 100644 test/cypress/component/ActivityViewer.mixedVersions.cy.tsx diff --git a/README.md b/README.md index fd52366..9da197b 100644 --- a/README.md +++ b/README.md @@ -1,3 +1,36 @@ # Doenet assignment viewer View assignments from questions written in DoenetML + +## Mixed DoenetML versions + +Each document in an assignment carries its own DoenetML `version`, and every +embedded viewer downloads and parses the multi-MB standalone bundle for its +document's version — so an assignment mixing versions multiplies that cost +by the number of distinct versions on the page. + +The viewer does not normalize versions itself (that would change how the +older documents behave). Instead, when an assignment mixes versions, it +reports the condition to the containing page — a console warning alone would +be invisible to the site's visitors — via the `reportWarningsCallback` prop, +and the page decides how to display it: + +```tsx + { + for (const warning of warnings) { + if (warning.type === "mixedDoenetmlVersions") { + showBanner( + `This assignment mixes DoenetML versions (${warning.versions.join(", ")}), ` + + "which slows loading. Consider updating its documents to one version.", + ); + } + } + }} +/> +``` + +To consolidate, normalize the documents' `version` fields in the assignment +source. Saved student state survives such a change: it is keyed on a source +hash that deliberately ignores `version`. diff --git a/src/Activity/activityState.ts b/src/Activity/activityState.ts index 3b62717..11c2032 100644 --- a/src/Activity/activityState.ts +++ b/src/Activity/activityState.ts @@ -144,6 +144,27 @@ export function isExportedActivityState( ); } +/** + * Collect the distinct DoenetML `version` values of all documents in + * `source`, in first-appearance order. An assignment mixing versions makes + * the embedded viewers download and parse a separate multi-MB standalone + * bundle per distinct version. + */ +export function collectDoenetmlVersions(source: ActivitySource): string[] { + if (source.type === "singleDoc") { + return [source.version]; + } + const versions: string[] = []; + for (const item of source.items) { + for (const version of collectDoenetmlVersions(item)) { + if (!versions.includes(version)) { + versions.push(version); + } + } + } + return versions; +} + /** * Initialize activity state from `source` so that it is ready to generate attempts. * diff --git a/src/activity-viewer.tsx b/src/activity-viewer.tsx index 07065a1..5429a74 100644 --- a/src/activity-viewer.tsx +++ b/src/activity-viewer.tsx @@ -1,12 +1,43 @@ import "./assignment-viewer.css"; -import { Component, ErrorInfo, ReactNode, useMemo, useState } from "react"; +import { + Component, + ErrorInfo, + ReactNode, + useEffect, + useMemo, + useRef, + useState, +} from "react"; import seedrandom from "seedrandom"; import { Viewer } from "./Viewer/Viewer"; import { DoenetMLFlags } from "./types"; -import { ActivitySource } from "./Activity/activityState"; +import { + ActivitySource, + collectDoenetmlVersions, +} from "./Activity/activityState"; import { useResolvedTheme } from "./utils/theme"; import type { ThemeSetting } from "./utils/theme"; +/** + * A condition in the provided activity worth surfacing to the user — passed + * to `reportWarningsCallback`, since a console warning is invisible to the + * site's visitors. How (and whether) to display it is the containing page's + * decision. + */ +export type ActivityViewerWarning = { + type: "mixedDoenetmlVersions"; + /** + * The distinct DoenetML versions the assignment's documents request, in + * first-appearance order. Every embedded viewer downloads and parses + * the multi-MB standalone bundle for its document's version, so mixing + * versions multiplies that cost by the number of distinct versions. + * (Normalizing the `version` fields in the source avoids it; saved + * student state is keyed on a hash that ignores `version`, so it + * survives such a change.) + */ + versions: string[]; +}; + type DoenetMLFlagsSubset = Partial; const defaultFlags: DoenetMLFlags = { @@ -56,6 +87,7 @@ export function ActivityViewer({ includeVariantSelector: _includeVariantSelector = false, showTitle = true, itemWord = "item", + reportWarningsCallback, }: { source: ActivitySource; flags?: DoenetMLFlagsSubset; @@ -81,6 +113,13 @@ export function ActivityViewer({ includeVariantSelector?: boolean; showTitle?: boolean; itemWord?: string; + /** + * Called (once per `source` analysis) with conditions worth surfacing + * to the user, e.g. an assignment mixing DoenetML versions. The + * containing page decides how to display them; a `console.warn` is + * also emitted for developers. + */ + reportWarningsCallback?: (warnings: ActivityViewerWarning[]) => void; }) { const [initialVariantIndex, setInitialVariantIndex] = useState< number | null @@ -88,6 +127,41 @@ export function ActivityViewer({ const resolvedTheme = useResolvedTheme(darkMode); + const warnings = useMemo(() => { + let versions: string[]; + try { + versions = collectDoenetmlVersions(source); + } catch { + // An unwalkable source produces its own error screen. + return []; + } + if (versions.length > 1) { + return [{ type: "mixedDoenetmlVersions", versions }]; + } + return []; + }, [source]); + + // Report warnings once per source analysis (not once per render, and + // not re-reported when only the callback identity changes — hence the + // ref indirection). + const reportWarningsCallbackRef = useRef(reportWarningsCallback); + useEffect(() => { + reportWarningsCallbackRef.current = reportWarningsCallback; + }); + useEffect(() => { + if (warnings.length > 0) { + for (const warning of warnings) { + // `mixedDoenetmlVersions` is currently the only type. + console.warn( + `ActivityViewer: this assignment mixes DoenetML versions (${warning.versions.join( + ", ", + )}). Each distinct version loads its own multi-MB standalone bundle; normalize the documents' \`version\` fields to avoid the multiplied download/parse cost (saved state survives, as its hash ignores \`version\`).`, + ); + } + reportWarningsCallbackRef.current?.(warnings); + } + }, [warnings]); + // Serializing the source is how prop "sameness" is detected for // consumers that pass a fresh `source` object each render; memoize it so // the (potentially large) assignment is only serialized when the diff --git a/src/index.ts b/src/index.ts index bcd1ba9..b75d730 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,4 +1,5 @@ export { ActivityViewer } from "./activity-viewer"; +export type { ActivityViewerWarning } from "./activity-viewer"; export type { ActivitySource } from "./Activity/activityState"; export { isActivitySource } from "./Activity/activityState"; diff --git a/src/test/collectDoenetmlVersions.test.ts b/src/test/collectDoenetmlVersions.test.ts new file mode 100644 index 0000000..7a6db67 --- /dev/null +++ b/src/test/collectDoenetmlVersions.test.ts @@ -0,0 +1,52 @@ +import { describe, expect, it } from "vitest"; +import { SequenceSource } from "../Activity/sequenceState"; +import { SelectSource } from "../Activity/selectState"; +import { SingleDocSource } from "../Activity/singleDocState"; +import { collectDoenetmlVersions } from "../Activity/activityState"; +import doc from "./testSources/doc.json"; +import seq2sel from "./testSources/seq2sel.json"; + +function mkDoc(id: string, version: string): SingleDocSource { + return { + id, + type: "singleDoc", + isDescription: false, + doenetML: "

hi

", + version, + }; +} + +describe("collectDoenetmlVersions", () => { + it("single document", () => { + expect(collectDoenetmlVersions(doc as SingleDocSource)).eqls(["0.7.4"]); + }); + + it("uniform versions collapse to one entry", () => { + expect( + collectDoenetmlVersions(seq2sel as SequenceSource), + ).to.have.length(1); + }); + + it("mixed versions are reported in first-appearance order", () => { + const source: SequenceSource = { + id: "seq", + type: "sequence", + title: "mixed", + shuffle: false, + items: [ + mkDoc("a", "0.7.4"), + { + id: "sel", + type: "select", + title: "sel", + numToSelect: 1, + selectByVariant: false, + items: [mkDoc("b", "0.6.5"), mkDoc("c", "0.7.4")], + } as SelectSource, + mkDoc("d", "0.7"), + ], + } as SequenceSource; + + expect(collectDoenetmlVersions(source)).eqls(["0.7.4", "0.6.5", "0.7"]); + }); +}); diff --git a/test/cypress/component/ActivityViewer.mixedVersions.cy.tsx b/test/cypress/component/ActivityViewer.mixedVersions.cy.tsx new file mode 100644 index 0000000..e8d5a47 --- /dev/null +++ b/test/cypress/component/ActivityViewer.mixedVersions.cy.tsx @@ -0,0 +1,94 @@ +import React from "react"; +import { ActivityViewer } from "../../../src/activity-viewer"; +import type { ActivityViewerWarning } from "../../../src/activity-viewer"; +import type { ActivitySource } from "../../../src/Activity/activityState"; + +// Issue #39: an assignment whose documents request different DoenetML +// versions makes every viewer load a separate multi-MB standalone bundle +// per distinct version. A console warning is invisible to the site's +// visitors, so the condition is reported to the containing page through +// `reportWarningsCallback` — the page decides how to display it. + +const MIXED_SOURCE: ActivitySource = { + type: "sequence", + id: "seq", + title: "mixed versions", + shuffle: false, + items: [ + { + type: "singleDoc", + id: "doc-a", + doenetML: "

doc a

", + version: "0.7.4", + isDescription: false, + numVariants: 1, + }, + { + type: "singleDoc", + id: "doc-b", + doenetML: "

doc b

", + version: "0.7.3", + isDescription: false, + numVariants: 1, + }, + ], +} as ActivitySource; + +const UNIFORM_SOURCE: ActivitySource = { + type: "singleDoc", + id: "doc-u", + doenetML: "

uniform

", + version: "0.7.4", + isDescription: false, + numVariants: 1, +}; + +describe("ActivityViewer — mixed DoenetML versions are reported to the host", () => { + it("calls reportWarningsCallback exactly once with the distinct versions", () => { + const received: ActivityViewerWarning[][] = []; + cy.mount( + { + received.push(warnings); + }} + />, + ); + + cy.wrap(null).should(() => { + expect(received, "reported once").to.have.length(1); + expect(received[0]).to.eql([ + { + type: "mixedDoenetmlVersions", + versions: ["0.7.4", "0.7.3"], + }, + ]); + }); + // Still exactly one report after re-renders settle. + cy.wrap(null).should(() => { + expect(received).to.have.length(1); + }); + }); + + it("stays silent for a uniform assignment", () => { + const received: ActivityViewerWarning[][] = []; + cy.mount( + { + received.push(warnings); + }} + />, + ); + + // The viewer mounts (its iframe appears) without any warning report. + cy.get("iframe").should("exist"); + cy.wrap(null).should(() => { + expect(received).to.have.length(0); + }); + }); +}); From ee0463c47f9b3aa086d70b265c1e27057fe25841 Mon Sep 17 00:00:00 2001 From: Duane Nykamp Date: Sun, 12 Jul 2026 11:09:06 -0500 Subject: [PATCH 2/3] Dedupe mixed-version report per source content, add edge-case tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The report effect keyed on the `warnings` memo's array identity, which is rebuilt whenever a consumer passes a fresh (new-but-equal) `source` object each render — the pattern `propSetKey` already tolerates. That re-fired the `console.warn` and `reportWarningsCallback` on every render, breaking the "once per source analysis" contract. Dedupe on the serialized warning content via a ref so each distinct condition is reported at most once. Add a vitest case for an empty container (no versions) and a cypress case asserting the callback still fires exactly once when the host re-renders with a fresh source object each render. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/activity-viewer.tsx | 36 +++++++++----- src/test/collectDoenetmlVersions.test.ts | 12 +++++ .../ActivityViewer.mixedVersions.cy.tsx | 48 +++++++++++++++++++ 3 files changed, 83 insertions(+), 13 deletions(-) diff --git a/src/activity-viewer.tsx b/src/activity-viewer.tsx index 5429a74..c478023 100644 --- a/src/activity-viewer.tsx +++ b/src/activity-viewer.tsx @@ -141,25 +141,35 @@ export function ActivityViewer({ return []; }, [source]); - // Report warnings once per source analysis (not once per render, and - // not re-reported when only the callback identity changes — hence the - // ref indirection). + // Report each distinct set of warnings at most once — not once per + // render. `warnings` is a fresh array whenever the consumer passes a + // new-but-equal `source` object each render (the same pattern + // `propSetKey` below is built to tolerate), so dedupe on the serialized + // content rather than on the array identity. The callback is read through + // a ref so a change in its identity alone never triggers a re-report. const reportWarningsCallbackRef = useRef(reportWarningsCallback); useEffect(() => { reportWarningsCallbackRef.current = reportWarningsCallback; }); + const lastReportedWarningsKey = useRef(null); useEffect(() => { - if (warnings.length > 0) { - for (const warning of warnings) { - // `mixedDoenetmlVersions` is currently the only type. - console.warn( - `ActivityViewer: this assignment mixes DoenetML versions (${warning.versions.join( - ", ", - )}). Each distinct version loads its own multi-MB standalone bundle; normalize the documents' \`version\` fields to avoid the multiplied download/parse cost (saved state survives, as its hash ignores \`version\`).`, - ); - } - reportWarningsCallbackRef.current?.(warnings); + const warningsKey = JSON.stringify(warnings); + if (warningsKey === lastReportedWarningsKey.current) { + return; } + lastReportedWarningsKey.current = warningsKey; + if (warnings.length === 0) { + return; + } + for (const warning of warnings) { + // `mixedDoenetmlVersions` is currently the only type. + console.warn( + `ActivityViewer: this assignment mixes DoenetML versions (${warning.versions.join( + ", ", + )}). Each distinct version loads its own multi-MB standalone bundle; normalize the documents' \`version\` fields to avoid the multiplied download/parse cost (saved state survives, as its hash ignores \`version\`).`, + ); + } + reportWarningsCallbackRef.current?.(warnings); }, [warnings]); // Serializing the source is how prop "sameness" is detected for diff --git a/src/test/collectDoenetmlVersions.test.ts b/src/test/collectDoenetmlVersions.test.ts index 7a6db67..4384604 100644 --- a/src/test/collectDoenetmlVersions.test.ts +++ b/src/test/collectDoenetmlVersions.test.ts @@ -49,4 +49,16 @@ describe("collectDoenetmlVersions", () => { expect(collectDoenetmlVersions(source)).eqls(["0.7.4", "0.6.5", "0.7"]); }); + + it("a container with no documents yields no versions", () => { + const source: SequenceSource = { + id: "seq", + type: "sequence", + title: "empty", + shuffle: false, + items: [], + } as SequenceSource; + + expect(collectDoenetmlVersions(source)).eqls([]); + }); }); diff --git a/test/cypress/component/ActivityViewer.mixedVersions.cy.tsx b/test/cypress/component/ActivityViewer.mixedVersions.cy.tsx index e8d5a47..7d4b864 100644 --- a/test/cypress/component/ActivityViewer.mixedVersions.cy.tsx +++ b/test/cypress/component/ActivityViewer.mixedVersions.cy.tsx @@ -72,6 +72,54 @@ describe("ActivityViewer — mixed DoenetML versions are reported to the host", }); }); + it("reports only once when the host re-renders with a fresh source object each render", () => { + const received: ActivityViewerWarning[][] = []; + + // A host that hands the viewer a brand-new (deep-cloned) but equal + // `source` object on every render — the pattern the viewer's + // `propSetKey` sameness check is built to tolerate. The warning must + // still be reported exactly once, not once per render. + function Host() { + const [renderCount, setRenderCount] = React.useState(0); + const freshSource = JSON.parse( + JSON.stringify(MIXED_SOURCE), + ) as ActivitySource; + return ( +
+ + { + received.push(warnings); + }} + /> +
+ ); + } + + cy.mount(); + + cy.wrap(null).should(() => { + expect(received, "reported once").to.have.length(1); + }); + cy.get('[data-cy="rerender"]').click().click().click(); + cy.wrap(null).should(() => { + expect( + received, + "still reported once after re-renders", + ).to.have.length(1); + }); + }); + it("stays silent for a uniform assignment", () => { const received: ActivityViewerWarning[][] = []; cy.mount( From eab70fccaf63da4a3baec1332de407699745ed16 Mon Sep 17 00:00:00 2001 From: Duane Nykamp Date: Sun, 12 Jul 2026 11:14:42 -0500 Subject: [PATCH 3/3] Clarify reportWarningsCallback dedup semantics in prop doc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Align the public prop JSDoc with the actual behavior: dedup is on the serialized warning content, so the callback fires once per distinct set of warnings — not "once per source analysis" (equal-warning sources do not re-fire). Co-Authored-By: Claude Opus 4.8 (1M context) --- src/activity-viewer.tsx | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/activity-viewer.tsx b/src/activity-viewer.tsx index c478023..b9b4bae 100644 --- a/src/activity-viewer.tsx +++ b/src/activity-viewer.tsx @@ -114,10 +114,12 @@ export function ActivityViewer({ showTitle?: boolean; itemWord?: string; /** - * Called (once per `source` analysis) with conditions worth surfacing - * to the user, e.g. an assignment mixing DoenetML versions. The - * containing page decides how to display them; a `console.warn` is - * also emitted for developers. + * Called with conditions in `source` worth surfacing to the user, e.g. + * an assignment mixing DoenetML versions. Invoked once per distinct set + * of warnings — not on every render, and not again when the consumer + * passes a fresh-but-equal `source` each render. The containing page + * decides how to display them; a `console.warn` is also emitted for + * developers. */ reportWarningsCallback?: (warnings: ActivityViewerWarning[]) => void; }) {