Skip to content

refactor(compiler): replace hardcoded registry with framework manifest#994

Merged
viniciusdacal merged 1 commit intofeat/cross-file-reactivityfrom
refactor/framework-manifest
Mar 7, 2026
Merged

refactor(compiler): replace hardcoded registry with framework manifest#994
viniciusdacal merged 1 commit intofeat/cross-file-reactivityfrom
refactor/framework-manifest

Conversation

@viniciusdacal
Copy link
Copy Markdown
Contributor

Summary

  • Introduces ReactivityManifest type system for describing reactivity shapes of exports
  • Creates packages/ui/reactivity.json — the framework manifest for @vertz/ui APIs (query, form, createLoader, useContext, signal)
  • Creates reactivity-manifest.ts with loading utilities (JSON string[]Set<string> conversion)
  • Adds CompileOptions.manifests parameter to pass pre-loaded manifests through the compile pipeline
  • Refactors buildImportAliasMap in ReactivityAnalyzer to consume manifests, with fallback to hardcoded registry

Closes #989

Key design decisions

  • Backward compatible: All existing callers that don't provide manifests still work via the hardcoded registry fallback
  • Manifest-first resolution: resolveSignalApiConfig() checks manifestConfigs before falling back to getSignalApiConfig()
  • Loaded types: JSON manifests use string[] for properties; loaded manifests use Set<string> for O(1) lookups
  • Version forward compatibility: Unsupported manifest versions fall back to treating all exports as unknown

Test plan

  • 10 new manifest loading tests (version handling, Set conversion, framework manifest loading)
  • 1 new integration test proving manifest overrides hardcoded registry (custom signal properties)
  • All 436 ui-compiler tests pass (including all existing signal API tests — backward compat verified)
  • Typecheck clean
  • Lint/format clean
  • 67 turborepo tasks pass

🤖 Generated with Claude Code

#989]

Introduces a ReactivityManifest system that replaces the hardcoded
SIGNAL_API_REGISTRY for classifying signal-returning APIs.

Changes:
- Add ReactivityManifest types (ReactivityManifest, LoadedReactivityManifest,
  ExportReactivityInfo, ReactivityShape) to types.ts
- Create packages/ui/reactivity.json with framework manifest for query(),
  form(), createLoader(), useContext(), signal()
- Create reactivity-manifest.ts with loadManifestFromJson() and
  loadFrameworkManifest() utilities (JSON arrays → Set<string> conversion)
- Add CompileOptions.manifests parameter for passing pre-loaded manifests
- Refactor buildImportAliasMap to use manifests when provided, falling
  back to hardcoded registry for backward compatibility
- Add resolveSignalApiConfig helper that checks manifest first, then
  hardcoded registry

The hardcoded registry is kept as fallback for callers that don't
provide manifests (e.g., existing tests). Layer 2b will wire the
Bun plugin to auto-load manifests.

10 new manifest tests + 1 new integration test. All 436 tests pass.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@viniciusdacal
Copy link
Copy Markdown
Contributor Author

✅ Adversarial Review — josh (DX)

Design compliance: ✅

Matches Section 2.2.5 (framework manifest) and Section 2.3 (Layer 2a) exactly.

Technical review: ✅

Cross-checked:

  1. Manifest vs registry parity — Verified every property in reactivity.json matches SIGNAL_API_REGISTRY:

    • query: signal=[data,loading,error,revalidating], plain=[refetch,revalidate,dispose] ✅
    • form: signal=[submitting,dirty,valid], plain=[action,method,onSubmit,reset,setFieldError,submit], field=[value,error,dirty,touched] ✅
    • createLoader: signal=[data,loading,error], plain=[refetch] ✅
    • useContext: reactive-source ✅
    • signal: added (not in old registry, additive only) ✅
  2. Fallback path — When no manifests provided, buildImportAliasMap only processes @vertz/ui via hardcoded registry. All 425 existing tests pass without manifests. ✅

  3. Override proof — Integration test uses custom manifest with customProp as signal property and data as plain. Proves manifest overrides registry (customProp.value appears, data.value does not). ✅

  4. Set conversiontoSet() handles both Set<string> and string[] inputs via instanceof Set check. ✅

  5. Version safety — Unsupported versions fall back to unknown for all exports. ✅

  6. Type safetyLoadedReactivityManifest vs ReactivityManifest properly separates JSON (string[]) from runtime (Set). No any or as. ✅

  7. Path resolutionresolve(__dirname, '../../ui/reactivity.json') works in monorepo structure. Cache prevents repeated I/O. ✅

