refactor(lit): tighten A2uiChildRef type#1520
Conversation
… path
A2uiChildRef declared a third variant (`{type, ...}`) for inline
component definitions, but no schema produces this shape and
`ComponentContext` would reject it at runtime. The matching
`!childRef.type` check in `renderNode` was unreachable.
Tighten the type to `ComponentId | {id: ComponentId; basePath: string}`
and drop the dead branch. Object-variant fields are now required: the
GenericBinder always sets both, and the string variant covers the
bare-id case.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request simplifies the A2uiChildRef type and refactors the renderNode method in a2ui-lit-element.ts to use ComponentId and clean up path resolution. The reviewer suggested making basePath optional in the A2uiChildRef object type to prevent breaking changes for existing code that might omit it, since the runtime logic already handles its absence gracefully.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| * pairing an ID with an explicit data context path. | ||
| */ | ||
| type A2uiChildRef = string | {id?: string; basePath?: string; type?: string}; | ||
| type A2uiChildRef = ComponentId | {id: ComponentId; basePath: string}; |
There was a problem hiding this comment.
Making basePath optional in the object type prevents breaking changes for existing customer code that might be passing {id: 'foo'} (as mentioned in the PR description). Since the runtime code in renderNode already gracefully falls back to parentPath when basePath is omitted, making it optional keeps the type tight (preventing runtime errors from missing id or using type) while maintaining backwards compatibility.
| type A2uiChildRef = ComponentId | {id: ComponentId; basePath: string}; | |
| type A2uiChildRef = ComponentId | {id: ComponentId; basePath?: string}; |
There was a problem hiding this comment.
I do not believe the cognitive burden of loose types are worth preserving more lenient typechecking here. I believe very few/no one will be broken by this (see PR description). Even if someone is broken, it will be caught at compilation time and be straightforward to troubleshoot.
2 participants
See #1513.
A2uiChildRefis the type that describes a "child to render," used by the helper methodrenderNodeonA2uiLitElement.A2uiLitElementis the base class for Lit A2UI components.This PR narrows the type from
to the two shapes the protocol actually uses:
ComponentIdis just an alias forstringthat exists presumably to make the code a little more intuitive to read.It is technically possible for customer code to get broken (typecheck error) by this change if they took a dependency on specific cases of the previous type definition. Typechecking will fail if a custom component calls
this.renderNodewith an object literal of the shape{id: 'foo'}(renderNodehas fallback behavior to just take the component ID out and pretend it was called asrenderNode('foo')). Other object literals (e.g.{basePath: '/path'}) would result in a runtime error, so those cases are not broken by this PR. For whatever's worth, I didn't find any internal override ofrenderNodeor reference toA2uiChildRefthat would be broken by this change.