diff --git a/src/Framework/Framework/Resources/Scripts/binding-handlers/markup-controls.ts b/src/Framework/Framework/Resources/Scripts/binding-handlers/markup-controls.ts index 4fd0c84760..22a458c718 100644 --- a/src/Framework/Framework/Resources/Scripts/binding-handlers/markup-controls.ts +++ b/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(valueAccessor: () => T, observableAccessor: () => KnockoutObservable | T = valueAccessor, - propertyDebugInfo: string | null = null) { + propertyDebugInfo: string | undefined = undefined, + isArray: boolean | undefined = undefined) { const computed = ko.pureComputed({ 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(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(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(accessor: () => KnockoutObservable | T, - propertyDebugInfo: string | null = null) { + propertyDebugInfo: string | undefined = undefined, + isArray: boolean | undefined = undefined) { return createWrapperComputed(/*valueAccessor:*/ () => processValue(accessor, accessor()), /*observableAccessor:*/ accessor, - propertyDebugInfo) + propertyDebugInfo, isArray) function processValue(accessor: () => KnockoutObservable | unknown, value: unknown): any { const unwrapped = ko.unwrap(value) @@ -87,7 +99,7 @@ function createWrapperComputedRecursive(accessor: () => KnockoutObservable // 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 diff --git a/src/Framework/Framework/Resources/Scripts/tests/markupControls.test.ts b/src/Framework/Framework/Resources/Scripts/tests/markupControls.test.ts index c2dfee01e1..3aa0f85e97 100644 --- a/src/Framework/Framework/Resources/Scripts/tests/markupControls.test.ts +++ b/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 = ` +
+ +
+ +
+
+ ` + + 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 = ` +
+ ` + 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]) +}) diff --git a/src/Framework/Framework/Resources/Scripts/utils/evaluator.ts b/src/Framework/Framework/Resources/Scripts/utils/evaluator.ts index 76fbe651ba..8f97079129 100644 --- a/src/Framework/Framework/Resources/Scripts/utils/evaluator.ts +++ b/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, value: a } } -function updateObservableArray(getObservableArray: () => KnockoutObservableArray, 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, 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