Quality gates: ✅

  • 436 tests pass, 67 turborepo tasks, typecheck + lint clean

Verdict: APPROVE — ready to merge.

@viniciusdacal viniciusdacal merged commit 6a02ba1 into feat/cross-file-reactivity Mar 7, 2026
3 checks passed
@viniciusdacal viniciusdacal deleted the refactor/framework-manifest branch March 7, 2026 16:39
@viniciusdacal
Copy link
Copy Markdown
Contributor Author

Adversarial Review — josh (DX lead)

Verdict: REQUEST CHANGES

Summary

The manifest types, JSON file, loading utilities, and wiring through CompileOptionsReactivityAnalyzerbuildImportAliasMap are well-structured. The reactivity.json matches SIGNAL_API_REGISTRY exactly. Tests pass. The integration test correctly proves manifest-driven compilation overrides the hardcoded registry.

However, there are issues ranging from "blocks ship" to "must address before next layer."


Blocking Issues

1. Missing exports from public API — loadManifestFromJson and loadFrameworkManifest are unreachable

packages/ui-compiler/src/index.ts does not export loadManifestFromJson, loadFrameworkManifest, or any of the manifest types (ReactivityManifest, LoadedReactivityManifest, LoadedExportReactivityInfo, LoadedReactivityShape).

