Skip to content

Commit f698113

Browse files
tomashercegtomasherceg
authored
Merge pull request #1914 from riganti/maybe-markupcontrol-observablearray
Maybe support observableArray in markup control properties
2 parents 42bb6ea + 8cb66f0 commit f698113

File tree

3 files changed

+120
-19
lines changed

3 files changed

+120
-19
lines changed

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

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
import { isDotvvmObservable, isFakeObservableObject, unmapKnockoutObservables } from '../state-manager';
2+
import { proxyObservableArrayMethods } from '../utils/evaluator';
3+
import { isObservableArray } from '../utils/knockout';
24
import { logWarning } from '../utils/logging';
35
import { defineConstantProperty, isPrimitive, keys } from '../utils/objects';
46
import * as manager from '../viewModules/viewModuleManager';
@@ -10,10 +12,16 @@ function isCommand(value: any, prop: string) {
1012
/** 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. */
1113
function createWrapperComputed<T>(valueAccessor: () => T,
1214
observableAccessor: () => KnockoutObservable<T> | T = valueAccessor,
13-
propertyDebugInfo: string | null = null) {
15+
propertyDebugInfo: string | undefined = undefined,
16+
isArray: boolean | undefined = undefined) {
1417
const computed = ko.pureComputed<T>({
1518
read() {
16-
return valueAccessor();
19+
const value = valueAccessor();
20+
if (Array.isArray(value) && isArray == null) {
21+
isArray = true
22+
proxyObservableArrayMethods(computed, observableAccessor)
23+
}
24+
return value
1725
},
1826
write(value: T) {
1927
const val = observableAccessor();
@@ -25,6 +33,9 @@ function createWrapperComputed<T>(valueAccessor: () => T,
2533
}
2634
});
2735
(computed as any)["wrappedProperty"] = observableAccessor;
36+
if (isArray) {
37+
proxyObservableArrayMethods(computed, observableAccessor)
38+
}
2839
Object.defineProperty(computed, "state", {
2940
get: () => unmapKnockoutObservables(observableAccessor(), true, true)
3041
})
@@ -57,10 +68,11 @@ function createWrapperComputed<T>(valueAccessor: () => T,
5768
* The function assumes that the object hierarchy which needs wrapping is relatively small or updates are rare and simply replaces everything
5869
* when the accessor value changes. */
5970
function createWrapperComputedRecursive<T>(accessor: () => KnockoutObservable<T> | T,
60-
propertyDebugInfo: string | null = null) {
71+
propertyDebugInfo: string | undefined = undefined,
72+
isArray: boolean | undefined = undefined) {
6173
return createWrapperComputed<T>(/*valueAccessor:*/ () => processValue(accessor, accessor()),
6274
/*observableAccessor:*/ accessor,
63-
propertyDebugInfo)
75+
propertyDebugInfo, isArray)
6476

6577
function processValue(accessor: () => KnockoutObservable<unknown> | unknown, value: unknown): any {
6678
const unwrapped = ko.unwrap(value)
@@ -87,7 +99,7 @@ function createWrapperComputedRecursive<T>(accessor: () => KnockoutObservable<T>
8799
// the value in observable is constant, we'll create new one if accessor returns new value
88100
// however, this process is asynchronnous, so for writes and `state`, `setState`, ... calls we call it again to be sure
89101
const processed = processValue(accessor, value)
90-
return createWrapperComputed(() => processed, accessor, propertyDebugInfo)
102+
return createWrapperComputed(() => processed, accessor, propertyDebugInfo, Array.isArray(processed))
91103
}
92104
}
93105

@@ -120,7 +132,9 @@ export function wrapControlProperties(valueAccessor: () => any) {
120132
} else {
121133
value[prop] = createWrapperComputedRecursive(
122134
() => valueAccessor()[prop],
123-
compileConstants.debug ? `'${prop}' at '${valueAccessor}'` : prop);
135+
compileConstants.debug ? `'${prop}' at '${valueAccessor}'` : prop,
136+
/*isArray:*/ Array.isArray(value[prop]) || isObservableArray(value[prop]) ? true : undefined
137+
);
124138
}
125139
}
126140
return value

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

Lines changed: 87 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { isObservableArray } from '../utils/knockout'
12
import { initDotvvm } from './helper'
23

34
const viewModel = {
@@ -6,7 +7,8 @@ const viewModel = {
67
ComplexObj: {
78
A: 1,
89
B: [ 1, 2, 3 ]
9-
}
10+
},
11+
NullField: null
1012
}
1113
initDotvvm({viewModel})
1214

@@ -131,3 +133,87 @@ it.each([
131133
if (requiresSync) dotvvm.rootStateManager.doUpdateNow()
132134
expect(context.Obj.state).toStrictEqual({ Something: {...viewModel.ComplexObj, A: 43210}, Arr: [1, 2] })
133135
})
136+
137+
it.each([
138+
[ "[1,2]", true, false ],
139+
[ "ko.observableArray([1, 2])", true, true ],
140+
[ "ComplexObj().B", true, true ],
141+
[ "ComplexObj().B()", true, false ],
142+
[ "ComplexObj().B().map(_ => 1)", true, false ],
143+
[ "ComplexObj().B().length", false, false ],
144+
[ "ComplexObj", false, false ],
145+
[ "ComplexObj()", false, false ],
146+
[ "null", false, false ],
147+
[ "NullField", false, false ],
148+
])("dotvvm-with-control-properties makes observable array before its touched (%s)", (binding, isArray, tryObservableArray) => {
149+
dotvvm.setState(viewModel); dotvvm.rootStateManager.doUpdateNow()
150+
151+
const div = document.createElement("div")
152+
div.innerHTML = `
153+
<div data-bind="dotvvm-with-control-properties: {
154+
Arr: ${binding}
155+
}">
156+
<span id=x />
157+
<div data-bind="dotvvm-with-control-properties: { Arr: $control.Arr }">
158+
<span id=y />
159+
</div>
160+
</div>
161+
`
162+
163+
ko.applyBindings(dotvvm.viewModelObservables.root, div)
164+
165+
const x = div.querySelector("#x")!
166+
const y = div.querySelector("#y")!
167+
const contextX: any = ko.contextFor(x).$control
168+
const contextY: any = ko.contextFor(y).$control
169+
170+
expect(contextX.Arr).observable()
171+
expect(contextY.Arr).observable()
172+
173+
expect(isObservableArray(contextX.Arr)).toBe(isArray)
174+
expect(isObservableArray(contextY.Arr)).toBe(isArray)
175+
176+
if (tryObservableArray) {
177+
contextX.Arr.unshift(99)
178+
const ix = contextX.Arr.indexOf(contextX.Arr()[0])
179+
const ix2 = contextY.Arr.indexOf(contextX.Arr()[0])
180+
expect(ix2).toEqual(ix)
181+
}
182+
})
183+
184+
test("dotvvm-with-control-properties correctly wraps null changed to array", () => {
185+
dotvvm.setState(viewModel)
186+
187+
dotvvm.patchState({ ComplexObj: null })
188+
dotvvm.rootStateManager.doUpdateNow()
189+
190+
const div = document.createElement("div")
191+
div.innerHTML = `
192+
<div data-bind="dotvvm-with-control-properties: {
193+
Arr: ComplexObj()?.B
194+
}"> <span id=x /> </div>
195+
`
196+
ko.applyBindings(dotvvm.viewModelObservables.root, div)
197+
const x = div.querySelector("#x")!
198+
const context: any = ko.contextFor(x).$control
199+
expect(context.Arr).observable()
200+
expect(context.Arr.state).toEqual(undefined)
201+
expect(context.Arr()).toEqual(undefined)
202+
expect("push" in context.Arr).toBe(false)
203+
204+
dotvvm.patchState({ ComplexObj: { B: [1, 2] } })
205+
dotvvm.rootStateManager.doUpdateNow()
206+
207+
expect(context.Arr()?.map((x: any) => x())).toStrictEqual([1, 2])
208+
expect(context.Arr.state).toStrictEqual([1, 2])
209+
expect("push" in context.Arr).toBe(true)
210+
211+
context.Arr.push(3)
212+
expect(context.Arr()?.map((x:any) => x())).toStrictEqual([1, 2, 3])
213+
expect(dotvvm.state.ComplexObj.B).toStrictEqual([1, 2, 3])
214+
215+
context.Arr.unshift(0)
216+
context.Arr.splice(1, 1)
217+
218+
expect(dotvvm.state.ComplexObj.B).toStrictEqual([0, 2, 3])
219+
})

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

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -94,10 +94,7 @@ export function wrapObservable(func: () => any, isArray?: boolean): KnockoutComp
9494
});
9595

