-
Notifications
You must be signed in to change notification settings - Fork 1.1k
fix: implement strict type coercion in DataContext #861
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,115 @@ | ||||||
| /* | ||||||
| * Copyright 2025 Google LLC | ||||||
| * | ||||||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||||||
| * you may not use this file except in compliance with the License. | ||||||
| * You may obtain a copy of the License at | ||||||
| * | ||||||
| * https://www.apache.org/licenses/LICENSE-2.0 | ||||||
| * | ||||||
| * Unless required by applicable law or agreed to in writing, software | ||||||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||||||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||||||
| * See the License for the specific language governing permissions and | ||||||
| * limitations under the License. | ||||||
| */ | ||||||
|
|
||||||
| /** | ||||||
| * Strict Type Coercion Utilities | ||||||
| * | ||||||
| * Implements the A2UI protocol's standard coercion rules to ensure | ||||||
| * consistent handling of null/undefined values and type conversions. | ||||||
| * | ||||||
| * Without central enforcement, component authors must manually handle | ||||||
| * these edge cases, leading to bugs like [object Object] appearing | ||||||
| * in text labels. | ||||||
| */ | ||||||
|
|
||||||
| /** | ||||||
| * Coerces any value to a string following A2UI protocol rules: | ||||||
| * - null/undefined → "" | ||||||
| * - objects → localized string representation (not "[object Object]") | ||||||
| * - other types → String(value) | ||||||
| */ | ||||||
| export function coerceToString(value: unknown): string { | ||||||
| if (value === null || value === undefined) { | ||||||
| return ""; | ||||||
| } | ||||||
| if (typeof value === "object") { | ||||||
| // Avoid "[object Object]" by using JSON.stringify for plain objects | ||||||
| // or calling toString for objects with custom implementations | ||||||
| if (Array.isArray(value)) { | ||||||
| return value.map(coerceToString).join(", "); | ||||||
| } | ||||||
| if (value instanceof Error) { | ||||||
| return value.message; | ||||||
| } | ||||||
| try { | ||||||
| return JSON.stringify(value); | ||||||
| } catch { | ||||||
| return String(value); | ||||||
| } | ||||||
| } | ||||||
| return String(value); | ||||||
| } | ||||||
|
|
||||||
| /** | ||||||
| * Coerces any value to a number following A2UI protocol rules: | ||||||
| * - null/undefined → 0 | ||||||
| * - strings → parsed number (NaN becomes 0) | ||||||
| * - booleans → 1 for true, 0 for false | ||||||
| * - other types → Number(value) | ||||||
| */ | ||||||
| export function coerceToNumber(value: unknown): number { | ||||||
| if (value === null || value === undefined) { | ||||||
| return 0; | ||||||
| } | ||||||
| if (typeof value === "string") { | ||||||
| const parsed = parseFloat(value); | ||||||
| return isNaN(parsed) ? 0 : parsed; | ||||||
| } | ||||||
| if (typeof value === "boolean") { | ||||||
| return value ? 1 : 0; | ||||||
| } | ||||||
| const result = Number(value); | ||||||
| return isNaN(result) ? 0 : result; | ||||||
| } | ||||||
|
|
||||||
| /** | ||||||
| * Coerces any value to a boolean following A2UI protocol rules: | ||||||
| * - "true" (case-insensitive) → true | ||||||
| * - non-zero numbers → true | ||||||
| * - null/undefined → false | ||||||
| * - other types → Boolean(value) | ||||||
| */ | ||||||
| export function coerceToBoolean(value: unknown): boolean { | ||||||
| if (value === null || value === undefined) { | ||||||
| return false; | ||||||
| } | ||||||
| if (typeof value === "string") { | ||||||
| return value.toLowerCase() === "true" || value !== ""; | ||||||
|
||||||
| return value.toLowerCase() === "true" || value !== ""; | |
| return value.toLowerCase() === "true"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This new file introduces important coercion logic but lacks unit tests to verify its correctness and prevent future regressions. The repository style guide requires tests for new code.
Please add a test file (e.g., coercion.test.ts) with unit tests for the new coercion functions. The tests should cover the cases mentioned in the PR description as well as other edge cases, such as:
coerceToString:null,undefined, objects with circular references.coerceToBoolean:"false", empty string, and other non-"true"strings.coerceToNumber:null,undefined, invalid strings.
References
- If there are code changes, code should have tests. (link)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
try...catchblock forJSON.stringifyis a good way to handle potential errors, but the fallback toString(value)can still result in"[object Object]"for objects with circular references. This contradicts the function's documentation and the PR's goal of eliminating"[object Object]".To make the behavior clearer, you could update the comment to acknowledge this edge case. If avoiding
"[object Object]"is a strict requirement, you might need to consider a more robust serialization approach that can handle circular references, though that might be out of scope for this change. For now, clarifying the comment would be a good improvement.