ReviewOS

stacks/stx

Sidebar's client script never parses in a served dashboard, though stx's interpolator is correct

#1894
Closed glennmichael123 opened this 14 days ago · 3 comments
14 days ago

packages/components · src/ui/sidebar/Sidebar.stx

Symptom

The Sidebar controller never runs. Collapse, width persistence, shell-selector pane offset and active-row sync are all dead in every app that renders <Sidebar>, because the emitted <script client> block is a syntax error and the browser discards the whole thing.

Cause

Lines 108 to 117 interpolate props into the client script with {⁠{ }}, which substitutes the raw value with no quoting:

const collapsedProp = {{ collapsed }}
const collapsibleProp = {{ collapsible }}
const collapseModeProp = {{ collapseMode }}
const widthProp = {{ width }}
const minWidthProp = {{ minWidth }}
const persistKeyProp = {{ persistKey }}
const shellSelectorProp = {{ shellSelector }}
const widthVarProp = {{ widthVar }}
const collapsedClassProp = {{ collapsedClass }}
const followSystemAppearanceProp = {{ followSystemAppearance }}

That is fine for the booleans and numbers and wrong for all five strings. Served HTML from a real dashboard, with persistKey, shellSelector and theme="macos" set:

const collapsedProp = false;                        // ok
const widthProp = 250;                              // ok
const collapseModeProp = hidden;                    // ReferenceError
const persistKeyProp = stacks-dashboard-sidebar;    // parses as subtraction, ReferenceError
const shellSelectorProp = [data-stx-content];       // ReferenceError
const widthVarProp = --stx-sidebar-width;           // SyntaxError
const collapsedClassProp = ;                        // SyntaxError

const collapsedClassProp = ; is unconditional: collapsedClass defaults to '', so every <Sidebar> emits it whether or not the consumer passes anything. One syntax error kills the entire block, so none of the ten assignments or the controller below them ever execute.

Reproduction

Render any <Sidebar :sections="…" theme="macos">, then look at the served HTML:

curl -s http://localhost:3002/ | grep -oE "const [A-Za-z]+Prop = ;"
# const collapsedClassProp = ;

The DevTools console shows the parse failure and data-stx-sidebar never gains its controller behaviour, though the server-rendered rows and their hrefs work, which is why this hides well.

Suggested fix

JSON-encode the string props at the interpolation site, e.g. const collapsedClassProp = {⁠{ JSON.stringify(collapsedClass) }}, or quote them. Worth checking the other components in the package for the same pattern, since nothing about it is Sidebar-specific.

Context

Found while fixing stacksjs/stacks#2279, where the dashboard's desktop sidebar rendered twelve empty section groups. That turned out to be a separate, unrelated prop-casing mistake on our side (stacksjs/stacks#2287). This one survived that fix.

14 days ago

I cannot reproduce this on current main — both paths emit quoted values

Not disputing the served HTML in the report; flagging that whatever produces it is not in main as it stands, so the fix may already be in and the useful next step is a re-check against a current build.

interpolateScriptExpressions JSON-encodes {⁠{ }} inside a script body, and has since well before 0.2.170 — d4b6a76a5b (4 Aug) only added < escaping on top of an existing JSON.stringify(value).

Rendering <Sidebar persistKey="dash-sidebar" shellSelector="[data-stx-content]" collapsedClass="" width="250">:

var collapsedClassProp = "";          // the assignment the report shows as `= ;`
var followSystemAppearanceProp = true;
// the rest arrive through the bundle map, e.g. persistKeyProp = __stxBundle_901ab08b776f["persistKeyProp"]

And the non-bundled path, since Sidebar's own client script imports ./route-selection and so gets bundled — a component with no imports, same interpolation shape:

const labelProp = "my-dash";
const countProp = "3";
const selectorProp = "[data-stx-content]";

Both quoted, including the empty string that the report shows emitting = ; unconditionally.

What that suggests

The reported output has raw unquoted values, which is not what either path produces here — so it looks like an older installed @stacksjs/stx, or a @stacksjs/components built against one. Worth confirming what version the dashboard actually resolves; stx doctor reports the dist/symlink/staleness chain, and #1896 is a reminder that an installed build can differ from the clone it appears to point at.

If it still reproduces on main, the missing piece is the serve path specifically — I rendered through renderTemplate, not through the dev server on :3002 — and a curl of the served HTML plus the resolved stx version would pin it immediately.

Leaving open rather than closing, since the served evidence is concrete and my not reproducing it is not the same as it being fixed.

Related, and worth having regardless: stx typecheck now parses {⁠{ }} inside <script client> (efc13f412a). It previously treated those blocks as a syntax error, which meant it was blind on 48 of the 95 components in this package rather than strict — so tooling would not have caught this class either.

