Skip to content

fix(render): unwrap a bare shiki class on pre back to a plain fence#259

Merged
farnabaz merged 9 commits into
comarkdown:mainfrom
hendrikheil:fix/pre-shiki-class-roundtrip
Jun 30, 2026
Merged

fix(render): unwrap a bare shiki class on pre back to a plain fence#259
farnabaz merged 9 commits into
comarkdown:mainfrom
hendrikheil:fix/pre-shiki-class-roundtrip

Conversation

@hendrikheil

@hendrikheil hendrikheil commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Keeps shiki-highlighted code blocks round-tripping to a plain fence instead of leaking a ::pre{.shiki} block-attribute wrapper.

The leak was introduced in #214, which added the ::pre{attr} wrapper form together with the userBlockAttrs strip guard — that guard never matched the bare shiki class.

The fix

The ::pre{attr} wrapper form treats any class that survives userBlockAttrs as a user attribute. userBlockAttrs strips shiki's injected class, but the guard only matched class="shiki " with a trailing space (the multi-token dual-theme form). Single-theme shiki emits a bare class="shiki", which slipped through and forced every highlighted code block to serialize as a ::pre{.shiki} wrapper — especially visible for code blocks nested in component slots (which also produced a duplicate closing ::).

Widened the guard from value.startsWith('shiki ') to value.startsWith('shiki').

Tests

packages/comark/test/plugins/highlight.test.ts:

  • bare class="shiki" → plain fence
  • multi-token class="shiki shiki-themes … dark:…" → plain fence (unchanged)
  • highlighted code block nested in a component slot → plain fence, no ::pre

Full suite green (1079 passed, 2 todo); existing shiki SPEC fixtures unaffected.

🤖 Generated with Claude Code

The `::pre{attr}` block-attribute wrapper form treats any surviving `class`
on a `pre` node as a user attribute. `userBlockAttrs` strips shiki's injected
class so highlighted code blocks round-trip to a plain fence, but the guard
only matched `class="shiki ..."` (with trailing theme tokens). Single-theme
shiki emits a bare `class="shiki"`, which slipped through and forced every
highlighted code block to serialize as `::pre{.shiki}\n```…```\n::` instead
of a clean fence — including code blocks nested inside component slots.

Widen the guard to `/^shiki(?:\s|$)/` so the bare-`shiki` case is recognized
and dropped like the multi-token form.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@vercel

vercel Bot commented Jun 29, 2026

Copy link
Copy Markdown

@hendrikheil is attempting to deploy a commit to the NuxtLabs Team on Vercel.

A member of the Team first needs to authorize it.

@pkg-pr-new

pkg-pr-new Bot commented Jun 29, 2026

Copy link
Copy Markdown

Open in StackBlitz

comark

npm i https://pkg.pr.new/comarkdown/comark@259

@comark/angular

npm i https://pkg.pr.new/comarkdown/comark/@comark/angular@259

@comark/ansi

npm i https://pkg.pr.new/comarkdown/comark/@comark/ansi@259

@comark/html

npm i https://pkg.pr.new/comarkdown/comark/@comark/html@259

@comark/nuxt

npm i https://pkg.pr.new/comarkdown/comark/@comark/nuxt@259

@comark/react

npm i https://pkg.pr.new/comarkdown/comark/@comark/react@259

@comark/svelte

npm i https://pkg.pr.new/comarkdown/comark/@comark/svelte@259

@comark/vue

npm i https://pkg.pr.new/comarkdown/comark/@comark/vue@259

commit: c5cfd14

hendrikheil and others added 3 commits June 29, 2026 14:07
`highlightCodeBlocks` merged any existing `pre` class as a user class via the
`shiki … . <user>` form. Running it twice on the same tree (e.g. a stored,
already-highlighted block re-highlighted after an edit round-trip) treated the
prior `shiki` class as a user class, accumulating `shiki . shiki` — which then
leaked back out as a `::pre{.shiki}` wrapper on render.