9696
if (isArray) {
97-
for (const i of ["push", "pop", "unshift", "shift", "reverse", "sort", "splice", "slice", "replace", "indexOf", "remove", "removeAll"]) {
98-
(wrapper as any)[i] = (...args: any) => updateObservableArray(func, i, args);
99-
}
100-
wrapper = wrapper.extend({ trackArrayChanges: true });
97+
proxyObservableArrayMethods(wrapper, () => getExpressionResult(func))
10198
}
10299

103100
return wrapper.extend({ notify: "always" });
@@ -113,16 +110,20 @@ function updateObservable(getObservable: () => KnockoutObservable<any>, value: a
113110
}
114111
}
115112

116-
function updateObservableArray(getObservableArray: () => KnockoutObservableArray<any>, fnName: string, args: any[]) {
117-
const result = getExpressionResult(getObservableArray);
118-
119-
if (!isObservableArray(result)) {
120-
logError("validation", `Cannot execute '${fnName}' function on ko.computed because the expression '${getObservableArray}' does not return an observable array.`);
121-
} else {
122-
result[fnName].apply(result, args);
113+
export function proxyObservableArrayMethods(wrapper: KnockoutObservable<any>, getObservableArray: () => any) {
114+
for (const fnName of ["push", "pop", "unshift", "shift", "reverse", "sort", "splice", "slice", "replace", "indexOf", "remove", "removeAll"]) {
115+
(wrapper as any)[fnName] = (...args: any) => {
116+
const result = getExpressionResult(getObservableArray)
117+
118+
if (!isObservableArray(result)) {
119+
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')
120+
} else {
121+
return result[fnName](...args)
122+
}
123+
}
123124
}
125+
wrapper.extend({ trackArrayChanges: true })
124126
}
125-
126127
export const unwrapComputedProperty = (obs: any) =>
127128
ko.isComputed(obs) && "wrappedProperty" in obs ?
128129
(obs as any)["wrappedProperty"]() : // workaround for dotvvm-with-control-properties handler

0 commit comments

Comments
 (0)