fix(cli): keep non-self-contained hook handlers out of body-only lowering (#1876)#1907
Merged
Conversation
…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>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addresses the live half of #1876 (build↔runtime validation parity).
Discovery summary
#1876 lists two repros. I verified both before writing any code:
object/aggregatedashboard widgets pass build, runtime rejects) — already closed onmain. The ADR-0021 single-form cutover madeDashboardWidgetSchema.dataset/.valuesrequired (dashboard.zod.ts:156,160);objectstack build(ObjectStackDefinitionSchema.safeParse) and the runtime metadata plugin parse with the same schema (metadata/plugin.ts:505), and there's already a regression test (dashboard.test.ts:58). I confirmed empirically that a legacy widget is rejected at build. No code needed.ReferenceError) — live; fixed here.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 buildto a metadata-onlybody. That body ships without the referenced definition →ReferenceErrorat 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-morph— no new dependency):lowerCallablesfalls back to bundling the real function (esbuild carries the closure) — noReferenceError, 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:
--strict-bodyis used nowhere in-repo).Tests
lowerCallables: free-var handler stays registered (bundled) withbodyundefined + a warning naming the identifier; self-contained handler still body-lowers.tscclean; example-crm / app-showcase / app-todo all build green (crm's self-contained callable stays body-only — no false positive).🤖 Generated with Claude Code