Skip to content

Node layer for web_core + React#2077

Draft
andrewkolos wants to merge 4 commits into
a2ui-project:mainfrom
andrewkolos:node-layer-experimental
Draft

Node layer for web_core + React#2077
andrewkolos wants to merge 4 commits into
a2ui-project:mainfrom
andrewkolos:node-layer-experimental

Conversation

@andrewkolos

Copy link
Copy Markdown
Collaborator

Not ready for review. Submitting this early as a draft so it's easier to reference by implementers of other renderers/adapters, I'm okay with this sitting here as a draft as I plan on getting it to ready-for-review state within days.

(ai-generated; rewrite coming later)

What it is

The node layer compiles a surface's flat component map plus data model into a live tree of ComponentNodes whose resolved properties update reactively. It ships beside the existing surface, with no changes to current renderers.

The approach

Nodes are long-lived, each with its own props signal, and each dynamic property is subscribed to the specific data path it reads. A data write notifies only the paths that changed, so only the node(s) reading them recompute and emit, and only their views re-render. The tree is never rebuilt wholesale; work is proportional to what changed, not to surface size.

What's here

  • web_core: ComponentNode + NodeResolver (v0_9/nodes/) with a conformance suite (node-resolver.test.ts) covering identity, notification granularity, placeholders, templates, and late action resolution.
  • react: A2uiNodeSurface, reusing the existing catalog views unchanged, each node subscribed to its own props signal (node-surface.test.tsx).
  • The React shell sample opts in via ?nodes, used for an end-to-end run (list → booking form → typed input → submit → confirmation) alongside the legacy renderer for parity.

… in types

Catalog and SurfaceModel gain a function-kind type parameter defaulting
to FunctionImplementation, so existing code is unchanged. A schema-only
catalog types its functions as FunctionApi (signatures without code),
matching the Python SDK's two-parameter Catalog. The catalog invoker
reports a schema-only function as an expression error instead of
failing on a missing execute method.
NodeResolver turns a surface's flat component map into a live tree of
resolved ComponentNodes exposed through a reactive rootNode. Child
references resolve to node objects, ChildList templates spawn one node
per array item with per-position reuse, missing components render as
placeholder nodes that are upgraded in place, and disposal follows
parent-scoped ownership. Malformed payloads degrade safely: cyclic
references and unknown catalog types render as placeholders with
dispatched errors. Construction requires a catalog typed with
FunctionImplementation, so resolving a tree without executable
functions is a compile error.

A node's props signal emits only when its own resolved properties
change; child-internal changes stay on the child's signal. Action
context resolves at dispatch time. The test suite ports
test_node_graph.py from the Python SDK and adds coverage for late
action resolution, shared-child disposal, template key stability,
cycles, and emission granularity.
A2uiNodeSurface renders a surface from a NodeResolver instead of the
per-component id recursion in A2uiSurface. One resolver per surface
owns binding and tree maintenance; the resolver is constructed inside
the store subscribe callback so renders discarded by concurrent
features cannot leak it. Each node's view subscribes only to that
node's props signal, so a data change re-renders exactly the affected
component.

Existing view code is reused unchanged: createComponentImplementation
now also exposes the unwrapped view, node props are converted back to
the string and {id, basePath} shapes views expect, and buildChild maps
those refs to live nodes. Refs the node layer cannot identify fall
back to DeferredChild, so the shipped basic catalog renders as-is.
Tests cover render isolation, template stability, StrictMode, action
dispatch, and input components writing scoped values through
synthesized setters.
Default rendering is unchanged; adding ?nodes to the URL renders every
surface through A2uiNodeSurface so the two paths can be compared in the
same app.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces an experimental React surface renderer (A2uiNodeSurface) driven by a new node layer (NodeResolver and ComponentNode) that resolves component references and templates into a live tree of nodes for fine-grained re-renders. It also updates the Catalog interface to distinguish schema-only catalogs from those with executable function implementations, and adds comprehensive unit tests. The review feedback highlights several key improvement opportunities, including restricting object traversal in toViewValue to plain objects to prevent breaking types like Date or Map, unwrapping ZodEffects in schema parsing to support refined schemas, removing a redundant synchronous onChange call to avoid duplicate React state updates, using public Zod APIs instead of internal _def properties, and cleaning up empty sets in pendingParents to prevent minor memory leaks.