14 days ago

I cannot make this fail, on current main or on the published 0.2.170

Pinned against regression in cd33fbe4d9 regardless. What I ran, all emitting "stacks-dashboard-sidebar" and "" correctly:

#configurationresult
1<Sidebar> itself, real component treevar collapsedClassProp = "";
2minimal component, no imports (unbundled path)quoted
3same, with an import (bundled path)quoted
4bundle that fails to resolve — fallback pathquoted
5server-dynamic :sections="…" bindingquoted
6name resolved from the page, not the componentquoted
7the same page served through stx devquoted

Plus all of it again against v0.2.170 in a detached worktree, since that is the version #1896 established you were resolving. Same results.

What the report does establish, and it is the useful part

Booleans and numbers were fine while every string was broken. That is the signature of raw String(value) substitution — their text happens to be valid JavaScript, which is exactly why the failure looks selective and reads like a problem with those particular props. So whatever produced your HTML substituted with HTML rules where JS rules were needed.

That has happened before, and knowing so may place it faster than I can from here: #1757 was the same class through a different door. processExpressions HTML-escapes, so a value containing a quote emitted &quot; into the JS and the bundle died with Unexpected token '&' — breaking every component that seeds a client signal from a server prop. The fix routed the component path through interpolateScriptExpressions, and the docblock at that call site in utils.ts is a full account of it.

So the question is which pass won in your app, not whether the encoding rule exists.

What would settle it in one command

bun pm ls | grep -E '@stacksjs/(stx|components)'

Given #1896 — where the published tarball genuinely differed from the clone your node_modules appeared to point at — a version mismatch between an installed @stacksjs/components and the stx resolving it is the most likely remaining explanation. stx doctor reports that chain.

What landed

4 tests pinning the property at the boundary rather than the plumbing behind it, so they hold whichever pass does the substitution: string props quoted, no bare = ;, no unresolved {⁠{ }} left in the block, and a value carrying a quote or a </script> neither ending the literal nor closing the tag.

Closing as not reproducible on current main. Please reopen with the resolved versions if it still happens — the served HTML is concrete and I would rather chase it with your environment than guess at mine.

14 days ago

Correcting this: my diagnosis was wrong. The symptom is real, the cause I gave is not.

I filed this saying Sidebar.stx interpolates string props into <script client> unquoted and therefore must quote them itself. That is not what is happening, and a fix along those lines would have been wrong.

What I verified since

stx's documented rule is exactly what the component assumes. packages/stx/src/process.ts:

{⁠{ expr }} emits JSON.stringify(value) — safe JS (strings quoted, objects/arrays as JSON literals).

It is implemented and tested. packages/stx/test/components/client-script-interpolation.test.ts already contains a case matching my scenario almost exactly, layout plus kebab-case props plus empty-string defaults:

expect(js).toContain('const persistKeyProp = "dashboard-sidebar"')
expect(js).toContain('const shellSelectorProp = "[data-stx-content]"')

Six pass at 0.2.170.

The published dist behaves correctly too. Driving processDirectives from the installed @stacksjs/stx@0.2.170 against a synthetic component with the same shape gives properly quoted output, including const collapsedClassProp = ""; for an empty default. Adding a relative import to the client script, which is what Sidebar.stx has, does not change that: it still emits var persistKeyProp = "dashboard-sidebar";.

So: not the component, and not the interpolator.

What is still true

A live Stacks dashboard on that same version serves this today:

const collapseModeProp = hidden;
const persistKeyProp = stacks-dashboard-sidebar;
const shellSelectorProp = [data-stx-content];
const widthVarProp = --stx-sidebar-width;
const collapsedClassProp = ;

const collapsedClassProp = ; is a hard syntax error, so the whole block fails to parse and the Sidebar controller never runs. Rows and their hrefs work because they are server-rendered; collapse, width persistence and active-row sync do not.

Where it must actually be

Whatever differs between the dashboard's serving path and processDirectives called directly. Two candidates I have not yet separated:

  1. the bun-plugin-stx dev pipeline rather than processDirectives
  2. <Sidebar> resolved as a plugin component out of node_modules rather than a partial in partialsDir

One lead worth checking first, because it bit us badly elsewhere today: version skew through the global install cache. This repo has @stacksjs/stx@0.2.170 in node_modules, while ~/.bun/install/cache now holds 0.2.171. A bare specifier resolved from a file outside the project silently prefers the cache, which is how a probe of mine ended up loading 0.2.171 and dying on a missing stx-router. If the dev server resolves stx that way, it is not running the version whose tests pass.

Retitling to match the evidence. Apologies for the noise on the original.

Sign in to comment on this issue.