Maybe support observableArray in markup control properties

src/Framework/Framework/Resources/Scripts/binding-handlers/markup-controls.ts

@@ -1,4 +1,6 @@
 import { isDotvvmObservable, isFakeObservableObject, unmapKnockoutObservables } from '../state-manager';
+import { proxyObservableArrayMethods } from '../utils/evaluator';
+import { isObservableArray } from '../utils/knockout';
 import { logWarning } from '../utils/logging';
 import { defineConstantProperty, isPrimitive, keys } from '../utils/objects';
 import * as manager from '../viewModules/viewModuleManager';
@@ -10,10 +12,16 @@ function isCommand(value: any, prop: string) {
 /** Wraps a function returning observable to make sure we have a single observable which we will not need to replace even if accessor returns a different instance. */
 function createWrapperComputed<T>(valueAccessor: () => T,
                                   observableAccessor: () => KnockoutObservable<T> | T = valueAccessor,
-                                  propertyDebugInfo: string | null = null) {
+                                  propertyDebugInfo: string | undefined = undefined,
+                                  isArray: boolean | undefined = undefined) {
     const computed = ko.pureComputed<T>({
         read() {
-            return valueAccessor();
+            const value = valueAccessor();
+            if (Array.isArray(value) && isArray == null) {
+                isArray = true
+                proxyObservableArrayMethods(computed, observableAccessor)
+            }
+            return value
         },
         write(value: T) {
             const val = observableAccessor();
@@ -25,6 +33,9 @@ function createWrapperComputed<T>(valueAccessor: () => T,
         }
     });
     (computed as any)["wrappedProperty"] = observableAccessor;
+    if (isArray) {
+        proxyObservableArrayMethods(computed, observableAccessor)
+    }
     Object.defineProperty(computed, "state", {
         get: () => unmapKnockoutObservables(observableAccessor(), true, true)
     })
@@ -57,10 +68,11 @@ function createWrapperComputed<T>(valueAccessor: () => T,
  *  The function assumes that the object hierarchy which needs wrapping is relatively small or updates are rare and simply replaces everything
  *  when the accessor value changes. */
 function createWrapperComputedRecursive<T>(accessor: () => KnockoutObservable<T> | T,
-                                           propertyDebugInfo: string | null = null) {
+                                           propertyDebugInfo: string | undefined = undefined,
+                                           isArray: boolean | undefined = undefined) {
     return createWrapperComputed<T>(/*valueAccessor:*/ () => processValue(accessor, accessor()),
                                     /*observableAccessor:*/ accessor,
-                                    propertyDebugInfo)
+                                    propertyDebugInfo, isArray)
 
     function processValue(accessor: () => KnockoutObservable<unknown> | unknown, value: unknown): any {
         const unwrapped = ko.unwrap(value)
@@ -87,7 +99,7 @@ function createWrapperComputedRecursive<T>(accessor: () => KnockoutObservable<T>
         // the value in observable is constant, we'll create new one if accessor returns new value
         // however, this process is asynchronnous, so for writes and `state`, `setState`, ... calls we call it again to be sure
         const processed = processValue(accessor, value)
-        return createWrapperComputed(() => processed, accessor, propertyDebugInfo)
+        return createWrapperComputed(() => processed, accessor, propertyDebugInfo, Array.isArray(processed))
     }
 }
 
@@ -120,7 +132,9 @@ export function wrapControlProperties(valueAccessor: () => any) {
         } else {
             value[prop] = createWrapperComputedRecursive(
                 () => valueAccessor()[prop],
-                compileConstants.debug ? `'${prop}' at '${valueAccessor}'` : prop);
+                compileConstants.debug ? `'${prop}' at '${valueAccessor}'` : prop,
+                /*isArray:*/ Array.isArray(value[prop]) || isObservableArray(value[prop]) ? true : undefined
+            );
         }
     }
     return value

src/Framework/Framework/Resources/Scripts/tests/markupControls.test.ts

@@ -1,3 +1,4 @@
+import { isObservableArray } from '../utils/knockout'
 import { initDotvvm } from './helper'
 
 const viewModel = {
@@ -6,7 +7,8 @@ const viewModel = {
 	ComplexObj: {
 		A: 1,
 		B: [ 1, 2, 3 ]
-	}
+	},
+	NullField: null
 }
 initDotvvm({viewModel})
 
@@ -131,3 +133,87 @@ it.each([
 	if (requiresSync) dotvvm.rootStateManager.doUpdateNow()
 	expect(context.Obj.state).toStrictEqual({ Something: {...viewModel.ComplexObj, A: 43210}, Arr: [1, 2] })
 })
+
+it.each([
+	[ "[1,2]", true, false ],
+	[ "ko.observableArray([1, 2])", true, true ],
+	[ "ComplexObj().B", true, true ],
+	[ "ComplexObj().B()", true, false ],
+	[ "ComplexObj().B().map(_ => 1)", true, false ],
+	[ "ComplexObj().B().length", false, false ],
+	[ "ComplexObj", false, false ],
+	[ "ComplexObj()", false, false ],
+	[ "null", false, false ],
+	[ "NullField", false, false ],
+])("dotvvm-with-control-properties makes observable array before its touched (%s)", (binding, isArray, tryObservableArray) => {
+	dotvvm.setState(viewModel); dotvvm.rootStateManager.doUpdateNow()
+
+	const div = document.createElement("div")
+	div.innerHTML = `
+		<div data-bind="dotvvm-with-control-properties: { 
+			Arr: ${binding}
+		}">
+			<span id=x />
+			<div data-bind="dotvvm-with-control-properties: { Arr: $control.Arr }">
+				<span id=y />
+			</div>
+		</div>
+	`
+
+	ko.applyBindings(dotvvm.viewModelObservables.root, div)
+
+	const x = div.querySelector("#x")!
+	const y = div.querySelector("#y")!
+	const contextX: any = ko.contextFor(x).$control
+	const contextY: any = ko.contextFor(y).$control
+
+	expect(contextX.Arr).observable()
+	expect(contextY.Arr).observable()
+
+	expect(isObservableArray(contextX.Arr)).toBe(isArray)
+	expect(isObservableArray(contextY.Arr)).toBe(isArray)
+
+	if (tryObservableArray) {
+		contextX.Arr.unshift(99)
+		const ix = contextX.Arr.indexOf(contextX.Arr()[0])
+		const ix2 = contextY.Arr.indexOf(contextX.Arr()[0])
+		expect(ix2).toEqual(ix)
+	}
+})
+
+test("dotvvm-with-control-properties correctly wraps null changed to array", () => {
+	dotvvm.setState(viewModel)
+
+	dotvvm.patchState({ ComplexObj: null })
+	dotvvm.rootStateManager.doUpdateNow()
+
+	const div = document.createElement("div")
+	div.innerHTML = `
+		<div data-bind="dotvvm-with-control-properties: { 
+			Arr: ComplexObj()?.B
+		}"> <span id=x /> </div>
+	`
+	ko.applyBindings(dotvvm.viewModelObservables.root, div)
+	const x = div.querySelector("#x")!
+	const context: any = ko.contextFor(x).$control
+	expect(context.Arr).observable()
+	expect(context.Arr.state).toEqual(undefined)
+	expect(context.Arr()).toEqual(undefined)
+	expect("push" in context.Arr).toBe(false)
+
+	dotvvm.patchState({ ComplexObj: { B: [1, 2] } })
+	dotvvm.rootStateManager.doUpdateNow()
+
+	expect(context.Arr()?.map((x: any) => x())).toStrictEqual([1, 2])
+	expect(context.Arr.state).toStrictEqual([1, 2])
+	expect("push" in context.Arr).toBe(true)
+
+	context.Arr.push(3)
+	expect(context.Arr()?.map((x:any) => x())).toStrictEqual([1, 2, 3])
+	expect(dotvvm.state.ComplexObj.B).toStrictEqual([1, 2, 3])
+
+	context.Arr.unshift(0)
+	context.Arr.splice(1, 1)
+
+	expect(dotvvm.state.ComplexObj.B).toStrictEqual([0, 2, 3])
+})

src/Framework/Framework/Resources/Scripts/utils/evaluator.ts

@@ -94,10 +94,7 @@ export function wrapObservable(func: () => any, isArray?: boolean): KnockoutComp
     });
 
     if (isArray) {
-        for (const i of ["push", "pop", "unshift", "shift", "reverse", "sort", "splice", "slice", "replace", "indexOf", "remove", "removeAll"]) {
-            (wrapper as any)[i] = (...args: any) => updateObservableArray(func, i, args);
-        }
-        wrapper = wrapper.extend({ trackArrayChanges: true });
+        proxyObservableArrayMethods(wrapper, () => getExpressionResult(func))
     }
 
     return wrapper.extend({ notify: "always" });
