Apply suggestions from code review

Co-authored-by: Leon Senft <leonsenft@users.noreply.github.com>
angular · mmalerba · Jul 1, 2025 · Jun 27, 2025 · Jun 27, 2025 · Jul 1, 2025
commit 94d44f2308e4783cd9780803b864d823330b176a
diff --git a/packages/forms/experimental/src/api/standard_schema.ts b/packages/forms/experimental/src/api/standard_schema.ts
@@ -1,15 +1,24 @@
-import {computed, resource} from '@angular/core';
+import {computed, resource, ɵisPromise} from '@angular/core';
 import {validateAsync} from './async';
 import {define} from './data';
 import {validateTree} from './logic';
 import {StandardSchemaV1} from './standard_schema_types';
-import {Field, FieldPath} from './types';
+import {Field, FieldPath, FormTreeError} from './types';
+
+/**
+ * A validation error produced by running a standard schema validator.
+ */
+interface StandardSchemaFormTreeError extends FormTreeError {
+  issue: StandardSchemaV1.Issue;
+}
 
 /**
  * Validates a field using a `StandardSchemaV1` compatible validator (e.g. a zod validator).
  *
+ * See https://github.com/standard-schema/standard-schema for more about standard schema.
+ *
  * @param path The `FieldPath` to the field to validate.
- * @param schema The standard schema copatible validator to use for validation.
+ * @param schema The standard schema compatible validator to use for validation.
  * @template T The type of the field being validated.
  */
 export function validateStandardSchema<T>(
@@ -28,7 +37,7 @@ export function validateStandardSchema<T>(
   validateTree(path, ({state, fieldOf}) => {
     // Skip sync validation if the result is a Promise.
     const result = state.data(schemaResult)!();
-    if (isPromise(result)) {
+    if (ɵisPromise(result)) {
       return [];
     }
     return result.issues?.map((issue) => standardIssueToFormTreeError(fieldOf(path), issue)) ?? [];
@@ -38,7 +47,7 @@ export function validateStandardSchema<T>(
     params: ({state}) => {
       // Skip async validation if the result is *not* a Promise.
       const result = state.data(schemaResult)!();
-      return isPromise(result) ? result : undefined;
+      return ɵisPromise(result) ? result : undefined;
     },
     factory: (params) => {
       return resource({
@@ -59,7 +68,10 @@ export function validateStandardSchema<T>(
  * @param issue The `StandardSchemaV1.Issue` to convert.
  * @returns A `FormTreeError` representing the issue.
  */
-export function standardIssueToFormTreeError(field: Field<unknown>, issue: StandardSchemaV1.Issue) {
+export function standardIssueToFormTreeError(
+  field: Field<unknown>,
+  issue: StandardSchemaV1.Issue,
+): StandardSchemaFormTreeError {
   let target = field as Field<Record<PropertyKey, unknown>>;
   for (const pathPart of issue.path ?? []) {
     const pathKey = typeof pathPart === 'object' ? pathPart.key : pathPart;
@@ -71,15 +83,3 @@ export function standardIssueToFormTreeError(field: Field<unknown>, issue: Stand
     issue,
   };
 }
-
-/**
- * Checks if a value is a Promise.
- * Use this function rather than `instanceof Promise` because the value could be a thenable rather
- * than a proper `Promise` (e.g. from nodejs)
- *
- * @param value The value to check.
- * @returns `true` if the value is a Promise, `false` otherwise.
- */
-export function isPromise(value: Object): value is Promise<unknown> {
-  return typeof (value as Promise<unknown>).then === 'function';
-}
diff --git a/packages/forms/experimental/src/api/standard_schema_types.ts b/packages/forms/experimental/src/api/standard_schema_types.ts
@@ -1,3 +1,6 @@
+// DO NOT EDIT: This file is copy/pasted from https://github.com/standard-schema/standard-schema
+// TODO: should we keep this copy/pasted version of depend on it from npm?
+
 /** The Standard Schema interface. */
 export interface StandardSchemaV1<Input = unknown, Output = Input> {
   /** The Standard Schema properties. */

diff --git a/packages/forms/experimental/test/node/BUILD.bazel b/packages/forms/experimental/test/node/BUILD.bazel
@@ -4,8 +4,6 @@ ts_library(
     name = "test_lib",
     testonly = True,
     srcs = glob(["**/*.ts"]),
-    # Visible to //:saucelabs_unit_tests_poc target
-    visibility = ["//:__pkg__"],
     deps = [
         "//packages/common/http",
         "//packages/common/http/testing",
@@ -21,7 +19,7 @@ ts_library(
 )
 
 # To run this:
-# yarn bazel test //packages/forms/experimental/test:test
+# yarn bazel test //packages/forms/experimental/test/node:test
 
 jasmine_node_test(
     name = "test",
@@ -31,7 +29,7 @@ jasmine_node_test(
     ],
 )
 
-# yarn bazel test //packages/forms/experimental/test:test_web_chromium
+# yarn bazel test //packages/forms/experimental/test/node:test_web_chromium
 karma_web_test_suite(
     name = "test_web",
     tags = ["manual"],

diff --git a/packages/forms/experimental/test/web/BUILD.bazel b/packages/forms/experimental/test/web/BUILD.bazel
@@ -4,8 +4,6 @@ ts_library(
     name = "test_lib",
     testonly = True,
     srcs = glob(["**/*.ts"]),
-    # Visible to //:saucelabs_unit_tests_poc target
-    visibility = ["//:__pkg__"],
     deps = [
         "//packages/core",
         "//packages/core/testing",
@@ -19,7 +17,7 @@ ts_library(
 )
 
 # To run this:
-# yarn bazel test //packages/forms/experimental/test:test
+# yarn bazel test //packages/forms/experimental/test/web:test
 
 karma_web_test_suite(
     name = "test",