ReviewOS

stacks/stx

CodeBlock ships its code into a client script even when copyable is false

#2014
Closed glennmichael123 opened this 9 days ago ·
9 days ago

<CodeBlock> puts its code prop into a client script unconditionally, which makes it unusable for any snippet that is not safe to ship to the browser as data.

The component

src/components/CodeBlock.stx:

<script client>
const code = {{ code }}
const copied = state(false)

async function copyToClipboard() {
  await navigator.clipboard.writeText(code)
  ...
}
</script>

The block is top-level. copyable only gates the button in the template:

@if(copyable)
  <button @click="copyToClipboard()" ...>
@endif

So :copyable="false" still emits const code = ... with the full content. There is no way to render a highlighted block without also shipping its text into page JavaScript.

Why that blocks adoption

A status-page app has five code blocks, all of them install or ping snippets that embed a per-resource credential. The app keeps those bindings behind a __ prefix specifically because stx's extractBridgeData skips __ and $ names, and it has a test enforcing that no server binding reaches page HTML through the bridge, written after raw rows carrying a token leaked into dashboard HTML once.

:code="__agentInstall.install" routes straight around that. The snippet is already visible as rendered text, so this is not a new disclosure to a third party, but it does defeat an invariant the app deliberately maintains, and a component should not force that choice on a consumer who has explicitly turned the copy button off.

It is also dead weight in the ordinary case: a block with copyable: false ships a duplicate of its own content for no reader-visible benefit.

Suggested

Emit the client script only when copyable is true. If the current single-block form has to stay, reading the text from the DOM at click time rather than from an interpolated constant would also work, and would drop the duplicate payload for every consumer.

Two smaller things found alongside

The copy button is invisible to keyboard and touch. It is opacity-0 group-hover:opacity-100 with no focus variant, so a keyboard user who tabs to it sees nothing, and on a touch device it never appears at all. focus-visible:opacity-100 would fix the keyboard half; the touch half needs the button not to depend on hover.

highlightCss is exported but never rendered, with a source comment explaining that a <style> in a component is extracted before interpolation. That is stacksjs/stx#1990, which is now closed, so this may be ready to un-work-around: if it is, the component can ship its own theme CSS and consumers stop having to plumb it by hand.

9 days ago

Fixed in b4d2b99667. There were two causes, and you found the second one's symptom without seeing it.

1. The ungated payload

As reported. The snippet is now decided server-side as copyPayload = copyable ? code : '', so copyable: false emits const code = ""; and nothing else.

2. copyable="false" was never false

export const copyable = $props.copyable !== false

A plain attribute arrives as a string, so 'false' !== false is true and the button rendered however you wrote the prop. That is stacksjs/stx#2006 in a third spelling — !== false — which the library-wide $bool sweep does not recognise. lineNumbers had it too. Both read through $bool now, and the test covers "false", "0", "no" and "off" alongside the bare and "true" forms.

So part of what you were seeing was the gate not working at all, not just the script being outside it.

On reading from the DOM instead

reading the text from the DOM at click time rather than from an interpolated constant would also work, and would drop the duplicate payload for every consumer

I tried this first because it is the better shape, and measured it before rejecting it: the highlighter renders line numbers as text.

h.highlight('const a = 1', 'typescript', { lineNumbers: true })
  -> textContent: "1const a = 1"

lineNumbers is currently accepted and dropped by the wrapper, so today textContent would be clean — but the moment that is honoured, every copy silently gains line numbers. Gating cannot corrupt the clipboard; DOM-reading can, and would do it quietly. Worth revisiting if the highlighter ever emits line numbers as a CSS counter.

Also note the server-side expression is a named export rather than an inline ternary: does not work in a client script. interpolateScriptExpressions resolves an identifier or a property path, not an arbitrary expression, so a ternary emits nothing and the declaration came out as a bare const code = .

The copy button

focus-visible:opacity-100 fixes the keyboard half. The touch half has nothing to hang on — there is no @media (hover: none) variant in the engine — so the button is now always present at 60% and comes to full opacity on hover or focus. That is a visible change: it no longer hides until pointed at. It is the only arrangement that is operable on a phone, and it matches what GitHub and most code viewers do.

highlightCss — measured, not done

