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

vercelBot commented Jun 15, 2026

Copy link
Copy Markdown

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

ProjectDeploymentActionsUpdated (UTC)
specReadyReadyPreview, CommentJun 15, 2026 9:54am

Request Review

@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tests tooling size/m labels Jun 15, 2026
@os-zhuang
os-zhuang merged commit 8c7e7e4 into mainJun 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 freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@os-zhuang