Skip to content

fix(sdk): eliminate Malloy chart flicker on re-render#886

Merged
kylenesbit merged 1 commit into
mainfrom
monty/fix-chart-flicker
Jul 20, 2026
Merged

fix(sdk): eliminate Malloy chart flicker on re-render#886
kylenesbit merged 1 commit into
mainfrom
monty/fix-chart-flicker

Conversation

@mlennie

@mlennie mlennie commented Jul 14, 2026

Copy link
Copy Markdown
Collaborator

Problem

Charts rendered through the SDK's RenderedResult bridge flicker whenever the result re-renders: a refetch that returns changed data, a theme toggle, or a remount. The layout effect clears the live container synchronously, and only repaints after await import("@malloydata/render") resolves and a fresh MalloyRenderer is constructed, so the browser paints one or more blank frames in between.

Charts show this far more than tables because @malloydata/render paints fill-sized charts asynchronously: parentSize starts at 0, so the chart subtree only mounts after a ResizeObserver plus requestAnimationFrame pass, which widens the empty window. Tables use fixed sizing and paint synchronously.

The gap opened when the renderer import became async. Before that, the clear and viz.render() ran in the same pre-paint tick, so no empty frame was ever painted.

Fix

Double-buffer the swap.

  • The live viz is held in a ref across renders. Each render paints a new viz into an offscreen but laid-out stage node, then swaps it in only once the renderer signals onReady, disposing the previous viz at that point. The old chart stays on screen until the new one has painted, so there is no blank frame on any trigger. The stage uses visibility: hidden rather than display: none on purpose: it has to keep layout so a fill chart measures a real size and onReady actually fires.
  • A bounded fallback forces the swap if onReady never fires, so a failed render surfaces instead of leaving a stale chart on screen indefinitely.
  • A generation counter plus a cancelled flag handle overlapping and superseded renders, and a dedicated unmount effect disposes the last viz.
  • Per-chart theme CSS variables are written on the stage rather than the shared container, so the outgoing chart's chrome does not repaint to the incoming theme mid-swap.
  • The renderer chunk is pre-warmed so the first paint does not wait on the dynamic import.

Separately, the chart-result queries no longer re-execute on tab refocus or reconnect. A Malloy result is a pure function of the query text and filters, which are already in the query key, so a focus refetch only ever repaints the same result. These overrides are scoped to the result queries (QueryResult, ModelCell) rather than set on the global client, so every other SDK query keeps react-query's default freshness behavior.

Verification

Measured in a running Publisher instance on a notebook containing a line chart, a dashboard, and a segment map, toggling the color mode (which re-runs the render bridge for every chart on the page) while sampling the rendered chart count on every frame:

rendered charts during the swap
before drops to 0, every chart blanks
after holds at baseline, never dips

The line chart, dashboard, segment map, and table all repaint correctly in light and dark, and the final chart count is unchanged, so no duplicate or leaked viz.

Gate is green: typecheck, lint, prettier:check, and the server test suite (247 pass).

Charts rendered through the SDK's RenderedResult bridge flickered
whenever the result re-rendered (a refetch that changed the data, a
theme toggle, or a remount). The layout effect cleared the live
container synchronously and only repainted after an async dynamic
import resolved, so the browser painted one or more blank frames.
Fill-sized charts made it worse because @malloydata/render paints them
asynchronously after a resize pass.

Double-buffer the swap instead. The live viz is held across renders;
each new render paints into an offscreen-but-laid-out stage node and is
swapped in only once the renderer signals onReady, disposing the old
viz at that point. The old chart stays on screen until the new one has
painted, so there is no blank frame on any trigger. A bounded fallback
forces the swap if onReady never fires, so a failed render surfaces
instead of leaving a stale chart on screen. Per-chart theme CSS
variables are written on the stage rather than the shared container so
the outgoing chart's chrome does not repaint mid-swap. The renderer
chunk is pre-warmed so the first paint does not wait on the import.

Also stop re-executing the chart-result queries on tab refocus and
reconnect: a Malloy result is a pure function of the query text and
filters (already in the query key), so a focus refetch only repaints
the same result. The staleTime + refetchOnWindowFocus/onReconnect
overrides are scoped to the result queries (QueryResult, ModelCell)
rather than the global client, leaving other SDK queries on default
freshness behavior.

Signed-off-by: Monty Lennie <[email protected]>

@kylenesbit kylenesbit left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the diff in full, traced the swap logic in context, and verified the renderer API this hinges on. Approving — one follow-up worth filing, noted at the end.

What I verified

  • The onReady contract is real. The SDK declares the viz interface by hand, so if onReady didn't exist the call would throw inside the try block and tear the render down. Checked the pinned @malloydata/render 0.0.422: onReady(callback) is public API, callbacks queue until the renderer signals ready, and it fires immediately if the render already completed — so synchronously-painting tables promote instantly rather than waiting on the 10s fallback. The ^0.0.422 floor guarantees the method exists for consumers.
  • The race handling holds up. Walked the overlap cases: a render superseded mid-import bails on the generation check and disposes its viz; a superseded render that already built a stage is torn down by the effect cleanup (which correctly skips the node if it was promoted into liveRef); unmount disposal lives in a separate passive effect precisely so effect re-runs don't kill the on-screen chart; a stale onReady firing late is a no-op via isCurrent() inside promote. React's cleanup-before-next-effect ordering makes the position: relative juggling on the shared container safe. No path found that leaks a viz, double-disposes, or leaves two charts mounted.
  • The buffer actually engages. ResultContainer renders RenderedResult without a key, so the component persists across result changes and the old chart genuinely stays up during a refetch.
  • Gate: all CI checks green including Playwright and the three-OS matrix; eslint --fix produced zero changes on a fresh checkout.

Follow-up to file (not blocking): refocus no longer picks up model changes.

The "results are pure" justification holds only while the model and warehouse data are fixed. Version-pinned views are safe (versionId rides in resourceUri, which is in the query key), but the unversioned dev loop changes behavior: edit a model, malloy_reloadPackage, alt-tab back to the browser — the tab previously refreshed itself via the focus refetch, and now shows stale results for up to 5 minutes unless the user hard-reloads. Nothing else in the React app compensates: no result-query invalidation fires on package reload, and the live-reload SSE endpoint is only consumed by the HTML data-app runtime (publisher.js) — the SDK constructs a WatchModeApi client but nothing uses it. Suggested fix: invalidate result queries on package reload (or fold a package version into the keys) rather than ever reverting the focus-refetch change.

Minor, no action needed: the 10s readyFallback deliberately promotes a possibly-blank stage so a failed render surfaces instead of stale data lingering — right call; a very slow first paint of a huge dashboard revealing partially is an acceptable edge.

@kylenesbit
kylenesbit merged commit bf57595 into main Jul 20, 2026
14 checks passed
@kylenesbit
kylenesbit deleted the monty/fix-chart-flicker branch July 20, 2026 02:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants