feat(web_core): add resolveAction and improve error handling#856
feat(web_core): add resolveAction and improve error handling#856gspencergoog merged 9 commits intogoogle:mainfrom
resolveAction and improve error handling#856Conversation
…updates - Update logical functions (`and`, `or`) to require the `values` array argument, removing deprecated individual arguments. - Update `formatDate` to require `format` parameter, utilizing `date-fns`. - Update expressions and math schemas with a preprocess layer for cleaner `null` layout to `undefined` coercion. - Update guards and data-context tests for expanded v0.9 safety checks.
gspencergoog
changed the title
gspencergoog
changed the title
resolveAction
- Added SurfaceModel.dispatchError to format and emit error events conforming to v0.9 generic error schema. - Updated DataContext.resolveDynamicValue and evaluateFunctionReactive to capture A2uiExpressionError & ZodError, dispatching to surface and returning fallbacks to avoid crashes. - Updated ComponentContext and test suites (data-context, basic_functions, function_execution) to align with return-instead-of-throw design. - All tests passing green.
|
/gemini review |
![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
The pull request primarily focuses on upgrading the core web rendering library to version 0.9.0, which includes significant updates to expression parsing and function validation. Key changes involve updating package-lock.json files across various renderers (Angular, Lit, Markdown) to reflect the new web_core version and its dependencies, notably adding date-fns and @preact/signals-core while removing rxjs. The CHANGELOG.md was updated to document changes such as logical functions (and, or) now requiring a values array, formatDate needing a format parameter, and improved validation for math and comparison expressions. Code changes include minor formatting, increased icon font size, and extensive updates to function schemas using z.preprocess for null to undefined coercion and .refine for stricter argument validation. Error handling for function calls has been enhanced to catch ZodError and re-throw as A2uiExpressionError, which is then dispatched to the surface model. The DataContext and ComponentModel were refactored to better integrate with the SurfaceModel and its error dispatching mechanism. A review comment highlighted an issue where a test case incorrectly expected an A2uiExpressionError for divide with an invalid number, when z.coerce.number returns NaN, suggesting the test should assert NaN instead. Another comment pointed out that the email validation regex is too basic and should be replaced with a more robust version for improved quality.
renderers/web_core/src/v0_9/basic_catalog/functions/basic_functions.test.ts
Show resolved
Hide resolved
renderers/web_core/src/v0_9/basic_catalog/functions/basic_functions.ts
Outdated
Show resolved
Hide resolved
| import { Action } from "../schema/common-types.js"; | ||
|
|
||
| /** An action wrapper that includes the origin surface ID. */ | ||
| export type SurfaceGroupAction = Action & { surfaceId: string }; |
There was a problem hiding this comment.
I have a few changes in #866 which address a similar problem in a slightly different way (adding a full A2uiClientActionSchema), though this is a strict improvement, so let's go with it and my PR can refine it if necessary.
There was a problem hiding this comment.
Sure, no problem. This felt like a "minimum" solution to me: it could probably be made better with a more formalized schema.
renderers/web_core/package.json
Outdated
| { | ||
| "name": "@a2ui/web_core", | ||
| "version": "0.8.5", | ||
| "version": "0.9.0", |
There was a problem hiding this comment.
Can we keep this at v0.8.5 for now, just in case @ditman and I need to push another v0.8 version of a renderer with fixes in web_core etc.
There was a problem hiding this comment.
Sure. I was thinking of bumping it with the 0.9 renderer implementation, but I don't have to.
| e.errors ?? e.issues, | ||
| ); | ||
| this.surface.dispatchError({ | ||
| code: "EXPRESSION_ERROR", |
There was a problem hiding this comment.
A couple of thoughts:
-
Should we add this to the client_to_server.json spec in v0.10? For v0.9 could it make sense to use the VALIDATION_FAILED error type which has some structure around surfaceId and path etc? No stress for this PR, just curious about direction.
-
Should we have some centralized mapping from error classes e.g. A2uiExpressionError to whatever dispatchError actually accepts, to reduce the boilerplate here?
Neither of these blocking - i'm thrilled to see error handling overall.
There was a problem hiding this comment.
I thought about adding it as a type in the client-to-server schema, but expressions are part of the basic catalog, not the main spec, so I wasn't sure if it belonged there.
A centralized mapping might make sense: at least it would give us a good place to add future mappings.
…tions.test.ts Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Updated the `EmailImplementation` to use a restricting regex requiring a 2-char TLD and standard valid characters. Increased the test coverage in `basic_functions.test.ts` for multiple email formats and fixed an unrelated issue in the `divide` test that was causing test failures due to strict type checking.
… design Updated the comment in `resolveAction` to explain that its one-level non-recursive behavior aligns with the schema spec, which requires values to be single `DynamicValue` types and restricts arbitrary nesting.
gspencergoog
changed the title
resolveActionresolveAction and improve error handling
2 participants
Changes in
renderers/web_coreHere are some changes to web_core to prepare for the 0.9 angular renderer.
Public API Changes
1.
DataContext(New Method)resolveAction(action: Action): anyActionobject and resolves its top-levelDynamicValueproperties (e.g., event context values) using the currentDataContext. This bridges the gap between static component action definitions and the actual data bound to them.2. Type-Safe Action Propagation
SurfaceModel:onActionstream anddispatchActionmethod are now strictly typed to use theActioninterface from the schema instead ofany.SurfaceGroupModel:onActionnow emits a strictly typedSurfaceGroupAction(Action & { surfaceId: string }). Spreading an action withsurfaceIdis now statically validated for type safety.3. Error Handling (Enhanced Specificity)
A2uiExpressionErrorinstead of generic errors or silent failures:DataContext: BothresolveDynamicValueandevaluateFunctionReactivetranslateZodErrorintoA2uiExpressionErrorwith detail scopes (issuesorerrors).Catalog:getFunction()now catchesZodErrorduring argument parsing and raisesA2uiExpressionError.RegexImplementation: Now throwsA2uiExpressionErroron invalid regex patterns rather than returningfalsewith a warning.Internal Refactorings & Improvements
1. Reactivity Upgrades (
DataContext)resolveSignal: Substantially refactored.computedargument bundles and isolatedeffectsubscriptions.2.
DataModelEdge CasesDataModel.setnow verifies that the base reference is populated (this.data = {}) before traversing branches, preventing crashes on absolute root paths.3. Testing Modernization
function_execution.spec.tsfrom old Node-style Node(done)callbacks to modern Promise-based resolutions.File Reorganization
FunctionInvoker: The type definition was moved fromsrc/v0_9/catalog/types.tsinto a standalone filesrc/v0_9/catalog/function_invoker.tsand re-exported viaindex.ts. This was done to avoid import circular references There are no changes to the declaration itself.