fix(ui): 0.33.0 adoption feedback from hosts (ui 0.34.0) - #1402
Merged
Conversation
Four follow-ups from the Workspaces adoption of @plannotator/ui 0.33.0.
1. Mermaid kept a shared KaTeX chunk alive in host builds: the runtime's own
import("katex") for $$ labels has no flag and no hook, so a host with a
mathRendererLoader and the math-default-loader alias still emitted a
katex-*.js chunk with two dynamic importers, and a math document fetched
two files. New alias target utils/mermaid-math-slot renders Mermaid's
labels through the math slot with Mermaid's own options; MermaidBlock
awaits loadMathRenderer() before a diagram whose source carries a $$
label. Measured on a scratch Vite 6 consumer: one shared KaTeX chunk
before (367 chunks), one host-owned KaTeX chunk after (366 chunks).
Plannotator does not redirect and its bundles keep Mermaid's direct KaTeX.
2. HtmlViewer bridgeErrorDisplay ('banner' default, 'none') so a host that
renders its own notice from onBridgeUnavailable gets no package strip.
3. HANDOFF bridge-script alias anchored to /^\.\/bridge-script$/ so it
cannot catch another package's bridge-script entry.
4. resetMathRenderer() keeps the registered loader and discards a stale
in-flight result; setMathRendererLoader(null) and getMathRendererLoader
added. Other reset helpers reviewed and left as is.
AI-assisted (Claude) under maintainer direction.
Version bump and lockfile refresh for the ui 0.34.0 release carrying the 0.33.0 adoption feedback fixes. Core stays 0.25.0. AI-assisted (Claude) under maintainer direction.
Layout-agnostic importer test for the Mermaid KaTeX redirect (hoisted, Bun isolated and pnpm layouts all end in node_modules/mermaid/), the slot module resolved from the host's config file rather than the Mermaid importer, the note that only a direct setMathRendererLoader(null) unregisters a loader, and the duplicate no-katex source pin dropped from the slot test (the entry-assets one stays). AI-assisted (Claude) under maintainer direction.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Four follow-ups from the Workspaces adoption of
@plannotator/ui0.33.0. All additive, no new dependencies, no server changes; Plannotator's own behavior is unchanged (its entries fill the slots eagerly before first render and the single-file builds inline everything as before). This becomes ui 0.34.0 on the already published core 0.25.0.1. Mermaid kept a KaTeX chunk alive for hosts
Mermaid 11.15.0 typesets
$$...$$labels through its ownimport("katex")inrenderKatexUnsanitized. There is nothing to configure it away:legacyMathML/forceLegacyMathMLonly pick the output mode, the guard around the import is the@mermaid-js/tinybuild marker, and no hook accepts a renderer. The import only runs for a label matching Mermaid's$$regex, but chunk emission is static, so a host that registeredmathRendererLoaderand aliasedutils/math-default-loaderstill built akatex-*.jschunk, and because that chunk had two dynamic importers (the host's loader module and the Mermaid runtime) Rollup kept it separate from the host's loader chunk: a math document fetched the 57-byte loader chunk plus the shared 261 KB KaTeX chunk.The fix is option (b) realized as option (c): a bundler-facing redirect of the
katexspecifier, for importers inside themermaidpackage only, to a new package moduleutils/mermaid-math-slot, whose default export renders through the math slot with Mermaid's own options passed through (throwOnError: true,displayMode: true, the MathMLoutputmode), so a KaTeX renderer produces exactly the markup Mermaid produced from its direct import.MermaidBlockawaitsloadMathRenderer()before rendering a diagram whose source carries a$$label (a resolved promise on Plannotator's eagerly filled slot, the host's loader otherwise); an empty slot at render time throws a message naming the cause into the block's existing error panel. The recipe (aresolveIdplugin, not a plain alias, since a plainkatexalias would also rewrite the host's own loader) is in HANDOFF.md "Lazy renderers and eager entries", item 2.Measurement on a scratch Vite 6 consumer of this checkout (loader registered,
math-default-loaderaliased, a document with inline math, a display block and a flowchart with a$$label):katex-*.js, 261 KB, shared)mermaid.core-*.jsdynamically, host loader chunk staticallyhost-katex-*.js, 261 KB, host-owned)import()only;mermaid.core-*.jshas no KaTeX importSo: one KaTeX chunk, owned by the host, one file fetched for a math document. Plannotator does not redirect; its Mermaid keeps its direct KaTeX, inlined by the single-file builds, and
tests/entry-assets.test.tsnow also pins thatmermaid-math-slot.tsnever nameskatex(which would re-create the chunk). No test in the repo renders a real Mermaid diagram with a math label (Mermaid does not render under happy-dom) and nothing in Plannotator's own documents exercises$$labels; the bridge is pinned byutils/mermaid-math-slot.test.ts(delegation with Mermaid's exact options, KaTeX parity, the empty-slot error, the label regex) and the pre-render warm by two new cases incomponents/DiagramBlock.lazyRetry.test.tsx.2. Double banner on bridge failure
HtmlViewergainsbridgeErrorDisplay?: 'banner' | 'none'.'banner'is the default and renders the[data-bridge-error]strip exactly as 0.33.0 did, so Plannotator and existing hosts are unchanged.'none'renders no strip and no dismiss button for either state whileonBridgeUnavailablefires exactly as before and the version-mismatch console warning still logs once. Both values are tested inHtmlViewer.bridgeAsset.test.tsx. Documented in README and HANDOFF: the package owns the strip by default; a host that renders its own passes'none'.3. HANDOFF alias regex
The documented bridge-script alias is anchored to
/^\.\/bridge-script$/(the relative sibling form, which is the only form the package uses internally:srcdoc.ts,useHtmlAnnotation.ts,index.ts), with a resolvable replacement, and the text explains why the unanchored/\/bridge-script$/was wrong: it matched any specifier ending in/bridge-script, including another package's entry or a deeper import in the host's own tree. Verified on the scratch consumer: with the anchored alias the viewer chunk drops from 553 kB to 366 kB and the bridge literal count goes from 2 (chunk plus asset) to 1 (asset only).4.
resetMathRenderer()over-reachReset now empties the renderer and its source, forgets a load in flight (a stale in-flight result no longer fills the slot; the next
loadMathRenderer()invokes the registered loader afresh) and keeps the registered loader.setMathRendererLoader(null)is the explicit way back to the package default andgetMathRendererLoader()reads the registration.setMathRendererLoaderitself is unchanged (a load already in flight at registration still fills the slot, since the component that started it is waiting on the result). New tests: register a loader, set a renderer, reset, assert the loader is still registered and the next load uses it and never the default; the in-flight discard; the explicitnull.utils/math.test.tswas the only test relying on reset clearing the loader and now drops it explicitly. The other reset-style helpers were reviewed:resetIdentityProviderandresetIdentityGeneratorreset exactly what they name, and__setMermaidRuntimeLoaderForTestsis a stand-in by name, so nothing else changes.Verification
bun run typecheck(including the strict-consumer gate): clean.bun test packages/ui packages/editor: 869 pass, 579 skip, 0 fail.DOM_TESTS=1on MathBlock.firstPaint, InlineMarkdown, MermaidBlock, DiagramBlock.lazyRetry, srcdoc, htmlPinpointProtocol, htmlLiveProtocol, HtmlViewer.bridgeAsset, HtmlViewer.vimHud, App.htmlChrome, mermaid-math-slot, math: 264 pass, 0 fail.build:review+build:hook+tests/entry-assets.test.ts: 26 pass, markers unchanged. Sizes against the main build in the primary checkout: hookindex.html21,822,225 bytes vs 21,818,752 (+3.5 KB),review.html17,570,771 vs 17,567,628 (+3.1 KB), which is the new module and prop code.build:viewer+check:manifest: manifest in sync, viewer hash unchanged.AI-assisted (Claude) under maintainer direction.