Both halves of the answer, so whoever picks it up does not have to re-measure:

  • A component <style> does interpolate now that #1990 is closed. Verified: a component emitting <style>:root[data-syntax-theme='github-dark']{--bg:#0d1117;--surface:#161b22;--surface-tint:#161b22;--line:#2f3237;--line-strong:#2f3237;--text:#c9d1d9;--muted:#747b82;--gutter:#161b22;--t-comment:#8b949e;--t-string:#a5d6ff;--t-numeric:#79c0ff;--t-keyword:#ff7b72;--t-operator:#ff7b72;--t-function:#d2a8ff;--t-type:#ffa657;--t-variable:#ffa657;--t-tag:#7ee787;--t-attribute:#79c0ff;--t-punctuation:#c9d1d9;--t-comment-style:italic} :root[data-syntax-theme='github-light']{--bg:#ffffff;--surface:#f6f8fa;--surface-tint:#f6f8fa;--line:#e6e6e6;--line-strong:#e6e6e6;--text:#24292f;--muted:#87898d;--gutter:#f6f8fa;--t-comment:#6e7781;--t-string:#0a3069;--t-numeric:#0550ae;--t-keyword:#cf222e;--t-operator:#cf222e;--t-function:#8250df;--t-type:#953800;--t-variable:#953800;--t-tag:#116329;--t-attribute:#0550ae;--t-punctuation:#24292f;--t-comment-style:italic} :root[data-syntax-theme='nord']{--bg:#2e3440;--surface:#3b4252;--surface-tint:#3b4252;--line:#4b505b;--line-strong:#4b505b;--text:#d8dee9;--muted:#8c929d;--gutter:#3b4252;--t-comment:#616e88;--t-string:#a3be8c;--t-numeric:#b48ead;--t-keyword:#81a1c1;--t-operator:#81a1c1;--t-function:#88c0d0;--t-type:#8fbcbb;--t-variable:#d8dee9;--t-tag:#81a1c1;--t-attribute:#8fbcbb;--t-punctuation:#eceff4;--t-comment-style:italic} :root[data-syntax-theme='midnight']{--bg:#10131c;--surface:#171b28;--surface-tint:#171b28;--line:#31343c;--line-strong:#31343c;--text:#c8d0e0;--muted:#757b88;--gutter:#171b28;--t-comment:#8790a8;--t-string:#7fd6a2;--t-numeric:#f2a97e;--t-keyword:#7aa2f7;--t-function:#7dcfff;--t-type:#4fd6be;--t-variable:#c8d0e0;--t-tag:#7aa2f7;--t-attribute:#4fd6be;--t-punctuation:#98a1b8;--t-operator:#89ddff;--t-comment-style:italic} :root[data-syntax-theme='daylight']{--bg:#fdfbf7;--surface:#f3efe7;--surface-tint:#f3efe7;--line:#e4e2de;--line-strong:#e4e2de;--text:#33302c;--muted:#8e8b87;--gutter:#f3efe7;--t-comment:#6f6960;--t-string:#2f7d52;--t-numeric:#a35a17;--t-keyword:#1e5fbf;--t-function:#0b6b8f;--t-type:#0f6b5f;--t-variable:#33302c;--t-tag:#1e5fbf;--t-attribute:#0f6b5f;--t-punctuation:#635e58;--t-operator:#8a4a12;--t-comment-style:italic} :root[data-syntax-theme='ember']{--bg:#191210;--surface:#221917;--surface-tint:#221917;--line:#393331;--line-strong:#393331;--text:#e8d9cf;--muted:#8b7f79;--gutter:#221917;--t-comment:#9d8378;--t-string:#b8cf7a;--t-numeric:#f0a05a;--t-keyword:#ff8f6b;--t-function:#ffc36b;--t-type:#7fc3c0;--t-variable:#e8d9cf;--t-tag:#ff8f6b;--t-attribute:#7fc3c0;--t-punctuation:#ab9086;--t-operator:#f0a05a;--t-comment-style:italic} :root[data-syntax-theme='moss']{--bg:#101613;--surface:#16201b;--surface-tint:#16201b;--line:#313734;--line-strong:#313734;--text:#cfdcd2;--muted:#79837c;--gutter:#16201b;--t-comment:#7d928a;--t-string:#a8d5a2;--t-numeric:#e0b56a;--t-keyword:#6fc2a0;--t-function:#b8d97a;--t-type:#7fd0c0;--t-variable:#cfdcd2;--t-tag:#6fc2a0;--t-attribute:#7fd0c0;--t-punctuation:#96a99e;--t-operator:#e0b56a;--t-comment-style:italic} :root[data-syntax-theme='paper']{--bg:#ffffff;--surface:#f2f2f2;--surface-tint:#f2f2f2;--line:#e6e6e6;--line-strong:#e6e6e6;--text:#1a1a1a;--muted:#818181;--gutter:#f2f2f2;--t-comment:#5f5f5f;--t-string:#0a6b2f;--t-numeric:#8a3d00;--t-keyword:#0033b3;--t-function:#00548a;--t-type:#005f56;--t-variable:#1a1a1a;--t-tag:#0033b3;--t-attribute:#005f56;--t-punctuation:#4a4a4a;--t-operator:#8a3d00;--t-comment-style:italic} :root[data-syntax-theme='dusk']{--bg:#16121f;--surface:#1e1929;--surface-tint:#1e1929;--line:#37333e;--line-strong:#37333e;--text:#d5cde0;--muted:#7f7989;--gutter:#1e1929;--t-comment:#8b81a1;--t-string:#9ad9a0;--t-numeric:#f0b26a;--t-keyword:#a78bfa;--t-function:#e2b0ff;--t-type:#7fd6d0;--t-variable:#d5cde0;--t-tag:#a78bfa;--t-attribute:#7fd6d0;--t-punctuation:#9d94b0;--t-operator:#f0b26a;--t-comment-style:italic} :root[data-syntax-theme='sea']{--bg:#f7fbfb;--surface:#eaf3f3;--surface-tint:#eaf3f3;--line:#dee2e2;--line-strong:#dee2e2;--text:#1f3336;--muted:#808d8f;--gutter:#eaf3f3;--t-comment:#57696c;--t-string:#0d6b52;--t-numeric:#8a4700;--t-keyword:#0a5c8a;--t-function:#0d7a8f;--t-type:#0b6b63;--t-variable:#1f3336;--t-tag:#0a5c8a;--t-attribute:#0b6b63;--t-punctuation:#4d5f62;--t-operator:#8a4700;--t-comment-style:italic} :root[data-syntax-theme='deuteranopia-dark']{--bg:#12141a;--surface:#1a1d26;--surface-tint:#1a1d26;--line:#33353a;--line-strong:#33353a;--text:#d3d8e0;--muted:#7c8087;--gutter:#1a1d26;--t-comment:#8b94a5;--t-string:#f0b429;--t-numeric:#e08b2e;--t-keyword:#58a6ff;--t-function:#ffd98a;--t-type:#79c0ff;--t-variable:#d3d8e0;--t-tag:#58a6ff;--t-attribute:#8fb8ff;--t-punctuation:#9aa3b2;--t-operator:#ffd98a;--t-comment-style:italic} :root[data-syntax-theme='deuteranopia-light']{--bg:#ffffff;--surface:#f0f3f8;--surface-tint:#f0f3f8;--line:#e6e6e6;--line-strong:#e6e6e6;--text:#1c2430;--muted:#82878d;--gutter:#f0f3f8;--t-comment:#5b6675;--t-string:#8a5a00;--t-numeric:#6b4300;--t-keyword:#0b4fa8;--t-function:#a06a00;--t-type:#0a6bbd;--t-variable:#1c2430;--t-tag:#0b4fa8;--t-attribute:#2f5fb8;--t-punctuation:#4d5866;--t-operator:#6b4300;--t-comment-style:italic} :root[data-syntax-theme='tritanopia-dark']{--bg:#14121a;--surface:#1d1a25;--surface-tint:#1d1a25;--line:#35333a;--line-strong:#35333a;--text:#ded6e2;--muted:#837e88;--gutter:#1d1a25;--t-comment:#9b90a6;--t-string:#4fd1c5;--t-numeric:#ff8fd0;--t-keyword:#ff6ec7;--t-function:#7fe3d8;--t-type:#ff9ecb;--t-variable:#ded6e2;--t-tag:#ff6ec7;--t-attribute:#7fe3d8;--t-punctuation:#a99eb3;--t-operator:#ff8fd0;--t-comment-style:italic} :root[data-syntax-theme='tritanopia-light']{--bg:#fffcff;--surface:#f5eef6;--surface-tint:#f5eef6;--line:#e6e3e6;--line-strong:#e6e3e6;--text:#241f28;--muted:#878289;--gutter:#f5eef6;--t-comment:#635a68;--t-string:#006d63;--t-numeric:#a3004f;--t-keyword:#b0007a;--t-function:#00566b;--t-type:#8a0060;--t-variable:#241f28;--t-tag:#b0007a;--t-attribute:#00566b;--t-punctuation:#575060;--t-operator:#a3004f;--t-comment-style:italic} :root[data-syntax-theme='monochrome-dark']{--bg:#101010;--surface:#1b1b1b;--surface-tint:#1b1b1b;--line:#313131;--line-strong:#313131;--text:#c6c6c6;--muted:#747474;--gutter:#1b1b1b;--t-comment:#8b8b8b;--t-string:#a0a0a0;--t-numeric:#e8e8e8;--t-keyword:#ffffff;--t-function:#ffffff;--t-type:#b6b6b6;--t-variable:#c6c6c6;--t-tag:#d7d7d7;--t-attribute:#b6b6b6;--t-punctuation:#8b8b8b;--t-operator:#d7d7d7;--t-comment-style:italic} :root[data-syntax-theme='monochrome-light']{--bg:#ffffff;--surface:#e8e8e8;--surface-tint:#e8e8e8;--line:#e6e6e6;--line-strong:#e6e6e6;--text:#3e3e3e;--muted:#959595;--gutter:#e8e8e8;--t-comment:#757575;--t-string:#636363;--t-numeric:#1b1b1b;--t-keyword:#000000;--t-function:#000000;--t-type:#505050;--t-variable:#3e3e3e;--t-tag:#2c2c2c;--t-attribute:#505050;--t-punctuation:#757575;--t-operator:#2c2c2c;--t-comment-style:italic}</style> renders the CSS, no raw mustache.
  • But @once does not dedupe across component instances — two <CodeBlock>s produced two copies. So a naive emit ships the ~2.7KB theme stylesheet once per block, five copies on your five-block page.

So it is ready apart from deduplication, which wants either a stack (@push) or the component registering the style once per render. Left open deliberately rather than shipping a 5× stylesheet.

Suite green: 14,172 pass.

Sign in to comment on this issue.