a2ui: @geajs/a2ui — A2UI protocol renderer (18-component catalog, e2e contact-form acceptance)#80
a2ui: @geajs/a2ui — A2UI protocol renderer (18-component catalog, e2e contact-form acceptance)#80puskuruk wants to merge 13 commits into
Conversation
The test harness needs a Node module hook: `tsx` transpiles JSX but does not run the Gea compiler, so an uncompiled Component mounts to nothing. The hook applies geaPlugin().transform to .tsx and resolves virtual:gea-compiler-runtime. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…atString, formatDate) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ution Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Local containers omit {props.children}: the compiler's reactive-text marker
renders as a literal "0" when no children thunk is passed. gea-ui containers
bind the slot to '' for the same reason.
Imports use gea-ui per-component subpaths rather than the barrel, which
eagerly loads NumberInput and fails Node's ESM named-export check via
@internationalized/number.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…copes Uses keyedList's array-path form rather than the getter form: the getter branch reconciles without change records, so reverse()/sort() fall into the dirty-scan path and never move DOM nodes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
DateTimeInput wires its DOM event with `input=`, Gea's delegated-event attribute; `onInput=` would compile to an inert DOM attribute. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Also exports the runtime types a2ui consumes from @geajs/core/compiler-runtime (Disposer, Entry, ItemObservable, KeyedListConfig) — reachable at runtime but previously untyped for external callers — and declares Change.itemDirty, which keyed-list reads but no type declared. Both are type-only; @geajs/core's 471 tests are unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe PR adds ChangesA2UI renderer
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Server
participant dispatch
participant SurfaceRegistry
participant Store
participant instantiateNode
participant Transport
Server->>dispatch: Send surface or component envelope
dispatch->>SurfaceRegistry: Create or retrieve surface
dispatch->>Store: Apply data-model updates
dispatch->>instantiateNode: Mount component definitions
instantiateNode->>Store: Read bound values and write user input
instantiateNode->>Transport: Send resolved event action
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
packages/a2ui/package.jsonOops! Something went wrong! :( ESLint: 10.3.0 TypeError: Error while loading rule 'no-irregular-whitespace': sourceCode.getAllComments is not a function Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (2)
packages/a2ui/tests/catalog-complete.test.ts (1)
75-79: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert the bound URLs for video and audio.
These checks only verify that elements exist, so regressions that drop
srcbinding would pass despite the test’s stated purpose.Proposed test assertions
const video = build([{ id: 'root', component: 'Video', url: '/v.mp4' }]).host - assert.ok(video.querySelector('video.a2ui-video')) + assert.equal(video.querySelector('video.a2ui-video')?.getAttribute('src'), '/v.mp4') const audio = build([{ id: 'root', component: 'AudioPlayer', url: '/a.mp3' }]).host - assert.ok(audio.querySelector('audio.a2ui-audio')) + assert.equal(audio.querySelector('audio.a2ui-audio')?.getAttribute('src'), '/a.mp3')🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/a2ui/tests/catalog-complete.test.ts` around lines 75 - 79, Extend the video and audio assertions in the catalog-complete test to verify their bound URLs, not just element presence. Using the queried elements from the Video and AudioPlayer cases, assert that their `src` attributes resolve to `/v.mp4` and `/a.mp3` respectively.packages/a2ui/src/instantiate.ts (1)
95-95: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider replacing
as nevercasts with safer type assertions.Two
as nevercasts bypass type checking entirely:mount(entry.component as never, ...)on line 95 andobs: null as neveron line 181. If the gea-ui component type orEntry.obsshape changes, these casts would silently suppress type errors. Consider using a more targeted cast (e.g.,as unknown as ExpectedType) or a typed adapter function.Also applies to: 181-181
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/a2ui/src/instantiate.ts` at line 95, Replace the `as never` casts in the mount call and `obs` initialization with targeted type assertions or a typed adapter matching the expected component and observer types. Update the relevant types around `mount`, `entry.component`, and `Entry.obs` so incompatibilities remain type-checked without bypassing the compiler.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/a2ui/package.json`:
- Line 3: Reset the package version in package.json from 0.1.0 to the
new-package baseline 0.0.0, allowing the changeset curly-donkeys-invent.md to
manage the intended version bump.
In `@packages/a2ui/src/catalog.ts`:
- Around line 23-31: Add action to the STRUCTURAL_KEYS set in resolveProps so
definition.action is excluded from generated component prop thunks while
remaining available to injectAction in instantiate.ts.
In `@packages/a2ui/src/dispatcher.ts`:
- Around line 84-85: Guard the deleteSurface handling branch before accessing
its fields: verify envelope.deleteSurface is present, and throw a clear
descriptive error for malformed or unknown envelopes when it is absent. Update
the logic around the registry.delete call and preserve normal deletion using
deleteSurface.surfaceId.
In `@packages/a2ui/src/functions.ts`:
- Around line 20-24: Update the regex() function to safely handle malformed or
unsafe pattern values: wrap RegExp construction and testing in try/catch,
returning false when pattern compilation or evaluation throws, while preserving
the existing string type checks.
In `@packages/a2ui/src/instantiate.ts`:
- Around line 39-47: writeFor currently captures resolved write parts using the
initial scope, causing edits to target stale rows after reordering. Change the
returned callback to recompute resolveWriteParts from definition.value.path and
the current scope at write time, ensuring patchEntry’s updated row scope is
honored before calling writePointer.
In `@packages/a2ui/src/surface.ts`:
- Around line 51-53: Update SurfaceRegistry.delete to retrieve the surface
before removing it, call its disposer’s dispose() method, and remove or clear
the associated host element’s rendered DOM as needed. Preserve safe behavior
when the surface ID is absent, and ensure cleanup occurs before or alongside
deleting the map entry.
In `@packages/a2ui/tests/contact-form.test.ts`:
- Around line 137-138: In the children-slot assertion in the contact-form test,
replace the broad /0/ text-content check with a precise validation that no
standalone "0" text node remains in the rendered host, while allowing legitimate
values such as prices or counts containing zero. Inspect the host’s text nodes
or DOM structure and retain the existing failure message.
In `@packages/a2ui/tests/gea-loader.ts`:
- Around line 33-35: Fix Prettier formatting in the test loader: make the
pathToFileURL(resolve(...)) expression a single line and compact the related
ternary chain referenced around the additional lines, preserving behavior and
satisfying the prettier/prettier ESLint rule.
---
Nitpick comments:
In `@packages/a2ui/src/instantiate.ts`:
- Line 95: Replace the `as never` casts in the mount call and `obs`
initialization with targeted type assertions or a typed adapter matching the
expected component and observer types. Update the relevant types around `mount`,
`entry.component`, and `Entry.obs` so incompatibilities remain type-checked
without bypassing the compiler.
In `@packages/a2ui/tests/catalog-complete.test.ts`:
- Around line 75-79: Extend the video and audio assertions in the
catalog-complete test to verify their bound URLs, not just element presence.
Using the queried elements from the Video and AudioPlayer cases, assert that
their `src` attributes resolve to `/v.mp4` and `/a.mp3` respectively.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: da72ed54-7496-49b1-8d74-9422dfae9c95
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (43)
.changeset/curly-donkeys-invent.mdpackages/a2ui/README.mdpackages/a2ui/package.jsonpackages/a2ui/src/actions.tspackages/a2ui/src/binding.tspackages/a2ui/src/catalog.tspackages/a2ui/src/catalog/basic.tspackages/a2ui/src/components/audio-player.tsxpackages/a2ui/src/components/column.tsxpackages/a2ui/src/components/date-time-input.tsxpackages/a2ui/src/components/icon.tsxpackages/a2ui/src/components/image.tsxpackages/a2ui/src/components/list.tsxpackages/a2ui/src/components/row.tsxpackages/a2ui/src/components/text.tsxpackages/a2ui/src/components/video.tsxpackages/a2ui/src/dispatcher.tspackages/a2ui/src/functions.tspackages/a2ui/src/index.tspackages/a2ui/src/instantiate.tspackages/a2ui/src/pointer.tspackages/a2ui/src/scope.tspackages/a2ui/src/surface.tspackages/a2ui/src/types.tspackages/a2ui/tests/actions.test.tspackages/a2ui/tests/binding.test.tspackages/a2ui/tests/catalog-complete.test.tspackages/a2ui/tests/catalog.test.tspackages/a2ui/tests/contact-form.test.tspackages/a2ui/tests/functions.test.tspackages/a2ui/tests/gea-loader.tspackages/a2ui/tests/instantiate.test.tspackages/a2ui/tests/pointer.test.tspackages/a2ui/tests/preload.tspackages/a2ui/tests/scope.test.tspackages/a2ui/tests/smoke.test.tspackages/a2ui/tests/surface.test.tspackages/a2ui/tests/template-list.test.tspackages/a2ui/tests/types.test.tspackages/a2ui/tsconfig.jsonpackages/a2ui/tsdown.config.tspackages/gea/src/compiler-runtime.tspackages/gea/src/store.ts
| @@ -0,0 +1,39 @@ | |||
| { | |||
| "name": "@geajs/a2ui", | |||
| "version": "0.1.0", | |||
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Version should be managed by changesets, not manually set.
The coding guideline states: "Never manually edit version numbers in package.json — let changesets own them." For a new package, the convention is to start at "0.0.0" so the changeset (.changeset/curly-donkeys-invent.md) can bump it to the intended version. Having "0.1.0" pre-set may cause the changeset to no-op or overshoot.
As per coding guidelines: **/package.json: Never manually edit version numbers in package.json — let changesets own them.
🔧 Proposed fix
- "version": "0.1.0",
+ "version": "0.0.0",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "version": "0.1.0", | |
| "version": "0.0.0", |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/a2ui/package.json` at line 3, Reset the package version in
package.json from 0.1.0 to the new-package baseline 0.0.0, allowing the
changeset curly-donkeys-invent.md to manage the intended version bump.
Source: Coding guidelines
| const STRUCTURAL_KEYS = new Set(['id', 'component', 'child', 'children']) | ||
|
|
||
| /** Every non-structural property is a DynamicValue and becomes a lazy thunk. */ | ||
| export function resolveProps(def: ComponentDefinition, ctx: BindingContext): PropThunks { | ||
| const props: PropThunks = {} | ||
| for (const [key, value] of Object.entries(def)) { | ||
| if (STRUCTURAL_KEYS.has(key)) continue | ||
| props[key] = toThunk(value as never, ctx) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Add action to STRUCTURAL_KEYS to prevent leaking the action definition into component props.
injectAction (in instantiate.ts) reads definition.action directly from the ComponentDefinition, but resolveProps does not skip action — it only skips id, component, child, and children. This means every component with an action gets a props.action thunk returning the raw action definition object, which no component consumes and which could conflict with future component props.
🛡️ Proposed fix
-const STRUCTURAL_KEYS = new Set(['id', 'component', 'child', 'children'])
+const STRUCTURAL_KEYS = new Set(['id', 'component', 'child', 'children', 'action'])📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const STRUCTURAL_KEYS = new Set(['id', 'component', 'child', 'children']) | |
| /** Every non-structural property is a DynamicValue and becomes a lazy thunk. */ | |
| export function resolveProps(def: ComponentDefinition, ctx: BindingContext): PropThunks { | |
| const props: PropThunks = {} | |
| for (const [key, value] of Object.entries(def)) { | |
| if (STRUCTURAL_KEYS.has(key)) continue | |
| props[key] = toThunk(value as never, ctx) | |
| } | |
| const STRUCTURAL_KEYS = new Set(['id', 'component', 'child', 'children', 'action']) | |
| /** Every non-structural property is a DynamicValue and becomes a lazy thunk. */ | |
| export function resolveProps(def: ComponentDefinition, ctx: BindingContext): PropThunks { | |
| const props: PropThunks = {} | |
| for (const [key, value] of Object.entries(def)) { | |
| if (STRUCTURAL_KEYS.has(key)) continue | |
| props[key] = toThunk(value as never, ctx) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/a2ui/src/catalog.ts` around lines 23 - 31, Add action to the
STRUCTURAL_KEYS set in resolveProps so definition.action is excluded from
generated component prop thunks while remaining available to injectAction in
instantiate.ts.
| // deleteSurface teardown is out of scope for the happy path. | ||
| registry.delete(envelope.deleteSurface.surfaceId) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Guard the deleteSurface fall-through branch.
The else branch assumes envelope.deleteSurface exists without checking. If a malformed or unknown envelope arrives at runtime (bypassing TypeScript's discriminated union), envelope.deleteSurface is undefined and .surfaceId throws a confusing TypeError: Cannot read properties of undefined. A guard with a clear error message would make debugging significantly easier.
🛡️ Proposed fix
- // deleteSurface teardown is out of scope for the happy path.
- registry.delete(envelope.deleteSurface.surfaceId)
+ if ('deleteSurface' in envelope) {
+ // deleteSurface teardown is out of scope for the happy path.
+ registry.delete(envelope.deleteSurface.surfaceId)
+ return
+ }
+ throw new Error(`A2UI: unknown envelope type: ${JSON.stringify(Object.keys(envelope))}`)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // deleteSurface teardown is out of scope for the happy path. | |
| registry.delete(envelope.deleteSurface.surfaceId) | |
| if ('deleteSurface' in envelope) { | |
| // deleteSurface teardown is out of scope for the happy path. | |
| registry.delete(envelope.deleteSurface.surfaceId) | |
| return | |
| } | |
| throw new Error(`A2UI: unknown envelope type: ${JSON.stringify(Object.keys(envelope))}`) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/a2ui/src/dispatcher.ts` around lines 84 - 85, Guard the
deleteSurface handling branch before accessing its fields: verify
envelope.deleteSurface is present, and throw a clear descriptive error for
malformed or unknown envelopes when it is absent. Update the logic around the
registry.delete call and preserve normal deletion using deleteSurface.surfaceId.
| regex(args) { | ||
| const { value, pattern } = args | ||
| if (typeof value !== 'string' || typeof pattern !== 'string') return false | ||
| return new RegExp(pattern).test(value) | ||
| }, |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for where `regex` function args are constructed — check if pattern can trace to user input.
rg -n -C3 "'regex'" packages/a2ui/src/ --type ts
rg -n -C3 'pattern' packages/a2ui/src/ --type ts | head -40Repository: dashersw/gea
Length of output: 658
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== Files mentioning A2UI protocol / schema / regex ==\n'
rg -n -S --type ts --type tsx --type json --type yml --type yaml --type md \
'regex\s*\(|pattern\b|A2UI|functions\.ts|Function' packages/a2ui . \
| head -300
printf '\n== Likely protocol/type files ==\n'
fd -a -t f '.*(schema|protocol|types|function|functions).*' packages/a2ui . | head -200
printf '\n== Map packages/a2ui/src files ==\n'
find packages/a2ui/src -maxdepth 2 -type f | sort | sed -n '1,200p'Repository: dashersw/gea
Length of output: 233
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== packages/a2ui file tree ==\n'
find packages/a2ui -maxdepth 3 -type f | sort | sed -n '1,200p'
printf '\n== regex / pattern references in packages/a2ui ==\n'
rg -n -S --type ts --type json --type yml --type yaml --type md \
'regex\s*\(|\bpattern\b|A2UI|function[s]?' packages/a2ui \
| sed -n '1,250p'
printf '\n== protocol/schema candidates ==\n'
rg -n -S --type ts --type json --type yml --type yaml --type md \
'\b(schema|protocol|validation|rules|config)\b' packages/a2ui \
| sed -n '1,250p'Repository: dashersw/gea
Length of output: 1906
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== Outline: packages/a2ui/src/types.ts ==\n'
ast-grep outline packages/a2ui/src/types.ts --view expanded || true
printf '\n== Outline: packages/a2ui/src/functions.ts ==\n'
ast-grep outline packages/a2ui/src/functions.ts --view expanded || true
printf '\n== Outline: packages/a2ui/src/dispatcher.ts ==\n'
ast-grep outline packages/a2ui/src/dispatcher.ts --view expanded || true
printf '\n== Outline: packages/a2ui/src/instantiate.ts ==\n'
ast-grep outline packages/a2ui/src/instantiate.ts --view expanded || true
printf '\n== Outline: packages/a2ui/src/binding.ts ==\n'
ast-grep outline packages/a2ui/src/binding.ts --view expanded || true
printf '\n== Relevant snippets from types.ts ==\n'
cat -n packages/a2ui/src/types.ts | sed -n '1,260p'
printf '\n== Relevant snippets from functions.test.ts ==\n'
cat -n packages/a2ui/tests/functions.test.ts | sed -n '1,260p'
printf '\n== Search for regex helper usage and pattern field construction ==\n'
rg -n -S --type ts --type tsx --type json --type md \
'regex\s*\(|\bpattern\b|functions\[[' packages/a2ui/src packages/a2ui/tests packages/a2ui/README.md \
| sed -n '1,260p'Repository: dashersw/gea
Length of output: 10525
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== packages/a2ui/src/functions.ts ==\n'
cat -n packages/a2ui/src/functions.ts | sed -n '1,220p'
printf '\n== packages/a2ui/src/binding.ts ==\n'
cat -n packages/a2ui/src/binding.ts | sed -n '1,220p'
printf '\n== packages/a2ui/src/instantiate.ts ==\n'
cat -n packages/a2ui/src/instantiate.ts | sed -n '1,220p'
printf '\n== packages/a2ui/README.md ==\n'
cat -n packages/a2ui/README.md | sed -n '1,260p'
printf '\n== Search for function calls in docs/tests ==\n'
rg -n -S --type ts --type md 'functionCall|call:\s*["'\'']regex["'\'']|regex\(' packages/a2ui | sed -n '1,240p'Repository: dashersw/gea
Length of output: 16371
Guard regex() against invalid patterns. new RegExp(pattern) can throw on malformed input, and data-bound pattern values can also create ReDoS exposure. Wrap construction in try/catch and return false for invalid patterns.
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 22-22: Do not use variable for regular expressions
Context: new RegExp(pattern)
Note: [CWE-1333] Inefficient Regular Expression Complexity. Security best practice.
(regexp-non-literal-typescript)
[warning] 22-22: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(pattern)
Note: [CWE-1333] Inefficient Regular Expression Complexity
(regexp-from-variable)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/a2ui/src/functions.ts` around lines 20 - 24, Update the regex()
function to safely handle malformed or unsafe pattern values: wrap RegExp
construction and testing in try/catch, returning false when pattern compilation
or evaluation throws, while preserving the existing string type checks.
Source: Linters/SAST tools
| function writeFor( | ||
| definition: ComponentDefinition, | ||
| ictx: InstantiateContext, | ||
| scope: Scope, | ||
| ): WriteFn | null { | ||
| if (!isDataBinding(definition.value)) return null | ||
| const parts = resolveWriteParts(definition.value.path, scope) | ||
| return (next: unknown) => writePointer(ictx.store, parts, next) | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether resolveWriteParts returns concrete index-based parts or key-based/scope-deferred parts.
# Map scope.ts to find resolveWriteParts implementation
ast-grep outline packages/a2ui/src/scope.ts --items all --type function --match 'resolveWriteParts'
# Read the resolveWriteParts implementation
rg -n -A 20 'export function resolveWriteParts' packages/a2ui/src/scope.ts
# Also check writePointer to see if it can handle non-concrete parts
rg -n -A 15 'export function writePointer' packages/a2ui/src/pointer.tsRepository: dashersw/gea
Length of output: 1098
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect instantiate/write plumbing and reorder handling.
ast-grep outline packages/a2ui/src/instantiate.ts --view expanded
printf '\n--- patchEntry references ---\n'
rg -n -A 40 -B 10 'patchEntry|writeFor|resolveWriteParts|WriteFn' packages/a2ui/src/instantiate.ts packages/a2ui/src/scope.ts packages/a2ui/src/pointer.tsRepository: dashersw/gea
Length of output: 18032
Recompute write paths after row reorders. writeFor resolves relative paths once, so the closure keeps the original scope.index; when patchEntry updates the row scope on reorder, edits can still land on the old item. packages/a2ui/src/instantiate.ts:39-47
🧰 Tools
🪛 ESLint
[error] 39-43: Replace ⏎··definition:·ComponentDefinition,⏎··ictx:·InstantiateContext,⏎··scope:·Scope,⏎ with definition:·ComponentDefinition,·ictx:·InstantiateContext,·scope:·Scope
(prettier/prettier)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/a2ui/src/instantiate.ts` around lines 39 - 47, writeFor currently
captures resolved write parts using the initial scope, causing edits to target
stale rows after reordering. Change the returned callback to recompute
resolveWriteParts from definition.value.path and the current scope at write
time, ensuring patchEntry’s updated row scope is honored before calling
writePointer.
| delete(surfaceId: string): void { | ||
| this.surfaces.delete(surfaceId) | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
SurfaceRegistry.delete leaks disposer and DOM resources.
delete removes the surface from the map but never calls surface.disposer.dispose(), leaving component instances, event listeners, and reactive subscriptions alive. The host element also retains rendered DOM nodes. While dispatcher.ts line 84 documents this as "out of scope for the happy path," SurfaceRegistry.delete is a public method callable directly. In a long-running app that creates and deletes surfaces, this accumulates memory and orphaned DOM.
🔧 Proposed fix
delete(surfaceId: string): void {
+ const surface = this.surfaces.get(surfaceId)
+ if (surface) {
+ surface.disposer.dispose()
+ surface.rootHostElement.replaceChildren()
+ }
this.surfaces.delete(surfaceId)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| delete(surfaceId: string): void { | |
| this.surfaces.delete(surfaceId) | |
| } | |
| delete(surfaceId: string): void { | |
| const surface = this.surfaces.get(surfaceId) | |
| if (surface) { | |
| surface.disposer.dispose() | |
| surface.rootHostElement.replaceChildren() | |
| } | |
| this.surfaces.delete(surfaceId) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/a2ui/src/surface.ts` around lines 51 - 53, Update
SurfaceRegistry.delete to retrieve the surface before removing it, call its
disposer’s dispose() method, and remove or clear the associated host element’s
rendered DOM as needed. Preserve safe behavior when the surface ID is absent,
and ensure cleanup occurs before or alongside deleting the map entry.
| // The compiler's children-slot placeholder is a literal "0" text node. | ||
| assert.doesNotMatch(host.textContent ?? '', /0/, 'no compiler slot markers leaked into the DOM') |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Tighten the slot-marker assertion to avoid false positives.
The regex /0/ matches any literal "0" in textContent, not just the compiler's children-slot placeholder. If a future component renders text containing "0" (e.g., a price or count), this test will break with a misleading message. A more precise check — e.g., asserting no standalone "0" text node or checking host.querySelectorAll('*') for stray text nodes — would be more robust.
💚 Proposed fix
- // The compiler's children-slot placeholder is a literal "0" text node.
- assert.doesNotMatch(host.textContent ?? '', /0/, 'no compiler slot markers leaked into the DOM')
+ // The compiler's children-slot placeholder is a literal "0" text node.
+ // Check for standalone "0" text nodes, not any "0" substring in text content.
+ const strayMarkers = Array.from(host.querySelectorAll('*'))
+ .flatMap(el => Array.from(el.childNodes))
+ .filter(node => node.nodeType === 3 && node.textContent === '0')
+ assert.equal(strayMarkers.length, 0, 'no compiler slot markers leaked into the DOM')🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/a2ui/tests/contact-form.test.ts` around lines 137 - 138, In the
children-slot assertion in the contact-form test, replace the broad /0/
text-content check with a precise validation that no standalone "0" text node
remains in the rendered host, while allowing legitimate values such as prices or
counts containing zero. Inspect the host’s text nodes or DOM structure and
retain the existing failure message.
| 'virtual:gea-compiler-runtime': pathToFileURL( | ||
| resolve(HERE, '../../gea/src/compiler-runtime.ts'), | ||
| ).href, |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix prettier formatting errors flagged by ESLint.
ESLint reports two prettier/prettier errors: the pathToFileURL call should be single-line, and the ternary chain should be compact. If CI runs ESLint with these as errors, the pipeline will fail.
🎨 Prettier fixes
const VIRTUAL_MODULES: Record<string, string> = {
- 'virtual:gea-compiler-runtime': pathToFileURL(
- resolve(HERE, '../../gea/src/compiler-runtime.ts'),
- ).href,
+ 'virtual:gea-compiler-runtime': pathToFileURL(resolve(HERE, '../../gea/src/compiler-runtime.ts')).href,
}- const compiled =
- transformed == null
- ? source
- : typeof transformed === 'string'
- ? transformed
- : transformed.code
+ const compiled = transformed == null ? source : typeof transformed === 'string' ? transformed : transformed.codeAlso applies to: 51-56
🧰 Tools
🪛 ESLint
[error] 33-35: Replace ⏎····resolve(HERE,·'../../gea/src/compiler-runtime.ts'),⏎·· with resolve(HERE,·'../../gea/src/compiler-runtime.ts')
(prettier/prettier)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/a2ui/tests/gea-loader.ts` around lines 33 - 35, Fix Prettier
formatting in the test loader: make the pathToFileURL(resolve(...)) expression a
single line and compact the related ternary chain referenced around the
additional lines, preserving behavior and satisfying the prettier/prettier
ESLint rule.
Source: Linters/SAST tools
1 participant
Summary
New
@geajs/a2uipackage implementing the A2UI protocol, built in 13 layered commits:DynamicValue→ lazy prop thunks with function-call resolution; registered function library (required,regex,email,formatString,formatDate)mount(); surface lifecycle, root mount gate, envelope dispatcherChildListbacked bykeyedListwith mutable row scopesEnds with an end-to-end canonical contact-form acceptance test; 24 test files total. Two minimal supporting touches in gea core (
compiler-runtime.ts,store.ts).Note: pushed from a fork because the repo's active ruleset (
main, id 14516181) has an emptyincludelist withexclude: [refs/heads/main], which appliesrequired_linear_historyto every non-main branch — and since main's history contains PR merge commits, creating any branch on the upstream repo is rejected (GH013). The ruleset likely intendedinclude: [~DEFAULT_BRANCH].🤖 Generated with Claude Code
https://claude.ai/code/session_01NUHaxNQQEn7AvUv1BJf7ND
Summary by CodeRabbit