ReviewOS

also looking at this

stacks/stx

fix(includes): preserve {⁠{ }} in @include partials that define their own signals (#1758)

#1759
Merged glennmichael123 wants to merge fix/1758-mustache-in-include-partials into main
3 files +76 -6
9 days ago

Fixes #1758.

Problem

reading a signal defined in an @include partial's own <script client> rendered empty server-side and never bound on the client. Pages were fine; partials/components were not.

includes.ts strips a partial's <script> (~L925) before running processExpressions on the body. processExpressions gates preservation on usesSignalsInScript — but that detects signals by scanning <script> blocks, which were just stripped. With no :if/:for/:show in the body to trip the other gate rules, the gate returned false, the was evaluated server-side (signal undefined → emptied), and the marker was consumed so the re-executed scoped script had nothing to bind.

This was the root cause behind the recent :text→ migration regressions (stacksjs/training#86, stacksjs/bench-review#67), which were worked around app-side by reverting those partials to :text.

Fix

  • processExpressions gains an optional { forceSignals } — when true, hasSignals is forced on.
  • includes.ts computes usesSignalsInScript(partialContent) on the full partial content (before the <script> strip) and passes it as forceSignals.

So a partial with a client signal script preserves exactly like a top-level page. The preservation is still guarded by the existing value === undefined && !expressionUsesOnlyContextVars(...) check, so:

  • server-var partials (<script server> → ) still evaluate server-side
  • signal-free partials (, no script) are not over-preserved

Tests

New test/includes/signal-partial-mustache.test.ts (src processDirectives, not Bun.build, to avoid the stale-dist realm): signal partial preserves, server partial evaluates to its value, signal-free partial not over-preserved. Full expressions + includes suites stay green (649 pass on the fix branch; 7755/7755 on the full suite locally).

Follow-up (not in this PR)

Partials with no local signals that rely entirely on a parent's scope (e.g. a dumb confirm dialog reading the parent's confirmTitle) are still not handled — they'd need the parent's signal status threaded into include processing. Those stay on :text for now.

🤖 Generated with Claude Code

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

Sign in to comment on this pull request.