fix(core): expose multi-theme keys on <code> for themed backgrounds by pablofdezr · Pull Request #274 · rehype-pretty/rehype-pretty-code

pablofdezr · 2026-07-24T12:09:12Z

Summary

Closes #222.

When multiple themes are configured, Shiki emits the theme keys
(--shiki-light-bg, --shiki-dark-bg, …) as inline styles on the
<pre> element only. However, the documented way to theme a code
block's background targets <code>:

code[data-theme*=' '] {
  color: var(--shiki-light);
  background-color: var(--shiki-light-bg);
}

Today that selector resolves only because CSS custom properties
inherit from <pre> down to <code> — the background variables are
never actually present on <code>, even though the docs (and the
container itself, via data-theme) imply they are. As #222 notes, the
keys are only really set on the <span>s and, since a later change, on
<pre>.

This PR mirrors the <pre> theme-key style onto <code> when multiple
themes are active and keepBackground is on, so the container's own
background can be styled per theme without relying on inheritance.

Behavior

Case	`<code>` style
Multiple themes (default `keepBackground: true`)	now carries `--shiki-` + `--shiki--bg` keys ✅
Multiple themes, `keepBackground: false`	unchanged — keys stripped from both `<pre>` and `<code>`
Single theme	unchanged — `<pre>` paints real `background-color`/`color`, so we deliberately do not duplicate it onto `<code>`

The themeNames.length > 1 guard aligns exactly with the documented
code[data-theme*=' '] selector, which only matches when data-theme
contains a space (i.e. two or more themes).

Diff

// after the inline branch, before the grid branch:
if (
  keepBackground &&
  themeNames.length > 1 &&
  typeof pre.properties.style === 'string'
) {
  code.properties.style = code.properties.style
    ? `${pre.properties.style};${code.properties.style}`
    : `${pre.properties.style};`;
}

Tests

Added a dedicated fixture codeBackgroundMultipleTheme.md asserting the
<code> element carries the theme keys.
Updated the existing showLineNumbersStartAtMultipleTheme snapshot
(only the <code> style line changed).
Full suite green (38/38); single-theme snapshots are untouched,
confirming no double-paint regression.
typecheck and biome check both pass.

With multiple themes, Shiki emits the theme keys (`--shiki-light-bg`, `--shiki-dark-bg`, ...) as inline styles on the `<pre>` element only. rehype-pretty-code's documented CSS targets `<code>`: code[data-theme*=' '] { background-color: var(--shiki-light-bg); } That worked only because the custom properties inherit from `<pre>` down to `<code>`, so the background variables were never actually present on `<code>` as the docs (and the container itself) imply. This mirrors the `<pre>` theme-key style onto `<code>` when multiple themes are active and `keepBackground` is on, letting the container's own background be styled per theme without relying on inheritance. Single-theme output is untouched (its `<pre>` style paints real colors, which we must not duplicate onto `<code>`), and `keepBackground: false` still strips the keys from both elements. Closes rehype-pretty#222 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

changeset-bot · 2026-07-24T12:09:22Z

🦋 Changeset detected

Latest commit: 8f5bb60

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
rehype-pretty-code	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

pkg-pr-new · 2026-07-24T12:09:50Z

npm i https://pkg.pr.new/rehype-pretty-code@274

npm i https://pkg.pr.new/@rehype-pretty/transformers@274

commit: 8f5bb60

atomiks · 2026-07-24T19:58:15Z

PR review

A transformer-style leak can unexpectedly change <code> layout in multi-theme configurations. The existing suite passes, but a focused transformer probe reproduces the issue.

Bugs (1)

1. 🔴 Copying the complete pre style leaks unrelated declarations

Location: packages/core/src/index.ts:122

code.properties.style = code.properties.style
  ? `${pre.properties.style};${code.properties.style}`
  : `${pre.properties.style};`;

The change intends to expose Shiki’s theme variables, but it copies every declaration added to <pre>, including styles supplied by public Shiki transformers.

Failure scenario: A transformer adds padding:123px to <pre>. With multiple themes, <code> also receives that padding and its layout changes unexpectedly.

Fix: Copy only the relevant --shiki-* custom-property declarations and add a transformer regression test.

Docs (1)

1. 🟠 Add a patch changeset

Location: packages/core/src/index.ts:112

// Shiki emits the dual-theme keys ... as inline styles on <pre> only.

This changes published rehype-pretty-code behavior but has no changeset, so merging it will not schedule a package version bump.

Failure scenario: The fix merges successfully but is absent from the next release because Changesets sees no package change.

Fix: Add a changeset declaring "rehype-pretty-code": patch.

Verdict

