Skip to content

fix(web_core): resolve DynamicValues inside function arg arrays#2016

Open
kokoro-ele wants to merge 5 commits into
a2ui-project:mainfrom
kokoro-ele:fix/web-core-resolve-dynamicvalue-arrays
Open

fix(web_core): resolve DynamicValues inside function arg arrays#2016
kokoro-ele wants to merge 5 commits into
a2ui-project:mainfrom
kokoro-ele:fix/web-core-resolve-dynamicvalue-arrays

Conversation

@kokoro-ele

Copy link
Copy Markdown
Contributor

Summary

Fixes DataContext.resolveDynamicValue() and resolveSignal() treating arrays as primitive literals, which prevented DynamicValue elements inside arrays (e.g., and/or values arrays) from being resolved.

Problem

When a function call like and/or receives an array-typed argument (values: [...]), any path bindings or nested function calls within that array were returned as-is without resolution. The and/or functions received raw {path: "..."} or {call: "..."} objects (always truthy) instead of actual resolved values, causing validation logic to silently pass.

Root Cause

The early-return guard in both resolveDynamicValue and resolveSignal conflated arrays with primitives:

// Before: arrays returned as-is
if (value === null || typeof value !== "object" || Array.isArray(value)) {
  return value as V;
}

Fix

Separate the array check from the primitive early-return and recursively resolve each element:

// Primitive literals only
if (value === null || typeof value !== "object") {
  return value as V;
}

// Arrays: recursively resolve each element
if (Array.isArray(value)) {
  return value.map(item => this.resolveDynamicValue(item)) as V;
}

Same fix applied to resolveSignal for reactive resolution.

Test Plan

Added unit tests covering:

  • Element-wise resolution of arrays containing path bindings
  • Reactive subscription for arrays containing path bindings
  • Nested function calls inside arrays (and/or values)

Closes #2015

🤖 Generated with Claude Code

@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 updates DataContext to support element-wise resolution and reactive subscription of arrays containing DynamicValues, along with corresponding unit tests. The feedback suggests optimizing both resolveDynamicValue and resolveSignal by checking if an array is completely static before recursively processing its elements, which avoids unnecessary allocations and signal creation for static arrays.

Comment thread renderers/web_core/src/v0_9/rendering/data-context.ts
Comment thread renderers/web_core/src/v0_9/rendering/data-context.ts
@kokoro-ele

Copy link
Copy Markdown
Contributor Author

@Varun-S10 Thanks for the review! Applied in 30ae9fb.

I extracted the static check into a shared private helper (containsDynamicValue) used by both resolveDynamicValue and resolveSignal, rather than duplicating the inline closure in each method. Fully static arrays now return as-is (no re-allocation) in resolveDynamicValue, and become a single static signal in resolveSignal, skipping the per-element signals + computed.

One trade-off worth noting: for arrays that do contain dynamic values, the pre-check adds an extra scan (and re-scans nested arrays during recursion). Since dynamic arrays in practice are small function args (e.g. and/or values) while static arrays can be large (e.g. dropdown options), optimizing the static path at this minor cost seemed like the right balance. Also added tests covering the no-reallocation fast path and nested dynamic elements.

Address review feedback: add a containsDynamicValue fast-path check so
static arrays are returned as-is in resolveDynamicValue and wrapped in a
single signal in resolveSignal, avoiding N element signals + computed.
@kokoro-ele
kokoro-ele force-pushed the fix/web-core-resolve-dynamicvalue-arrays branch from 30ae9fb to f9be169 Compare July 16, 2026 15:54
@github-actions github-actions Bot added the status: needs-triage auto-managed: https://github.com/a2ui-project/a2ui/blob/main/scripts/triage.mjs label Jul 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status: needs review status: needs-triage auto-managed: https://github.com/a2ui-project/a2ui/blob/main/scripts/triage.mjs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG]: resolveDynamicValue does not resolve DynamicValue elements inside arrays

2 participants