Comment on lines +79 to +98
function toViewValue(parent: ComponentNode, value: unknown, index: ChildIndex): unknown {
if (value instanceof ComponentNode) {
registerChild(index, value);
if (value.dataPath !== parent.dataPath) {
return {id: value.componentId, basePath: value.dataPath};
}
return value.componentId;
}
if (Array.isArray(value)) {
return value.map(item => toViewValue(parent, item, index));
}
if (value && typeof value === 'object') {
const result: Record<string, unknown> = {};
for (const [key, inner] of Object.entries(value)) {
result[key] = toViewValue(parent, inner, index);
}
return result;
}
return value;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The toViewValue function currently converts any non-null object into a plain object by traversing its entries. This will break non-plain objects like Date, Map, Set, or custom class instances by stripping their prototype and converting them into plain/empty objects. We should restrict this traversal to plain objects only.

function isPlainObject(value: unknown): value is Record<string, unknown> {
  if (!value || typeof value !== 'object' || Array.isArray(value) || value instanceof ComponentNode) {
    return false;
  }
  const proto = Object.getPrototypeOf(value);
  return proto === Object.prototype || proto === null;
}

function toViewValue(parent: ComponentNode, value: unknown, index: ChildIndex): unknown {
  if (value instanceof ComponentNode) {
    registerChild(index, value);
    if (value.dataPath !== parent.dataPath) {
      return {id: value.componentId, basePath: value.dataPath};
    }
    return value.componentId;
  }
  if (Array.isArray(value)) {
    return value.map(item => toViewValue(parent, item, index));
  }
  if (isPlainObject(value)) {
    const result: Record<string, unknown> = {};
    for (const [key, inner] of Object.entries(value)) {
      result[key] = toViewValue(parent, inner, index);
    }
    return result;
  }
  return value;
}

Comment on lines +139 to +141
if (typeName === 'ZodOptional' || typeName === 'ZodNullable' || typeName === 'ZodDefault') {
current = current._def.innerType;
} else {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The unwrap helper does not unwrap ZodEffects (which represents refined or transformed schemas via .refine() or .transform()). If a component schema uses refinement or transformation at the top level, its child references will be silently ignored. We should unwrap ZodEffects by extracting the inner schema from _def.schema.

    if (typeName === 'ZodOptional' || typeName === 'ZodNullable' || typeName === 'ZodDefault') {
      current = current._def.innerType;
    } else if (typeName === 'ZodEffects') {
      current = current._def.schema;
    } else {

Comment on lines +184 to +189
box.resolver = resolver;
onChange();
const stopEffect = effect(() => {
getValue(resolver.rootNode);
onChange();
});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The synchronous onChange() call right before creating the effect is redundant because effect runs its callback synchronously on creation, which will trigger onChange() anyway. Removing this redundant call avoids triggering duplicate synchronous state updates in React.

      box.resolver = resolver;
      const stopEffect = effect(() => {
        getValue(resolver.rootNode);
        onChange();
      });

const list = new Set<string>();
const nested = new Map<string, ReadonlySet<string>>();

const shape = unwrapped.schema._def.shape() as Record<string, z.ZodTypeAny>;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Accessing internal properties like _def.shape() is discouraged in Zod as they can change between minor versions. It is safer and cleaner to cast the unwrapped schema to z.AnyZodObject and use the public .shape property.

Suggested change
const shape = unwrapped.schema._def.shape() as Record<string, z.ZodTypeAny>;
const shape = (unwrapped.schema as z.AnyZodObject).shape;

Comment on lines +473 to +475
if (!stillWaiting) {
this.pendingParents.get(child.componentId)?.delete(record.node);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

When a parent node stops waiting for a placeholder component, the node is deleted from the corresponding set in pendingParents. However, if the set becomes empty, the key is not removed from the map, leading to a minor memory leak/bloat over time. We should clean up empty sets from pendingParents.

          if (!stillWaiting) {
            const waiting = this.pendingParents.get(child.componentId);
            if (waiting) {
              waiting.delete(record.node);
              if (waiting.size === 0) {
                this.pendingParents.delete(child.componentId);
              }
            }
          }

Comment on lines +514 to +516
for (const waiting of this.pendingParents.values()) {
waiting.delete(node);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

When disposing a node, we remove it from all sets in pendingParents. If any of these sets become empty, we should delete the corresponding key from the map to prevent memory bloat.

    for (const [componentId, waiting] of this.pendingParents.entries()) {
      waiting.delete(node);
      if (waiting.size === 0) {
        this.pendingParents.delete(componentId);
      }
    }

@jacobsimionato jacobsimionato left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great! Neat way to prototype it!

}

get isPlaceholder(): boolean {
return this.type === PLACEHOLDER_TYPE;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does this mean again? Could you add a doc comment about it?

* come from the resolver's cache; untouched arrays keep their identity), so
* shallow comparison is exact rather than heuristic.
*/
setProps(next: TProps): void {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we don't want application code to be able to call this. Can we provide a readonly public interface for Node, so that the NodeResolver has write access, but the rest of the application code can only read values and update the data model via the bindings in the node, rather than the node props directly?

}
}

function serializeValue(value: unknown): unknown {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this need to be public?

* unchanged props keep reference identity across rebuilds. Child
* `ComponentNode`s and action closures compare by identity.
*/
function stabilize(prev: unknown, next: unknown): unknown {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this be private?

return next;
}

function isPlainObject(value: unknown): value is Record<string, unknown> {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this be private too?

* Use in place of a bare string id so the node layer can resolve the child.
*/
export function componentReference(): z.ZodTypeAny {
return ComponentIdSchema.describe(`${SINGLE_REF_MARKER}|${ComponentIdSchema.description ?? ''}`);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already do a hack like this in our schemas here:

.describe('REF:common_types.json#/$defs/ComponentId|The unique identifier for a component.');

We needed it so that when we convert an in-memory Zod catalog to a wire-format catalog, we can replace the subschemas for DynamicString etc with references to common_types.json.

Seems like you needed basically the same thing here - to be able to identify specific special subschemas to understand the meaning of different strings etc.

Could you use the same magic references, e.g. centralize some definition in common-types?

BTW I wrote a proposal at go/a2ui-catalogs-without-json-schema to design our own schema format that would make this less hacky. I'm not a fan of this description hack, but I don't know a way around it without abandoning JSON schema!

/**
* One resolved component instance in the rendered tree.
*
* A node's `props` hold fully resolved values: primitives for dynamic values,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add some way to update the value of two-way bound variables from the node? E.g. for ChoicePicker, it should be possible to somehow update the bound "value" property of the node. I'm not sure the best way to do this. Some ideas:

A. props['value'] returns a BoundValue object which exposes get and set. If we do this, then I think we should probably always return a BoundValue for properties that are DynamicValues etc in the schema, even if they happen to be populated with a static value at runtime. That way, component implementations can always access the value in the same way e.g. props['value'].get() and props['value'].set('new value').

B. Expose setters as additional keys in props e.g. props['setValue'](updatedValue). I think we basically did this in the generic binder.

C. Or expose some other parallel map of setters that is separate to props. E.g. boundValues['value']('updated value').

Vercel did something between A and C I think: https://json-render.dev/docs/registry#component-props

* without `render`'s own binder. Absent on binderless implementations,
* which the node surface falls back to rendering through `render`.
*/
view?: React.FC<ReactA2uiComponentProps<any>>;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense for this to just accept a node and buildChild as the parameter? Then we can have extra sugar in createComponentImplementation that exposes props, in a typesafe way?

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.

2 participants