Request changes - the broad style copy can regress public transformer configurations.

🤖 Review generated with Codex

Copying the whole <pre> style moved every declaration on it, including styling owned by user transformers: a transformer adding `padding:123px` to <pre> also changed <code>'s layout under a multi-theme config. Filter the copy down to the `--shiki-*` custom properties, which is all the themed-background selector needs. Adds a transformer regression test asserting the padding stays on <pre> while the theme variables still reach <code>. Existing snapshots are unchanged: Shiki writes nothing but those custom properties on <pre> in these fixtures. Also adds the patch changeset.

atomiks · 2026-07-24T21:12:09Z

PR review

The transformer-leak fix from the previous round is correct, but the premise underneath it does not hold: Shiki already writes the --shiki-* variables onto <pre>, and custom properties inherit, so the documented code[data-theme*=' '] selector resolves them today without this change. As written the PR ships duplicated inline styles on every multi-theme <code> for zero rendering difference, which is merge-blocking in the sense that the change has no effect to justify it.

Simplifications (2)

1. 🔴 The mirrored declarations change nothing — `<pre>` already provides them by inheritance

Location: packages/core/src/index.ts:113

if (
  keepBackground &&
  themeNames.length > 1 &&
  typeof pre.properties.style === 'string'
) {
  const themeProperties = getShikiCustomProperties(pre.properties.style);

getOptions passes defaultColor: false for multi-theme configs (packages/core/src/index.ts:220), so Shiki emits every theme's variables as custom properties on <pre> rather than resolving one theme to plain color / background-color. This is visible in the committed fixtures on master:

<pre style="--shiki-dark:#e1e4e8;--shiki-light:#24292e;--shiki-dark-bg:#24292e;--shiki-light-bg:#fff" ...>

CSS custom properties inherit, so <code> and its descendant spans already resolve var(--shiki-light-bg). Serving packages/core/test/results/showLineNumbersStartAtMultipleTheme.html from master (unpatched) and reading the computed style of code[data-theme*=" "] confirms it:

{
  "inline": "display: grid;",
  "lightBg": "#fff",
  "darkBg": "#24292e",
  "bg": "rgb(36, 41, 46)",
  "color": "rgb(225, 228, 232)"
}

No inline --shiki-* on <code>, yet both variables resolve and the background computes to the active theme's colour. Issue #222 predates Shiki emitting these on <pre> and appears to be stale.

The changeset states the goal as applying the theme background "without relying on inheritance from <pre>" — but inheritance is the intended mechanism for custom properties, and opting out of it is what costs the extra bytes.

Failure scenario: Every multi-theme code block gains roughly 100 bytes of duplicated inline style in the rendered HTML, on a patch release, with no visual or behavioural change for any consumer.

Fix: Close the PR and the issue as already-resolved, or repurpose it into a docs note on #222 explaining that the variables live on <pre> and inherit. If there is a concrete configuration where inheritance does not reach <code>, add a failing test for that configuration first — the current fixture passes identically before and after the source change.

2. 🟡 The inline-code path still copies the whole `<pre>` style

Location: packages/core/src/index.ts:105

if (inline) {
  if (keepBackground) {
    code.properties.style = pre.properties.style;
  }
  return code;
}

The new block sits after this early return, so the transformer-leak problem raised in the previous review still applies verbatim to inline code: a transformer that adds padding:123px to <pre> has it copied onto the inline <code>. The regression test added in this PR only exercises the block path.

Failure scenario: A transformer sets a padding or margin on <pre>; inline code spans inherit that layout declaration and render with unexpected spacing.

Fix: If any part of this PR is kept, route the inline path through getShikiCustomProperties as well, and extend the new test to cover inline code.

Verdict

Request changes - the change is verifiably a no-op: the documented selector already resolves the theme variables through inheritance from <pre> on master.

🤖 Review generated with Claude Code

pablofdezr · 2026-07-24T21:16:31Z

Addressed the review.

The copy is now filtered down to the --shiki-* custom properties via a getShikiCustomProperties() helper in utils.ts, so declarations owned by someone else — a transformer's own styling in particular — stay on <pre>.

Added the transformer regression test: a transformer puts padding:123px on <pre>, and the test asserts that --shiki-light-bg/--shiki-dark-bg still reach <code> while the padding does not. Confirmed it fails without the fix (expected ... not to contain 'padding') and passes with it.

Existing snapshots are unchanged — Shiki writes nothing but those custom properties on <pre> in these fixtures, so the narrower copy produces identical output.

Patch changeset added. Ready for another look.

atomiks · 2026-07-24T23:19:47Z

Before this year these would've been merged already 😅. But now models keep finding issues that the back-and-forth takes forever.

PR review

This introduces a merge-blocking CSS-cascade regression. Default rendering remains unchanged, but consumers can no longer override the documented multi-theme variables using ordinary CSS.

Bugs (1)

1. 🔴 Mirrored theme variables block consumer overrides

Location: packages/core/src/index.ts:127

const themeProperties = getShikiCustomProperties(pre.properties.style);
if (themeProperties.length > 0) {
  const declarations = `${themeProperties.join(';')};`;
  code.properties.style = code.properties.style
    ? `${declarations}${code.properties.style}`
    : declarations;
}

On master, the --shiki-* variables are declared on <pre> and inherited by <code>. A consumer declaration directly on <code> therefore overrides the inherited value. This PR copies the variables into <code>’s inline style, which outranks normal stylesheet declarations.

Failure scenario: A consumer applies code[data-theme*=' '] { --shiki-light-bg: hotpink }. The background is hot pink on master but remains Shiki’s inline white after this change unless the consumer adds !important.

Fix: Remove the mirroring block and its helper/output changes. The variables already resolve through inheritance; emitting them inline necessarily changes their override precedence.

Verdict

Request changes - the inline declarations break the public multi-theme customization contract.

🤖 Review generated with Codex

pablofdezr · 2026-07-25T10:10:48Z

You're right on both counts, and I checked each before conceding rather than taking the review's word for it.

On master, multi-theme <pre> already carries the whole set:

<pre style="--shiki-dark:#e1e4e8;--shiki-light:#24292e;--shiki-dark-bg:#24292e;--shiki-light-bg:#fff">
  <code data-theme="github-dark github-light" style="display: grid;">

<code> carries none of them, and custom properties inherit, so code[data-theme*=' '] resolves var(--shiki-light-bg) today without this change. getOptions passes defaultColor: false for multi-theme configs, so this is what every multi-theme fixture in the repo looks like. There is no block configuration where <code> is not a descendant of that <pre> — the inline path is the only one that lacks the parent, and it already copies the style across. So the premise I built this on doesn't hold: the PR changes no rendering.

And the cascade point is the more serious half. Mirroring the variables into <code>'s inline style outranks any author stylesheet, so code[data-theme*=' '] { --shiki-light-bg: hotpink } works on master and would silently stop working after this — for a patch release, in exchange for nothing. That is a worse trade than the no-op alone.

Closing this. #222 looks stale for the same reason: the variables it asks for are on <pre> and reach <code> by inheritance. Happy to send a short docs note saying so if you'd rather have it written down than just close the issue.

Sorry for the rounds this one took to get to a clear answer.

pablofdezr closed this Jul 25, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(core): expose multi-theme keys on <code> for themed backgrounds - #274

fix(core): expose multi-theme keys on <code> for themed backgrounds#274
pablofdezr wants to merge 2 commits into
rehype-pretty:masterfrom
pablofdezr:fix/multi-theme-code-background

pablofdezr commented Jul 24, 2026

Uh oh!

changeset-bot Bot commented Jul 24, 2026 •

edited

Loading

Uh oh!

pkg-pr-new Bot commented Jul 24, 2026 •

edited

Loading

Uh oh!

atomiks commented Jul 24, 2026

Uh oh!

atomiks commented Jul 24, 2026

Uh oh!

pablofdezr commented Jul 24, 2026

Uh oh!

atomiks commented Jul 24, 2026

Uh oh!

pablofdezr commented Jul 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

pablofdezr commented Jul 24, 2026

Summary

Behavior

Diff

Tests

Uh oh!

changeset-bot Bot commented Jul 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

pkg-pr-new Bot commented Jul 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

atomiks commented Jul 24, 2026

PR review

Bugs (1)

1. 🔴 Copying the complete pre style leaks unrelated declarations

Docs (1)

1. 🟠 Add a patch changeset

Verdict

Uh oh!

atomiks commented Jul 24, 2026

PR review

Simplifications (2)

1. 🔴 The mirrored declarations change nothing — <pre> already provides them by inheritance

2. 🟡 The inline-code path still copies the whole <pre> style

Verdict

Uh oh!

pablofdezr commented Jul 24, 2026

Uh oh!

atomiks commented Jul 24, 2026

PR review

Bugs (1)

1. 🔴 Mirrored theme variables block consumer overrides

Verdict

Uh oh!

pablofdezr commented Jul 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

changeset-bot Bot commented Jul 24, 2026 •

edited

Loading

pkg-pr-new Bot commented Jul 24, 2026 •

edited

Loading

1. 🔴 The mirrored declarations change nothing — `<pre>` already provides them by inheritance

2. 🟡 The inline-code path still copies the whole `<pre>` style