@@ -113,16 +110,20 @@ function updateObservable(getObservable: () => KnockoutObservable<any>, value: a
     }
 }
 
-function updateObservableArray(getObservableArray: () => KnockoutObservableArray<any>, fnName: string, args: any[]) {
-    const result = getExpressionResult(getObservableArray);
-
-    if (!isObservableArray(result)) {
-        logError("validation", `Cannot execute '${fnName}' function on ko.computed because the expression '${getObservableArray}' does not return an observable array.`);
-    } else {
-        result[fnName].apply(result, args);
+export function proxyObservableArrayMethods(wrapper: KnockoutObservable<any>, getObservableArray: () => any) {
+    for (const fnName of ["push", "pop", "unshift", "shift", "reverse", "sort", "splice", "slice", "replace", "indexOf", "remove", "removeAll"]) {
+        (wrapper as any)[fnName] = (...args: any) => {
+            const result = getExpressionResult(getObservableArray)
+
+            if (!isObservableArray(result)) {
+                return logError("validation", compileConstants.debug ? `Cannot execute '${fnName}' function on ko.computed because the expression '${getObservableArray}' does not return an observable array.` : 'Target is not observableArray')
+            } else {
+                return result[fnName](...args)
+            }
+        }
     }
+    wrapper.extend({ trackArrayChanges: true })
 }
-
 export const unwrapComputedProperty = (obs: any) =>
     ko.isComputed(obs) && "wrappedProperty" in obs ?
     (obs as any)["wrappedProperty"]() : // workaround for dotvvm-with-control-properties handler