Skip to content

onStylePalettes is a one-shot report: a callback attached after boot never receives the palettes #1626

Description

@dqnykamp

Flagging a latent hole in onStylePalettes for @doenet/doenetml-iframe. No host has reported hitting this — it surfaced while diagnosing a test flake (#1625) — so this is a "watch for it, and here's the fix if it bites" issue rather than a confirmed defect.

The hole

<DoenetViewer> and <DoenetEditor> report the booted bundle's palettes exactly once, inline in the iframeReady branch of the message listener (packages/doenetml-iframe/src/index.tsx, ~L748 for the viewer and ~L1547 for the editor):

} else if (data.iframeReady) {
    onStylePalettesRef.current?.(
        (data.stylePalettes as StylePaletteInfo[] | null) ?? null,
    );

The ?. is the whole story: if onStylePalettes happens to be absent at that instant, the report is dropped and nothing ever re-sends it. The iframe posts stylePalettes only alongside iframeReady (iframe-viewer-index.ts / iframe-editor-index.ts), so there is no second chance for the life of that iframe.

The README's example passes the callback inline (onStylePalettes={setPalettes}), which is always defined at mount, so the documented usage is safe. A host is exposed when the callback is conditional — attached after some async setup, gated on a feature flag or user action, or simply undefined on the first render:

// Silently never fires if the bundle boots before `picker` turns on.
<DoenetViewer onStylePalettes={picker ? setPalettes : undefined} ... />

That is not hypothetical: this is precisely how the spec in DoenetViewer.stylePalettes.cy.tsx attaches its callback, and it lost the race often enough to fail ~3 runs in 5. In that spec the window was only a couple of milliseconds wide — booting the ~32 MB bundle starves the parent's timers so the two events collide — but a host that attaches its callback on a genuine user action can miss it by seconds, deterministically.

Symptom to watch for

A palette picker that renders empty (or stays hidden) for a viewer that is otherwise working normally, with no error anywhere. Because null is the documented "this bundle is too old" signal and no call at all is indistinguishable from "still booting," a host cannot currently tell the difference between "not ready yet," "too old," and "we missed it."

Quick confirmation for anyone who suspects it: attach onStylePalettes unconditionally at mount. If the palettes show up, this is the bug.

How to fix it

Remember the report and hand it to a callback that arrives later. Deliver once per boot, keyed on the report rather than on the callback's identity — hosts routinely pass inline arrows whose identity churns every render, and this package has already been bitten by that (#1251).

In both DoenetViewer and DoenetEditor:

// The palettes are announced once, on iframeReady. Hold onto that
// announcement so a callback attached afterwards still receives it.
const stylePaletteReportRef = React.useRef<{
    palettes: StylePaletteInfo[] | null;
    delivered: boolean;
} | null>(null);

function deliverStylePalettes() {
    const report = stylePaletteReportRef.current;
    const callback = onStylePalettesRef.current;
    if (report && !report.delivered && callback) {
        report.delivered = true;
        callback(report.palettes);
    }
}

// ...in the `iframeReady` branch, replacing the `?.` call:
stylePaletteReportRef.current = {
    palettes: (data.stylePalettes as StylePaletteInfo[] | null) ?? null,
    delivered: false,
};
deliverStylePalettes();

// ...and, so a late callback is picked up (no dep array on purpose):
React.useEffect(deliverStylePalettes);

delivered guarantees exactly one call per boot, so identity churn cannot cause repeats, and a genuine re-boot (a windowed viewer unparking, a srcDoc rebuild) installs a fresh report and legitimately reports again.

Worth doing at the same time:

  • Regression test. test(doenetml-iframe): de-flake the style-palette callback spec #1625 makes the existing after-mount spec deterministic by installing the callback before the boot can finish — which is the right call for that spec, but it means nothing then covers a genuinely late attachment. With the fix above, add a spec that waits for the document to render (so the report has definitively already happened) and only then attaches the callback, asserting it still fires.
  • README. packages/doenetml-iframe/README.md currently says the callback "fires once the bundle has booted." If the fix lands, say explicitly that a callback attached after boot still receives the palettes; if it does not, say explicitly that the callback must be attached at mount.

Scope

Wrapper-side only. @doenet/doenetml's getStylePalettes() and the standalone bundle's window.getDoenetStylePalettes are plain synchronous calls with no timing element — only the iframe wrapper turns them into a one-shot push.

🤖 Generated with Claude Code

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    Fields

    Priority

    Hold off

    Effort

    Medium

    Projects

    Status
    No status

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions