ReviewOS

also looking at this

stacks/ts-syntax-highlighter

fix: browser-safe config loading and correct dual-theme token colors

#477
Open jmadore-payfacto wants to merge fix/browser-safe-config-and-dual-theme-colors into main
6 files +237 -21
8 days ago

What's broken

1. Importing anything from this package crashes immediately in a browser.

config.ts has a static top-level import { loadConfig } from 'bunfig'. bunfig's own module has an eager, unguarded process.env.CLARITY_LOG_DIR access that runs the moment it's evaluated - fine under Bun/Node, but process doesn't exist in a browser, so:

TypeError: Cannot read properties of undefined (reading 'CLARITY_LOG_DIR')

This throws on import even for a consumer who only calls resolveGrammar/Tokenizer/Renderer/findLanguageForFilename and never touches config loading at all, because index.ts re-exports everything from config.ts.

2. renderDualTheme doesn't actually recolor tokens for dark mode.

It renders both themes but only ever returns the light theme's html. Every Renderer output bakes each token's color into style="color:...", so no stylesheet - light or dark - can override it. The returned "dark" stylesheet only ever recolors the wrapper background and base text; every keyword, string, and number keeps the light theme's color on a dark background.

What this fixes

  • src/config.ts: bunfig is now imported dynamically inside loadSyntaxHighlighterConfig(), the only place that needs it, instead of at module scope. Nothing else changes about its behavior - config loading still works exactly the same for a caller that opts in.
  • src/renderer.ts / src/types.ts: added a colorViaClass render option. When set, a token's color is recorded as a .token.<scope> { color: ... } rule instead of inlined, leaving only fontStyle-derived styles (italic/bold/underline, which don't vary by theme) in the inline style.
  • src/dual-theme.ts: renderDualTheme now renders both themes in colorViaClass mode and rescopes the dark ruleset under the caller's selector, so the returned stylesheet actually overrides token colors per theme.

Testing

  • test/config.test.ts (new): asserts the default config still works without calling loadSyntaxHighlighterConfig(), that loadSyntaxHighlighterConfig() still loads from disk when called, and a structural check that config.ts never regresses to a static bunfig import.
  • test/dual-theme.test.ts (new): asserts no theme color is ever inlined into the HTML, that the light color rule applies with no selector prefix, that the dark color rule only applies under the given selector, and that markup is identical regardless of which themes are passed.
  • Full suite: 1008 passing (was 1001 before this change), 0 failures.
  • Verified against a real downstream app (production Vite/rolldown build, real browser via Chrome DevTools MCP): the crash is gone, and dark-mode code previews now render with correct per-token colors instead of light colors on a dark background.

No existing test in the suite exercised renderDualTheme before this PR, which is likely why both issues went unnoticed.

6 changed files on the files tab, with 0 review threads.

Sign in to comment on this pull request.