feat(TypeScript): support binding with optional fields

BREAKING CHANGE: TypeScript the `value` property of `BinderNode` now has
optionally `undefined` type for non-initialised optional fields.

In terms of forms, an object field could be used in both ways: as
an individual field or a grouping for the object’s nested fields.

Previously, the optional fields in empty form value objects were initialised
by the client-side binder with a generated value when `BinderNode` or
form binding is created for such fields. Such a behavior works well with
grouping object fields, but makes it problematic to bind an individual
optional field of object type by making empty value already defined.

With this change, the client-side form binder skips initialising
optional fields, keeping their values `undefined`, unless there is
a binding for a nested field inside an optional parent object.

For example: with a binding for `model.optionalObject` will have
a corresponding empty form value of `{"optionalObject": undefined}`,
however, with a binding of `model.optionalObject.nestedString`, the
empty form value will contain a generated object: `{"optioanlObject":
{"nestedString": ""}}`.

flow-client/src/main/resources/META-INF/resources/frontend/form/BinderNode.ts

@@ -39,11 +39,11 @@ function getErrorPropertyName(valueError: ValueError<any>): string {
 }
 
 /**
- * The BinderNode<T, M> class provides the form binding related APIs 
+ * The BinderNode<T, M> class provides the form binding related APIs
  * with respect to a particular model instance.
- * 
- * Structurally, model instances form a tree, in which the object 
- * and array models have child nodes of field and array item model 
+ *
+ * Structurally, model instances form a tree, in which the object
+ * and array models have child nodes of field and array item model
  * instances.
  */
 export class BinderNode<T, M extends AbstractModel<T>> {
@@ -91,11 +91,11 @@ export class BinderNode<T, M extends AbstractModel<T>> {
   /**
    * The current value related to the model
    */
-  get value(): T {
+  get value(): T | undefined {
     return this.parent!.value[this.model[_key]];
   }
 
-  set value(value: T) {
+  set value(value: T | undefined) {
     this.setValueState(value);
   }
 
@@ -132,7 +132,7 @@ export class BinderNode<T, M extends AbstractModel<T>> {
 
   /**
    * Returns a binder node for the nested model instance.
-   * 
+   *
    * @param model The nested model instance
    */
   for<NM extends AbstractModel<any>>(model: NM) {
@@ -145,8 +145,8 @@ export class BinderNode<T, M extends AbstractModel<T>> {
   }
 
   /**
-   * Runs all validation callbacks potentially affecting this 
-   * or any nested model. Returns the combined array of all 
+   * Runs all validation callbacks potentially affecting this
+   * or any nested model. Returns the combined array of all
    * errors as in the errors property.
    */
   async validate(): Promise<ReadonlyArray<ValueError<any>>> {
@@ -164,7 +164,7 @@ export class BinderNode<T, M extends AbstractModel<T>> {
 
   /**
    * A helper method to add a validator
-   * 
+   *
    * @param validator a validator
    */
   addValidator(validator: Validator<T>) {
@@ -359,46 +359,61 @@ export class BinderNode<T, M extends AbstractModel<T>> {
     ];
   }
 
-  private initializeValue() {
+  private initializeValue(requiredByChildNode: boolean = false) {
     // First, make sure parents have value initialized
     if (this.parent && ((this.parent.value === undefined) || (this.parent.defaultValue === undefined))) {
-      this.parent.initializeValue();
+      this.parent.initializeValue(true);
     }
 
     let value = this.parent
       ? this.parent.value[this.model[_key]]
       : undefined;
 
     if (value === undefined) {
-      // Initialize value if necessary
-      value = value !== undefined
-        ? value
-        : (this.model.constructor as ModelConstructor<T, M>).createEmptyValue()
-      this.setValueState(value, this.defaultValue === undefined);
+      // Initialize value if a child node is accessed or for the root-level node
+      if (requiredByChildNode || !this.parent) {
+        value = value !== undefined
+          ? value
+          : (this.model.constructor as ModelConstructor<T, M>).createEmptyValue();
+        this.setValueState(value, this.defaultValue === undefined);
+      } else if (
+        this.parent
+        && (this.parent.model instanceof ObjectModel)
+        && !(this.model[_key] in this.parent.value)
+      ) {
+        this.setValueState(undefined, this.defaultValue === undefined);
+      }
     }
   }
 
-  private setValueState(value: T, keepPristine: boolean = false) {
+  private setValueState(value: T | undefined, keepPristine: boolean = false) {
     const modelParent = this.model[_parent];
-    if (modelParent instanceof ArrayModel) {
-      // Value contained in array - replace array in parent
-      const array = (this.parent!.value as ReadonlyArray<T>).slice();
-      array[this.model[_key] as number] = value;
-      this.parent!.setValueState(array, keepPristine);
-    } else if (modelParent instanceof ObjectModel) {
+    if (modelParent instanceof ObjectModel) {
       // Value contained in object - replace object in parent
       const object = {
         ...this.parent!.value,
         [this.model[_key]]: value
       };
       this.parent!.setValueState(object, keepPristine);
+      return;
+    }
+
+    if (value === undefined) {
+      throw new TypeError('Unexpected undefined value');
+    }
+
+    if (modelParent instanceof ArrayModel) {
+      // Value contained in array - replace array in parent
+      const array = (this.parent!.value as ReadonlyArray<T>).slice();
+      array[this.model[_key] as number] = value;
+      this.parent!.setValueState(array, keepPristine);
     } else {
       // Value contained elsewhere, probably binder - use value property setter
       const binder = modelParent as Binder<T, M>;
       if (keepPristine && !binder.dirty) {
         binder.defaultValue = value;
       }
-      binder.value = value;
+      binder.value = value!;
     }
   }
 }

flow-client/src/main/resources/META-INF/resources/frontend/form/Field.ts

@@ -60,6 +60,16 @@ export class CheckedFieldStrategy extends GenericFieldStrategy {
   }
 }
 
+export class ComboBoxFieldStrategy extends VaadinFieldStrategy {
+  get value() {
+    const selectedItem = (this.element as any).selectedItem;
+    return selectedItem === null ? undefined : selectedItem;
+  }
+  set value(val: any) {
+    (this.element as any).selectedItem = val === undefined ? null : val;
+  }
+}
+
 export class SelectedFieldStrategy extends GenericFieldStrategy {
   set value(val: any) {
     (this.element as any).selected = val;
@@ -73,6 +83,8 @@ export function getDefaultFieldStrategy(elm: any): FieldStrategy {
   switch(elm.localName) {
     case 'vaadin-checkbox': case 'vaadin-radio-button':
       return new CheckedFieldStrategy(elm);
+    case 'vaadin-combo-box':
+      return new ComboBoxFieldStrategy(elm);
     case 'vaadin-list-box':
       return new SelectedFieldStrategy(elm);
     case 'vaadin-rich-text-editor':
@@ -86,9 +98,9 @@ export function getDefaultFieldStrategy(elm: any): FieldStrategy {
 
 /**
  * Binds a form field component into a model.
- * 
+ *
  * Exmaple usage:
- * 
+ *
  * ```
  * <vaadin-text-field ...="${field(model.name)}">
  * </vaadin-text-field>

flow-client/src/main/resources/META-INF/resources/frontend/form/Models.ts

@@ -61,7 +61,11 @@ export abstract class AbstractModel<T> {
     return String(this.valueOf());
   }
   valueOf(): T {
-    return getBinderNode(this).value;
+    const value = getBinderNode(this).value;
+    if (value === undefined) {
+      throw new TypeError('Value is undefined');
+    }
+    return value;
   }
 }
 
@@ -99,12 +103,11 @@ export class ObjectModel<T> extends AbstractModel<T> {
         // Initialise the properties in the value object with empty value
         .reduce((o, propertyName) => {
           const propertyModel = (modelInstance as any)[propertyName] as AbstractModel<any>;
-          // Skip initialising optional properties
-          if (!propertyModel[_optional]) {
-            (o as any)[propertyName] = (
+          (o as any)[propertyName] = propertyModel[_optional]
+            ? undefined
+            : (
               propertyModel.constructor as ModelConstructor<any, AbstractModel<any>>
             ).createEmptyValue();
-          }
           return o;
         }, obj)
     }
@@ -154,7 +157,7 @@ export class ArrayModel<T, M extends AbstractModel<T>> extends AbstractModel<Rea
    * Iterates the current array value and yields a binder node for every item.
    */
   *[Symbol.iterator](): IterableIterator<BinderNode<T, M>> {
-    const array = getBinderNode(this).value;
+    const array = this.valueOf();
     const ItemModel = this[_ItemModel];
     if (array.length !== this.itemModels.length) {
       this.itemModels.length = array.length;

flow-client/src/main/resources/META-INF/resources/frontend/form/Validation.ts

@@ -46,10 +46,10 @@ export class ServerValidator implements Validator<any> {
 export async function runValidator<T>(model: AbstractModel<T>, validator: Validator<T>): Promise<ReadonlyArray<ValueError<T>>> {
   const value = getBinderNode(model).value;
   // if model is not required and value empty, do not run any validator
-  if (!getBinderNode(model).required && !new Required().validate(value)) {
+  if (!getBinderNode(model).required && !new Required().validate(value!)) {
     return [];
   }
-  return (async () => validator.validate(value, getBinderNode(model).binder))()
+  return (async () => validator.validate(value!, getBinderNode(model).binder))()
     .then(result => {
       if (result === false) {
         return [{ property: getBinderNode(model).name, value, validator, message: validator.message }];

flow-client/src/test/frontend/form/BinderTests.ts

@@ -249,7 +249,8 @@ suite("form/Binder", () => {
 
     const expectedEmptyEmployee: Employee = {
       idString: '',
-      fullName: ''
+      fullName: '',
+      supervisor: undefined
     };
 
     beforeEach(() => {
@@ -272,32 +273,33 @@ suite("form/Binder", () => {
     test('should not initialize optional in binder value and default value', () => {
       assert.isUndefined(binder.defaultValue.supervisor);
       assert.deepEqual(binder.defaultValue, expectedEmptyEmployee);
+      // Ensure the key is present in the object
+      assert.isTrue('supervisor' in binder.defaultValue);
       assert.isUndefined(binder.value.supervisor);
       assert.deepEqual(binder.value, expectedEmptyEmployee);
+      assert.isTrue('supervisor' in binder.value);
     });
 
-    test('should initialize optional on binderNode access', () => {
+    test('should not initialize optional on binderNode access', () => {
       binder.for(binder.model.supervisor);
 
-      assert.isDefined(binder.defaultValue.supervisor);
-      assert.deepEqual(binder.defaultValue.supervisor, expectedEmptyEmployee);
-      assert.deepEqual(getOnlyEmployeeData(binder.defaultValue), expectedEmptyEmployee);
-      assert.isDefined(binder.value.supervisor);
-      assert.deepEqual(binder.value.supervisor, expectedEmptyEmployee);
-      assert.deepEqual(getOnlyEmployeeData(binder.value), expectedEmptyEmployee);
+      assert.isUndefined(binder.defaultValue.supervisor);
+      assert.deepEqual(binder.defaultValue, expectedEmptyEmployee);
+      assert.isTrue('supervisor' in binder.defaultValue);
+      assert.isUndefined(binder.value.supervisor);
+      assert.deepEqual(binder.value, expectedEmptyEmployee);
+      assert.isTrue('supervisor' in binder.value);
     });
 
-    test('should initialize optional on deep binderNode access', () => {
+    test('should initialize parent optional on child binderNode access', () => {
       binder.for(binder.model.supervisor.supervisor);
 
-      assert.isDefined(binder.defaultValue.supervisor!.supervisor);
-      assert.deepEqual(getOnlyEmployeeData(binder.defaultValue), expectedEmptyEmployee);
-      assert.deepEqual(getOnlyEmployeeData(binder.defaultValue.supervisor!), expectedEmptyEmployee);
-      assert.deepEqual(getOnlyEmployeeData(binder.defaultValue.supervisor!.supervisor!), expectedEmptyEmployee);
-      assert.isDefined(binder.value.supervisor!.supervisor);
-      assert.deepEqual(getOnlyEmployeeData(binder.value), expectedEmptyEmployee);
-      assert.deepEqual(getOnlyEmployeeData(binder.value.supervisor!), expectedEmptyEmployee);
-      assert.deepEqual(getOnlyEmployeeData(binder.value.supervisor!.supervisor!), expectedEmptyEmployee);
+      assert.isDefined(binder.defaultValue.supervisor);
+      assert.deepEqual(binder.defaultValue.supervisor, expectedEmptyEmployee);
+      assert.deepEqual(getOnlyEmployeeData(binder.defaultValue), getOnlyEmployeeData(expectedEmptyEmployee));
+      assert.isDefined(binder.value.supervisor);
+      assert.deepEqual(binder.value.supervisor, expectedEmptyEmployee);
+      assert.deepEqual(getOnlyEmployeeData(binder.value), getOnlyEmployeeData(expectedEmptyEmployee));
     });
 
     test('should not become dirty on binderNode access', () => {