CompileOptions.manifests expects Record<string, LoadedReactivityManifest>, but no external consumer can construct a LoadedReactivityManifest because:

  • loadManifestFromJson() is not exported
  • loadFrameworkManifest() is not exported
  • The LoadedReactivityManifest type itself is not exported (so consumers can't even type-annotate)

This means the manifests option on CompileOptions is publicly visible but practically unusable from @vertz/ui-server or any other consumer. The Bun plugin cannot wire the framework manifest without reaching into internals.

Fix: Export loadManifestFromJson, loadFrameworkManifest, and the manifest types from index.ts. These are the public API for Layer 2a.

2. Missing version warning — design doc requires it, code silently swallows it

Design doc Section 2.2.1:

When loading a manifest with an unknown version, the compiler falls back to treating all exports as unknown with a warning: [vertz:reactivity] Manifest version ${v} not supported (expected 1). Exports treated as unknown.

The code silently falls back to unknown with no console output. The test is named "warns and falls back to unknown" but only asserts the fallback, not the warning. This is a spec violation — a consumer shipping a v2 manifest against a v1 compiler would get silent degradation with no diagnostic trail.

Fix: Add console.warn(\[vertz:reactivity] Manifest version ${json.version} not supported (expected ${SUPPORTED_VERSION}). Exports treated as unknown.`)in the unsupported version branch. Update the test to assert the warning is emitted (e.g., spy onconsole.warn`).

3. loadFrameworkManifest is not wired into the compile pipeline — the registry is not replaced

The design doc Layer 2a (Section 2.3) says: "Replace hardcoded SIGNAL_API_REGISTRY with framework manifest." The PR title says "replace hardcoded registry with framework manifest." But in practice, the framework manifest is never loaded or passed to compile() automatically. loadFrameworkManifest() exists as a utility, but the Bun plugin (@vertz/ui-server) hasn't been updated and there's no auto-loading in compiler.ts.

The hardcoded SIGNAL_API_REGISTRY is still the only path used in production. The manifest system is tested in isolation but never activated. This means the PR title/description is misleading — this is "add manifest infrastructure alongside hardcoded registry," not "replace hardcoded registry."

Either:

  • (a) Wire loadFrameworkManifest() into compile() as the default (when no manifests are provided, auto-load the framework manifest for @vertz/ui). This actually replaces the registry.
  • (b) Or rename the PR to accurately reflect this is infrastructure-only, and create a follow-up issue to wire the framework manifest into the Bun plugin. But then Layer 2a is not done.

I'd prefer (a) — it's a few lines in compiler.ts and it completes the stated goal.


Non-Blocking Issues (address before merge or as immediate follow-up)

4. Module-level cache with no invalidation for loadFrameworkManifest

The cachedFrameworkManifest is a module-level singleton with no reset mechanism. This makes testing harder and prevents cache invalidation if the manifest file changes during a long-running process. Consider exporting a _resetManifestCache() function (underscore-prefixed = test-only convention) or using a WeakRef-based approach.

Not blocking because the manifest is a static JSON file, but this will bite someone.

5. as LoadedReactivityShape cast in convertReactivityShape

Line 66 of reactivity-manifest.ts uses return shape as LoadedReactivityShape. The project convention discourages unnecessary as casts. Since non-signal-api shapes (static, signal, reactive-source, unknown) are structurally identical between ReactivityShape and LoadedReactivityShape, this could be replaced with explicit construction per variant:

if (shape.type === 'static') return { type: 'static' };
if (shape.type === 'signal') return { type: 'signal' };
if (shape.type === 'reactive-source') return { type: 'reactive-source' };
return { type: 'unknown' };

Verbose but cast-free and exhaustive.

6. resolveSignalApiConfig fallback is partially dead code when manifests are in play

In buildImportAliasMap, when a manifest is found for a module, only exports listed in the manifest are added to signalApiAliases. So resolveSignalApiConfig's fallback to getSignalApiConfig() is only reachable when no manifest is provided for @vertz/ui. This is semantically correct but creates a subtle coupling: the fallback looks like it could fire for manifest-classified imports, but it never will because the gating happens earlier. A comment clarifying this would help.

7. Design doc mentions reactiveProps?: string[] on ExportReactivityInfo — types omit it

The design doc defines reactiveProps?: string[] on ExportReactivityInfo (deferred, for component prop reactivity). The PR types omit it entirely. Since it's deferred, that's acceptable, but a // DEFERRED: reactiveProps?: string[] comment on the type would document the intent and prevent someone re-designing it.

What's Good

  • reactivity.json is byte-for-byte faithful to SIGNAL_API_REGISTRY. No behavioral drift.
  • The toSet() function correctly handles the "already a Set" case (idempotent loading).
  • The integration test is the strongest part: it constructs a manifest with customProp as signal and data as plain (the opposite of the hardcoded registry), then verifies the compiler follows the manifest, not the registry. That proves override semantics.
  • The fallback logic in buildImportAliasMap correctly preserves the hardcoded registry when no manifest is provided, so existing behavior is safe.
  • __dirname resolution works from both src/ and dist/ since both are one level deep under packages/ui-compiler/.

Verdict: REQUEST CHANGES — Issues #1 (missing exports), #2 (missing warning), and #3 (manifest not wired into pipeline) must be addressed. Issues #4-#7 can be follow-ups.

viniciusdacal added a commit that referenced this pull request Mar 7, 2026
* plan: cross-file reactivity analysis design doc (#987)

Design doc for fixing callback/thunk classification (Layer 1) and
cross-file reactivity manifest system (Layer 2). Includes adversarial
reviews from josh (DX), pm (scope), and ben (technical feasibility).

All 10 blocking review items resolved. CTO approved 2026-03-07.

Co-authored-by: vertz-dev-front[bot] <2828126+vertz-dev-front[bot]@users.noreply.github.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>

* fix(compiler): skip function defs in computed classification (#992)

* fix(compiler): skip function defs in computed classification [#988]

Arrow functions and function expressions are stable references — they
should never be wrapped in computed(). The initializer's top-level AST
node kind (ArrowFunction/FunctionExpression) is checked, and such
consts are excluded from the computed classification loop.

Updates 2 existing tests and adds 4 new tests: function expression,
arrow thunk capturing let signal, IIFE (still computed), and a
regression guard for value expressions.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* test(compiler): add e2e tests for callback classification [#988]

Adds 3 integration tests that verify the full compiler pipeline
(analyzer → transformers → output) correctly handles function
definitions:
- Arrow function referencing signal → NOT wrapped in computed()
- Arrow function referencing query signal property → NOT computed()
- Function expression referencing signal → NOT computed()

Each test also verifies that value expressions in the same component
are still correctly wrapped in computed() (regression guard).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

---------

Co-authored-by: vertz-dev-front[bot] <2828126+vertz-dev-front[bot]@users.noreply.github.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>

* refactor(compiler): replace hardcoded registry with framework manifest [#989] (#994)

Introduces a ReactivityManifest system that replaces the hardcoded
SIGNAL_API_REGISTRY for classifying signal-returning APIs.

Changes:
- Add ReactivityManifest types (ReactivityManifest, LoadedReactivityManifest,
  ExportReactivityInfo, ReactivityShape) to types.ts
- Create packages/ui/reactivity.json with framework manifest for query(),
  form(), createLoader(), useContext(), signal()
- Create reactivity-manifest.ts with loadManifestFromJson() and
  loadFrameworkManifest() utilities (JSON arrays → Set<string> conversion)
- Add CompileOptions.manifests parameter for passing pre-loaded manifests
- Refactor buildImportAliasMap to use manifests when provided, falling
  back to hardcoded registry for backward compatibility
- Add resolveSignalApiConfig helper that checks manifest first, then
  hardcoded registry

The hardcoded registry is kept as fallback for callers that don't
provide manifests (e.g., existing tests). Layer 2b will wire the
Bun plugin to auto-load manifests.

10 new manifest tests + 1 new integration test. All 436 tests pass.

Co-authored-by: vertz-dev-front[bot] <2828126+vertz-dev-front[bot]@users.noreply.github.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>

* fix(compiler): unwrap type wrappers in isFunctionDef check [#988] (#996)

The isFunctionDef check only inspected the direct initializer node kind,
missing arrow functions wrapped in ParenthesizedExpression, AsExpression,
SatisfiesExpression, or TypeAssertion. Adds unwrapTypeWrappers() helper
that peels through these wrappers before checking for ArrowFunction or
FunctionExpression.

Found by adversarial review on PR #992.

Co-authored-by: vertz-dev-front[bot] <2828126+vertz-dev-front[bot]@users.noreply.github.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>

* fix(compiler): address manifest review issues (#997)

* fix(compiler): address manifest review issues [#989]

- Export manifest types and functions from public API (index.ts)
- Add console.warn for unsupported manifest versions per design doc 2.2.1
- Auto-load framework manifest for @vertz/ui when no explicit manifest provided
- Remove dead code (hardcoded registry fallback in buildImportAliasMap)
- Add integration test verifying auto-load behavior

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix(compiler): use require.resolve for framework manifest path [#989]

- Replace __dirname-based path resolution with require.resolve('@vertz/ui/reactivity.json')
  so the path survives bundling and works in published packages
- Add reactivity.json to @vertz/ui files and exports map
- Remove dead hardcoded registry fallback (getSignalApiConfig) from resolveSignalApiConfig
- Remove unused node:path import

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

---------

Co-authored-by: vertz-dev-front[bot] <2828126+vertz-dev-front[bot]@users.noreply.github.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>

---------

Co-authored-by: vertz-dev-front[bot] <2828126+vertz-dev-front[bot]@users.noreply.github.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant