Skip to content

fix(cli): keep non-self-contained hook handlers out of body-only lowering (#1876)#1907

Merged
os-zhuang merged 1 commit into
mainfrom
fix/1876-hook-body-free-vars
Jun 15, 2026
Merged

fix(cli): keep non-self-contained hook handlers out of body-only lowering (#1876)#1907
os-zhuang merged 1 commit into
mainfrom
fix/1876-hook-body-free-vars

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Addresses the live half of #1876 (build↔runtime validation parity).

Discovery summary

#1876 lists two repros. I verified both before writing any code:

A parity sweep also confirmed schema-level parity holds end-to-end (both sides use ObjectStackDefinitionSchema), so the remaining divergence is semantic/callable-level — this PR.

The fix (repro #2)

A hook/action handler referencing a module-scope identifier (helper, import, top-level const) was lowered by objectstack build to a metadata-only body. That body ships without the referenced definition → ReferenceError at runtime. Build green, app won't boot.

extract-hook-body.ts now runs a conservative free-identifier analysis (detect-free-identifiers.ts, TS AST via the already-present ts-morphno new dependency):

  • free vars = identifiers referenced but bound neither by the function (params/locals, over-approximated) nor by the runtime (a generous global allow-list);
  • any hit refuses extraction, so lowerCallables falls back to bundling the real function (esbuild carries the closure) — no ReferenceError, no build break.

Why this is safe (the #1903 lesson, applied)

The failure mode of a build-time check is false positives breaking real builds. This analysis is biased so that can't happen:

  • over-approximate bindings (a name declared anywhere counts as bound) → only risks false negatives;
  • generous globals → only risks false negatives;
  • a false positive merely bundles a self-contained handler instead of inlining it — a size cost, never a correctness or build failure (and --strict-body is used nowhere in-repo).

Tests

  • 23-case analyzer battery: flagging (helper/const/method-shorthand/implicit-return/multi) + false-positive guards (member access, locals, globals, destructuring, loop/catch bindings, shorthand props, param defaults, named-fn recursion, rest params, …).
  • Extractor integration: throws on a module-scope helper, extracts a self-contained handler.
  • End-to-end lowerCallables: free-var handler stays registered (bundled) with body undefined + a warning naming the identifier; self-contained handler still body-lowers.
  • Full CLI suite 376 pass; tsc clean; example-crm / app-showcase / app-todo all build green (crm's self-contained callable stays body-only — no false positive).

🤖 Generated with Claude Code

…ring (#1876)

A hook/action handler referencing a module-scope identifier (helper, import,
top-level const) was lowered to a metadata-only `body`; that body ships without
the definition and throws `ReferenceError` at runtime — build green, app won't
boot (a build↔runtime parity gap, #1876).

`extractHookBody` now runs a conservative free-identifier analysis (TS AST via
the already-present `ts-morph`): free vars = names referenced but bound neither
by the function (params/locals) nor by the runtime (generous global allow-list).
Any hit refuses extraction, so `lowerCallables` falls back to BUNDLING the real
closure (esbuild carries it along) — no ReferenceError, no build break. Biased to
never over-report: a miss preserves today's behavior; a false positive only
bundles a self-contained handler (size cost, never a build/correctness failure).

Tests: 23-case analyzer battery (flagging + false-positive guards), extractor
integration (throws on free var, extracts self-contained), and an end-to-end
lowerCallables fallback assertion. All example app builds stay green.

Note: the other #1876 repro (legacy object/aggregate widgets) is already closed
on main by the ADR-0021 single-form cutover — verified.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@vercel

vercel Bot commented Jun 15, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
spec Ready Ready Preview, Comment Jun 15, 2026 9:54am

Request Review

@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling size/m labels Jun 15, 2026
@os-zhuang
os-zhuang merged commit 8c7e7e4 into main Jun 15, 2026
13 checks passed
@os-zhuang
os-zhuang deleted the fix/1876-hook-body-free-vars branch June 15, 2026 11:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant