fix(ui): render language-less code blocks as plain text - #1212
Conversation
Bare fenced code blocks (no language tag) were passed to hljs.highlightElement() unconditionally, so highlight.js auto-detected a language and colored ordinary words. Skip highlighting when the block has no language so it renders as plain monospaced text. Fixes backnotprop#1210
|
Heads up @backnotprop! This is not a slop PR. 🤚 This is intended to be a small meaningful change to a real bug. Hope it's helpful. PS I love Plannotator. So useful. Thanks for creating it. |
|
Hey, great. I'll get this in and deploy a patch for you. There was reason to do this early on and specifically use highlight JS. Without auto detection I'll probably still end up removing it, but for better reasons than I first assumed: we already ship a second highlighter (Shiki, via the diff renderer), so hljs is redundant, and hljs hasn't published to npm in a long while even though fixes keep landing in git. Size-wise it's actually not the heavy part of our bundle. |
|
and thanks |
|
Merged. Clean diagnosis and a surgical fix; the screenshot made the case instantly. This ships in the next release. Welcome aboard as a contributor! |
… highlight.js (#1218) * perf(build): stub out the dead Oniguruma WASM in every bundle @pierre/diffs picks its Shiki engine with a runtime ternary: engine: preferredHighlighter === "shiki-wasm" ? createOnigurumaEngine(import("shiki/wasm")) : createJavaScriptRegexEngine() Plannotator pins `preferredHighlighter: 'shiki-js'` (and Pierre's own default is 'shiki-js'), so the Oniguruma branch never executes. Because the choice is a runtime ternary, bundlers keep the `import("shiki/wasm")` edge anyway and inline `@shikijs/engine-oniguruma/wasm-inlined`, a ~622 KB base64 blob, into the single-file HTML builds. The review app paid for it twice: once on the main thread (via `highlighter/shared_highlighter.js`) and once inside the `?worker&inline` Pierre worker. Alias `shiki/wasm` to a stub that throws if it is ever reached. Wired via `resolve.alias` rather than a plugin because `resolve.alias` is shared with Vite's worker build and `plugins` are not. Highlighting output is unchanged: the JS regex engine was already the one doing the work. Opting back into 'shiki-wasm' now fails loudly instead of silently costing every user a megabyte of dead bytes. apps/review/dist/index.html 19,424,646 -> 18,180,545 (-1,244,101 raw / -463,348 gzip) apps/hook/dist/index.html 23,032,467 -> 22,410,416 (-622,051 raw / -233,485 gzip) * perf(ui): consolidate code highlighting onto Shiki, drop highlight.js The app shipped two highlighters. Shiki already tokenised the code-review diff pane (via @pierre/diffs, JavaScript regex engine); highlight.js separately coloured markdown fences and review suggestion snippets at ~982 KB minified for a full build of ~190 grammars. That second highlighter is now gone. Every call site moves onto `packages/ui/utils/codeHighlight.ts`, a thin wrapper over Pierre's SHARED Shiki instance: CodeBlock, Viewer, PlanCleanDiffView markdown fences InlineMarkdown code-file hover preview HighlightedCode review suggestion snippets Reusing Pierre's instance rather than standing up a second fine-grained one is deliberate. Pierre imports Shiki's full bundle, so every grammar and theme is ALREADY inlined in the single-file builds: a separate highlighter with a curated language list would have duplicated a subset of bytes that are already there. Sharing costs nothing, gives every language Shiki bundles instead of a shortlist, and — the point of the change — guarantees fences resolve the exact same theme the diff pane resolves. Theming. `SHIKI_THEME_MAP` / `resolveSyntaxTheme` move from `packages/review-editor/hooks/usePierreTheme.ts` to `packages/ui/utils/syntaxTheme.ts`; usePierreTheme re-exports them, so the review editor's imports are unchanged. `useFenceTheme()` feeds the components and re-highlights on palette or mode change. Code blocks now follow the active palette across all ~52 themes in both light and dark, instead of always rendering github-dark and relying on hand-written `.hljs-*` override stacks to stay legible. Those stacks are deleted: `packages/editor/index.css`'s light-mode token palette, and `colorblind.css`'s hand-tuned tokens which existed to APPROXIMATE @pierre/theme's protanopia-deuteranopia themes that are now simply used. Behaviour held fixed: - Language-less fences stay plain text (#1212). No auto-detection anywhere, including the hover preview, which previously called `hljs.highlightAuto`. `HighlightedCode` derives its language from the caller's file path; an unknown extension renders plain. - `applyHighlight(el, ...)` keeps the imperative `hljs.highlightElement` DOM contract the annotation layer reaches into, and writes plain text at final size first so async highlighting causes no layout shift. Already-attached grammars highlight synchronously — no flicker on cached highlights. - It also verifies the rendered text is byte-identical to the source and falls back to plain otherwise, because annotations address code blocks by text offset. - `@plannotator/ui`'s public API is unchanged: the highlighter is a module-level default like the package's other seams, no new props. The `hljs` class on fenced `<code>` becomes `pn-code` (it is a structural hook for blockTargeting, vim navigation and print.css, and it named a library we no longer ship). `language-*` stays. apps/review/dist/index.html 18,180,545 -> 17,270,889 (-909,656 raw / -291,921 gzip) apps/hook/dist/index.html 22,410,416 -> 21,704,434 (-705,982 raw / -238,096 gzip) Verified the diff pane is untouched: the rendered Pierre shadow-DOM markup is byte-for-byte identical between an origin/main build and this one (SHA-256 aa1ee88a…). * fix(ui): strip stray NUL bytes from the code-highlight source Two U+0000 bytes slipped into comments in the previous commit, which made git treat the file as binary. Replaced with spaces; no behaviour change. * fix(ui): keep code-block annotation marks across highlight swaps Fenced code is annotated by hand: one `<mark data-bind-id>` inside the `<code>` element, which `applyHighlight` also owns. Every highlight swap (palette change, dark/light toggle, or the first async grammar attach after load) replaces that element's children, so the mark was silently wiped and nothing put it back. Annotation state, the sidebar panel and exports were unaffected; the loss was purely visual, and deterministic. `applyHighlight` now publishes every write through `onCodeHighlightSwap`, synchronously, immediately after it. `Viewer` subscribes and re-paints the fence's mark, so a swapped block ends up with BOTH the new theme's tokens and its annotation. The shared painter (`paintCodeBlockMark`) moves the token spans into the mark instead of flattening them to text, so creating an annotation no longer costs a block its colours either. Being driven by the swap also fixes the cousin race by ordering rather than timing: share/draft restore runs on a timer after load, and on a slow machine the first async swap could land after it and wipe the restored marks per block. A restore that painted before the swap is now re-established in the same task the swap ran in, and one that runs after finds the mark already there. Removal tombstones the id before re-highlighting, because the host drops the annotation from state a tick later — without it the swap listener would paint the just-removed annotation back in, and a fence carrying a second annotation would end up bare. Also closes the named gap in the WASM coverage: entry-assets only grepped source, so a future @pierre/diffs bump could reintroduce the inlined blob through a different import specifier unnoticed. It now greps the built `apps/{review,hook}/dist/index.html` for the base64 WASM magic, skipping on an unbuilt checkout and running for real in the CI job that builds the bundles.
This PR makes bare fenced code blocks (triple backticks with no language tag) render as plain monospaced text instead of getting auto-detected syntax highlighting.
hljs.highlightElement()was called unconditionally in three components. When a block has no language, highlight.js falls back to auto-detection and colors ordinary words likein,and,open,is,to. This is most visible in annotate mode when a document uses fenced blocks as plain-text quote/preview containers.The parser already sets
languagetoundefinedfor a bare fence, so the fix just guards the highlight call in:packages/ui/components/blocks/CodeBlock.tsxpackages/ui/components/plan-diff/PlanCleanDiffView.tsxpackages/ui/components/Viewer.tsxBlocks with a language are unchanged.
Fixes #1210