refactor(lit-form): deprecate AbstractModel methods that require BinderNode

packages/ts/lit-form/src/Models.ts

@@ -3,14 +3,15 @@ import { type BinderNode, getBinderNode } from './BinderNode.js';
 import type { Validator } from './Validation.js';
 import { IsNumber } from './Validators.js';
 
-export const _createEmptyItemValue = Symbol('itemModel');
+export const _createEmptyItemValue = Symbol('createEmptyItemValue');
 export const _parent = Symbol('parent');
 export const _key = Symbol('key');
 export const _fromString = Symbol('fromString');
 export const _validators = Symbol('validators');
 export const _meta = Symbol('meta');
 export const _getPropertyModel = Symbol('getPropertyModel');
 export const _enum = Symbol('enum');
+export const _items = Symbol('items');
 
 const _optional = Symbol('optional');
 
@@ -77,10 +78,26 @@ export abstract class AbstractModel<T = unknown> {
     this[_meta] = options?.meta ?? {};
   }
 
+  /**
+   * @deprecated Use {@link BinderNode.value} with string conversion instead
+   *
+   * @example
+   * ```ts
+   * const result = String(binder.for(model).value);
+   * ```
+   */
   toString(): string {
     return String(this.valueOf());
   }
 
+  /**
+   * @deprecated Use {@link BinderNode.value} instead
+   *
+   * @example
+   * ```ts
+   * const result = binder.for(model).value;
+   * ```
+   */
   valueOf(): T {
     const { value } = getBinderNode(this);
 
@@ -213,40 +230,76 @@ export class ArrayModel<MItem extends AbstractModel = AbstractModel> extends Abs
 
   [_createEmptyItemValue]: () => Value<MItem>;
 
-  readonly #createItem: (parent: this, index: number) => MItem;
+  // The `parent` parameter is AbstractModel here for purpose; for some reason, setting it to `ArrayModel<MItem>` or
+  // `this` breaks the type inference in TS (v5.3.2).
+  readonly #createItem: (parent: AbstractModel, index: number) => MItem;
   #items: Array<MItem | undefined> = [];
 
   constructor(
     parent: ModelParent,
     key: keyof any,
     optional: boolean,
-    createItem: (parent: ArrayModel<MItem>, key: number) => MItem,
+    createItem: (parent: AbstractModel, key: number) => MItem,
     options?: ModelOptions<Array<Value<MItem>>>,
   ) {
     super(parent, key, optional, options);
     this.#createItem = createItem;
     this[_createEmptyItemValue] = createItem(this, 0).constructor.createEmptyValue as () => Value<MItem>;
   }
 
-  /**
-   * Iterates the current array value and yields a binder node for every item.
-   */
-  *[Symbol.iterator](): IterableIterator<BinderNode<MItem>> {
-    const array = this.valueOf();
+  *[_items](): Generator<MItem, void, void> {
+    const values = getBinderNode(this).value ?? [];
 
-    if (array.length !== this.#items.length) {
-      this.#items.length = array.length;
+    if (values.length !== this.#items.length) {
+      this.#items.length = values.length;
     }
 
-    for (let i = 0; i < array.length; i++) {
+    for (let i = 0; i < values.length; i++) {
       let item: MItem | undefined = this.#items[i];
 
       if (!item) {
         item = this.#createItem(this, i);
         this.#items[i] = item;
       }
 
+      yield item;
+    }
+  }
+
+  /**
+   * Iterates over the current model and yields a binder node for every item
+   * model.
+   *
+   * @deprecated Use the {@link m.items} function instead. For example, in React:
+   * ```tsx
+   * const {model, field} = useForm(GroupModel);
+   * return Array.from(m.items(model.people), (personModel) =>
+   *   <TextField label="Full name" {...field(personModel.fullName)} />
+   * );
+   * ```
+   * In Lit:
+   * ```ts
+   * return html`${repeat(
+   *   m.items(this.binder.model.people),
+   *   (personModel) => html`<vaadin-text-field label="Full name" ${field(personModel.fullName)}></vaadin-text-field>`,
+   * )}`;
+   * ```
+   */
+  *[Symbol.iterator](): IterableIterator<BinderNode<MItem>> {
+    for (const item of this[_items]()) {
       yield getBinderNode(item);
     }
   }
 }
+
+export const m = {
+  /**
+   * Returns an iterator over item models in the array model.
+   *
+   * @param model - The array model to iterate over.
+   * @returns An iterator over item models.
+   */
+  items<M extends ArrayModel>(model: M): Generator<ArrayItemModel<M>, void, void> {
+    return model[_items]() as Generator<ArrayItemModel<M>, void, void>;
+  },
+};

packages/ts/lit-form/src/Validation.ts

@@ -1,7 +1,7 @@
 import type { BinderNode } from './BinderNode.js';
 import { getBinderNode } from './BinderNode.js';
 import type { BinderRoot } from './BinderRoot.js';
-import { type AbstractModel, NumberModel, type Value } from './Models.js';
+import { AbstractModel, NumberModel, type Value } from './Models.js';
 import { Required } from './Validators.js';
 
 export interface ValueError<T = unknown> {
@@ -24,9 +24,10 @@ export class ValidationError extends Error {
     super(
       [
         'There are validation errors in the form.',
-        ...errors.map(
-          (e) => `${e.property.toString()} - ${e.validator.constructor.name}${e.message ? `: ${e.message}` : ''}`,
-        ),
+        ...errors.map((e) => {
+          const property = e.property instanceof AbstractModel ? String(getBinderNode(e.property).value) : e.property;
+          return `${property} - ${e.validator.constructor.name}${e.message ? `: ${e.message}` : ''}`;
+        }),
       ].join('\n - '),
     );
     this.errors = errors;

packages/ts/lit-form/test/Binder.test.ts

@@ -9,13 +9,11 @@ import { Binder, type BinderConfiguration } from '../src/index.js';
 import {
   type Employee,
   EmployeeModel,
+  Level1Model,
+  Level2Model,
   type Order,
   OrderModel,
-  type TestEntity,
   TestModel,
-  type Level1,
-  Level1Model,
-  Level2Model,
 } from './TestModels.js';
 
 use(sinonChai);
@@ -126,22 +124,11 @@ describe('@vaadin/hilla-lit-form', () => {
         assert.deepEqual(binder.defaultValue, expectedEmptyOrder);
       });
 
-      it('should have valueOf', () => {
-        assert.equal(binder.model.notes.valueOf(), '');
-      });
-
-      it('should have toString', () => {
-        assert.equal(binder.model.notes.toString(), '');
-      });
-
       it('should have initial value', () => {
         assert.equal(binder.value, binder.defaultValue);
         assert.equal(binder.for(binder.model).value, binder.value);
         assert.equal(binder.for(binder.model.notes).value, '');
         assert.equal(binder.for(binder.model.customer.fullName).value, '');
-        assert.equal(binder.model.valueOf(), binder.value);
-        assert.equal(binder.model.notes.valueOf(), '');
-        assert.equal(binder.model.customer.fullName.valueOf(), '');
       });
 
       it('should change value on setValue', () => {

packages/ts/lit-form/test/Model.test.ts

@@ -5,16 +5,19 @@ import {
   _fromString,
   _key,
   _meta,
+  type AbstractModel,
   ArrayModel,
   Binder,
+  type BinderNode,
   EnumModel,
   IsNumber,
+  m,
+  type ModelMetadata,
   NotBlank,
   NotEmpty,
   NotNull,
   NumberModel,
   ObjectModel,
-  type ModelMetadata,
   Positive,
   Size,
 } from '../src/index.js';
@@ -148,6 +151,14 @@ describe('@vaadin/hilla-lit-form', () => {
     });
 
     describe('array model', () => {
+      function* toBinderNode<M extends AbstractModel>(
+        iterable: Iterable<M>,
+      ): Generator<BinderNode<M>, undefined, void> {
+        for (const value of iterable) {
+          yield binder.for(value);
+        }
+      }
+
       const strings = ['foo', 'bar'];
 
       const idEntities: readonly IdEntity[] = [
@@ -166,14 +177,12 @@ describe('@vaadin/hilla-lit-form', () => {
       it('should be iterable', () => {
         [binder.model.fieldArrayString, binder.model.fieldArrayModel].forEach((arrayModel) => {
           const values = binder.for(arrayModel).value!;
-          const iterator = arrayModel[Symbol.iterator]();
+          const iterator = toBinderNode(m.items(arrayModel));
           for (let i = 0; i < values.length; i++) {
             const iteratorResult = iterator.next();
             expect(iteratorResult.done).to.be.false;
-            const binderNode = iteratorResult.value;
-            // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
+            const binderNode = iteratorResult.value!;
             expect(binderNode.model[_key]).to.equal(i);
-            // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
             expect(binderNode.value).to.equal(values[i]);
           }
 
@@ -284,11 +293,11 @@ describe('@vaadin/hilla-lit-form', () => {
       });
 
       it('should reuse model instance for the same array item', () => {
-        const nodes1 = [...binder.model.fieldArrayModel].slice();
+        const nodes1 = [...toBinderNode(m.items(binder.model.fieldArrayModel))];
         [0, 1].forEach((i) => expect(nodes1[i].value).to.be.equal(idEntities[i]));
 
         binder.for(binder.model.fieldArrayModel).value = idEntities.slice();
-        const nodes2 = [...binder.model.fieldArrayModel].slice();
+        const nodes2 = [...toBinderNode(m.items(binder.model.fieldArrayModel))];
         [0, 1].forEach((i) => {
           expect(nodes1[i]).to.be.equal(nodes2[i]);
           expect(nodes1[i].model).to.be.equal(nodes2[i].model);
@@ -297,7 +306,7 @@ describe('@vaadin/hilla-lit-form', () => {
       });
 
       it('should reuse model instance for the same array item after it is modified', () => {
-        const nodes1 = [...binder.model.fieldArrayModel].slice();
+        const nodes1 = [...toBinderNode(m.items(binder.model.fieldArrayModel))];
         [0, 1].forEach((i) => expect(nodes1[i].value).to.be.equal(idEntities[i]));
 
         binder.for(nodes1[0].model.idString).value = 'foo';
@@ -307,7 +316,7 @@ describe('@vaadin/hilla-lit-form', () => {
         binder.for(binder.model.fieldArrayModel).prependItem();
         binder.for(binder.model.fieldArrayModel).appendItem();
 
-        const nodes2 = [...binder.model.fieldArrayModel].slice();
+        const nodes2 = [...toBinderNode(m.items(binder.model.fieldArrayModel))];
 
         [0, 1].forEach((i) => {
           expect(nodes1[i]).to.be.equal(nodes2[i]);
@@ -317,20 +326,20 @@ describe('@vaadin/hilla-lit-form', () => {
       });
 
       it('should update model keySymbol when inserting items', () => {
-        const nodes1 = [...binder.model.fieldArrayModel].slice();
+        const nodes1 = [...toBinderNode(m.items(binder.model.fieldArrayModel))];
         [0, 1].forEach((i) => expect(nodes1[i].value).to.be.equal(idEntities[i]));
 
         for (let i = 0; i < nodes1.length; i++) {
           expect(nodes1[i].model[_key]).to.be.equal(i);
         }
 
         binder.for(nodes1[0].model.idString).value = 'foo';
-        expect(binder.model.fieldArrayModel.valueOf()[0].idString).to.be.equal('foo');
+        expect(binder.for(binder.model.fieldArrayModel).value?.[0].idString).to.be.equal('foo');
 
         binder.for(binder.model.fieldArrayModel).prependItem();
-        expect(binder.model.fieldArrayModel.valueOf()[1].idString).to.be.equal('foo');
+        expect(binder.for(binder.model.fieldArrayModel).value?.[1].idString).to.be.equal('foo');
 
-        const nodes2 = [...binder.model.fieldArrayModel].slice();
+        const nodes2 = [...toBinderNode(m.items(binder.model.fieldArrayModel))];
         expect(nodes2.length).to.be.equal(3);
         for (let i = 0; i < nodes2.length; i++) {
           expect(nodes2[i].model[_key]).to.be.equal(i);
@@ -369,7 +378,7 @@ describe('@vaadin/hilla-lit-form', () => {
 
       it('should record and return the value', () => {
         binder.for(binder.model.fieldEnum).value = RecordStatus.REMOVED;
-        expect(binder.model.fieldEnum.valueOf()).to.equal(RecordStatus.REMOVED);
+        expect(binder.for(binder.model.fieldEnum).value).to.equal(RecordStatus.REMOVED);
       });
 
       it('should be undefined if the EnumModel.createEmptyValue() is used', () => {