Recover the real user class from a previously-injected class before re-merging,
so the result is stable across passes. Genuine user classes are still preserved.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The idempotency helper and the widened guard add ~1k to comark's published
size, crossing the rounded-kilobyte boundary in the bundle-size snapshot.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The `pre` handler reads `node[1].code` as the fence body when present (editors
preserve the raw source there while children hold highlighted spans). Since it
rides in the fence, `code` must not also echo back as a `::pre{code="…"}`
block-attribute wrapper. Add it to the `pre` implicit-attr drop list alongside
`language`/`filename`/etc. Also bump the bundle-size snapshot accordingly.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
hendrikheil and others added 2 commits June 29, 2026 15:56
Move shiki-codeblock-roundtrip.test.ts to plugins/highlight.test.ts to
match the per-plugin test convention.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Trimming the verbose comments shrank the (unminified, tsc-built) dist.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@hendrikheil hendrikheil marked this pull request as ready for review June 29, 2026 14:12
@hendrikheil hendrikheil requested a review from farnabaz as a code owner June 29, 2026 14:12
Replace the three hard-coded `.`/` . ` sentinels (highlight plugin write +
re-highlight read, stringify read) with HIGHLIGHT_CLASS_SEPARATOR from the
stringify module.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@farnabaz

Copy link
Copy Markdown
Collaborator

Thanks for the PR @hendrikheil
I think the changes are somewhat too much and changing line 131 in attributes.ts would be enough:

-    if (key === 'class' && tag === 'pre' && typeof value === 'string' && value.startsWith('shiki ')) {
+    if (key === 'class' && tag === 'pre' && typeof value === 'string' && value.startsWith('shiki')) {

But as for code prop, does it come from Nuxt Content / MDC packages? Comark parser does not add code prop anymore.

@farnabaz

Copy link
Copy Markdown
Collaborator

We can keep code attribute in drop list for now to ease the migration and remove it later.

@hendrikheil

hendrikheil commented Jun 29, 2026

Copy link
Copy Markdown
Contributor Author

The PR really contains two fixes:

  1. strip guard: I can collapse it to startsWith('shiki'), I only used /^shiki(?:\s|$)/ so a genuine user class that happens to start with shiki (e.g. ::pre{.shiki-toggle}) isn't stripped as if it were highlighter output. Cheap insurance, but I'm fine going with the simpler form if you'd rather have that 👍
  2. highlighting idempotency: highlightCodeBlocks merges the entire existing pre.class as a user class, so running it twice on the same tree produces shiki . shiki, which then leaks back out as ::pre{.shiki} regardless of the strip guard

Drop the highlight idempotency change — re-highlighting can't accumulate
`shiki . shiki` in practice, since highlightCodeBlocks only selects `pre >
code` nodes whose code child is still a string and skips already-highlighted
blocks. Collapse the render-side guard to `startsWith('shiki')` and keep the
`code` drop (nuxt-studio stores the raw source there). Revert highlight.ts and
the bundle-size snapshot to match.
Drop the `code` attribute change — the `code`-on-`pre` leak will be fixed at
the source in nuxt-studio instead. Leaves only the render-side guard widening
(`startsWith('shiki ')` -> `startsWith('shiki')`) so a single-theme `shiki`
class unwraps to a plain fence, plus the test covering it.
@hendrikheil

Copy link
Copy Markdown
Contributor Author

Actually, I'll just fix the code issue in nuxt-studio directly, no need to do it in comark imo. I've also dropped the rehighlighting path. Let's keep this PR focused on the primary issue we're encountering in nuxt-studio right now...

@farnabaz farnabaz 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.

Thanks

@farnabaz farnabaz merged commit 63d0217 into comarkdown:main Jun 30, 2026
3 of 11 checks passed
@hendrikheil hendrikheil deleted the fix/pre-shiki-class-roundtrip branch June 30, 2026 11:04
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