From fd0fb5be008c69f2ca3a0fb6298543ec0ef39d3d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= Date: Tue, 20 Aug 2019 15:34:13 +0200 Subject: [PATCH] [REF] observer: refactor observer using proxy closes #253 --- src/component/component.ts | 4 +- src/core/observer.ts | 206 ++++---------- src/store/connected_component.ts | 29 +- src/store/store.ts | 3 +- tests/core/observer.test.ts | 353 ++++++++++++++---------- tests/store/connected_component.test.ts | 38 ++- tests/store/store.test.ts | 5 +- tools/benchmarks/owl-master/app.js | 3 +- 8 files changed, 294 insertions(+), 347 deletions(-) diff --git a/src/component/component.ts b/src/component/component.ts index a53298a1..224f643f 100644 --- a/src/component/component.ts +++ b/src/component/component.ts @@ -506,7 +506,7 @@ export class Component { return this.__render(false, [], scope, vars); } - async __render( + __render( force: boolean = false, patchQueue: any[] = [], scope?: Object, @@ -581,7 +581,7 @@ export class Component { if (this.state) { const __owl__ = this.__owl__; __owl__.observer = new Observer(); - __owl__.observer.observe(this.state); + this.state = __owl__.observer.observe(this.state); __owl__.observer.notifyCB = this.render.bind(this); } } diff --git a/src/core/observer.ts b/src/core/observer.ts index 40d2a931..316c8c17 100644 --- a/src/core/observer.ts +++ b/src/core/observer.ts @@ -7,59 +7,13 @@ * This is a Observer class that can observe any JS values. The way it works * can be summarized thusly: * - primitive values are not observed at all - * - Objects are observed by replacing all their keys with getters/setters - * (recursively) - * - Arrays are observed by replacing their prototype with a customized version, - * which wrap some methods to allow the tracking of each state change. + * - Objects and arrays are observed by replacing them with a Proxy + * - each object/array metadata are tracked in a weakmap, and keep a revision + * number * - * Note that this code is inspired by Vue. + * Note that this code is loosely inspired by Vue. */ -//------------------------------------------------------------------------------ -// Modified Array prototype -//------------------------------------------------------------------------------ - -// we define here a new modified Array prototype, which basically override all -// Array methods that change some state to be able to track their changes -const methodsToPatch = ["push", "pop", "shift", "unshift", "splice", "sort", "reverse"]; -const methodLen = methodsToPatch.length; - -const ArrayProto = Array.prototype; -const ModifiedArrayProto = Object.create(ArrayProto); - -for (let i = 0; i < methodLen; i++) { - const method = methodsToPatch[i]; - const initialMethod = ArrayProto[method]; - ModifiedArrayProto[method] = function(...args) { - if (!this.__observer__.allowMutations) { - throw new Error(`Array cannot be changed here")`); - } - this.__observer__.rev++; - this.__observer__.notifyChange(); - this.__owl__.rev++; - let parent = this; - do { - parent.__owl__.deepRev++; - } while ((parent = parent.__owl__.parent)); - let inserted; - switch (method) { - case "push": - case "unshift": - inserted = args; - break; - case "splice": - inserted = args.slice(2); - break; - } - if (inserted) { - for (let i = 0, iLen = inserted.length; i < iLen; i++) { - this.__observer__.observe(inserted[i], this); - } - } - return initialMethod.call(this, ...args); - }; -} - //------------------------------------------------------------------------------ // Observer //------------------------------------------------------------------------------ @@ -67,128 +21,90 @@ export class Observer { rev: number = 1; allowMutations: boolean = true; dirty: boolean = false; - - static set(target: any, key: number | string, value: any) { - if (!target.__owl__) { - throw Error("`Observer.set()` can only be called with observed Objects or Arrays"); - } - target.__owl__.observer.set(target, key, value); - } - - static delete(target: any, key: number | string) { - if (!target.__owl__) { - throw Error("`Observer.delete()` can only be called with observed Objects"); - } - target.__owl__.observer.delete(target, key); - } + weakMap: WeakMap = new WeakMap(); notifyCB() {} - notifyChange() { + async notifyChange() { + this.dirty = true; - Promise.resolve().then(() => { - if (this.dirty) { - this.dirty = false; - this.notifyCB(); - } - }); + await Promise.resolve(); + if (this.dirty) { + this.dirty = false; + this.notifyCB(); + } } - observe(value: any, parent?: any) { - if (value === null) { + observe(value: any, parent?: any): any { + if (value === null || typeof value !== "object") { // fun fact: typeof null === 'object' - return; - } - if (typeof value !== "object") { - return; - } - if ("__owl__" in value) { - // already observed - value.__owl__.parent = parent; - return; - } - if (Array.isArray(value)) { - this._observeArr(value, parent); - } else { - this._observeObj(value, parent); + return value; } + let metadata = this.weakMap.get(value) || this._observe(value, parent); + return metadata.proxy; } - set(target: any, key: number | string, value: any) { - let alreadyDefined = - key in target && Object.getOwnPropertyDescriptor(target, key)!.configurable === false; - if (alreadyDefined) { - target[key] = value; - } else { - this._addProp(target, key, value); - this._updateRevNumber(target); - } - this.notifyChange(); + revNumber(value): number { + const metadata = this.weakMap.get(value); + return metadata ? metadata.rev : 0; + } + deepRevNumber(value): number { + const metadata = this.weakMap.get(value); + return metadata ? metadata.deepRev : 0; } - delete(target: any, key: number | string) { - delete target[key]; - this._updateRevNumber(target); - this.notifyChange(); - } - - _observeObj(obj: T, parent?: any) { - obj.__owl__ = { - rev: this.rev, - deepRev: this.rev, - parent, - observer: this - }; - Object.defineProperty(obj, "__owl__", { enumerable: false }); - for (let key in obj) { - this._addProp(obj, key, obj[key]); - } - } - - _observeArr(arr: Array, parent?: any) { - (arr).__owl__ = { - rev: this.rev, - deepRev: this.rev, - parent, - observer: this - }; - Object.defineProperty(arr, "__owl__", { enumerable: false }); - (arr).__proto__ = Object.create(ModifiedArrayProto); - (arr).__proto__.__observer__ = this; - for (let i = 0, iLen = arr.length; i < iLen; i++) { - this.observe(arr[i], arr); - } - } - - _addProp(obj: T, key: string | number, value: any) { + _observe(value, parent) { var self = this; - Object.defineProperty(obj, key, { - configurable: true, - enumerable: true, - get() { - return value; + + const proxy = new Proxy(value, { + get(target, k) { + const targetValue = target[k]; + return self.observe(targetValue, value); }, - set(newVal) { + set(target, key: string, newVal): boolean { + const value = target[key]; if (newVal !== value) { if (!self.allowMutations) { throw new Error( `Observed state cannot be changed here! (key: "${key}", val: "${newVal}")` ); } - self._updateRevNumber(obj); - value = newVal; - self.observe(newVal, obj); + self._updateRevNumber(target); + target[key] = newVal; self.notifyChange(); } + return true; + }, + deleteProperty(target, key) { + if (key in target) { + delete target[key]; + self._updateRevNumber(target); + self.notifyChange(); + } + return true; } }); - this.observe(value, obj); + + const metadata = { + value, + proxy, + rev: this.rev, + deepRev: this.rev, + parent + }; + + this.weakMap.set(value, metadata); + this.weakMap.set(metadata.proxy, metadata); + return metadata; } + _updateRevNumber(target: any) { this.rev++; - target.__owl__.rev!++; + let metadata = this.weakMap.get(target); + metadata.rev!++; let parent = target; do { - parent.__owl__.deepRev++; - } while ((parent = parent.__owl__.parent) && parent !== target); + metadata = this.weakMap.get(parent); + metadata.deepRev++; + } while ((parent = metadata.parent) && parent !== target); } } diff --git a/src/store/connected_component.ts b/src/store/connected_component.ts index 00ab3947..8033fc3f 100644 --- a/src/store/connected_component.ts +++ b/src/store/connected_component.ts @@ -4,20 +4,6 @@ import { Component, Env } from "../component/component"; // Connect function //------------------------------------------------------------------------------ -function revNumber(o: T): number { - if (o !== null && typeof o === "object" && (o).__owl__) { - return (o).__owl__.rev; - } - return 0; -} - -function deepRevNumber(o: T): number { - if (o !== null && typeof o === "object" && (o).__owl__) { - return (o).__owl__.deepRev; - } - return 0; -} - type HashFunction = (a: any, b: any) => number; export class ConnectedComponent extends Component { @@ -27,15 +13,16 @@ export class ConnectedComponent extends Component } hashFunction: HashFunction = ({ storeProps }, options) => { - let refFunction = this.deep ? deepRevNumber : revNumber; + const observer = (this.__owl__ as any).store.observer; + let refFunction = this.deep ? observer.deepRevNumber : observer.revNumber; if ("__owl__" in storeProps) { - return refFunction(storeProps); + return refFunction.call(observer, storeProps); } const { currentStoreProps } = options; let hash = 0; for (let key in storeProps) { const val = storeProps[key]; - const hashVal = refFunction(val); + const hashVal = refFunction.call(observer, val); if (hashVal === 0) { if (val !== currentStoreProps[key]) { options.didChange = true; @@ -70,9 +57,7 @@ export class ConnectedComponent extends Component (this.__owl__).storeHash = this.hashFunction( { state: store.state, - storeProps: storeProps, - revNumber, - deepRevNumber + storeProps: storeProps }, { currentStoreProps: storeProps @@ -109,9 +94,7 @@ export class ConnectedComponent extends Component const storeHash = this.hashFunction( { state: (this.__owl__).store.state, - storeProps: storeProps, - revNumber, - deepRevNumber + storeProps: storeProps }, options ); diff --git a/src/store/store.ts b/src/store/store.ts index c5ba3f50..bce38e7f 100644 --- a/src/store/store.ts +++ b/src/store/store.ts @@ -53,14 +53,13 @@ export class Store extends EventBus { constructor(config: StoreConfig, options: StoreOption = {}) { super(); this.debug = options.debug || false; - this.state = config.state || {}; this.actions = config.actions; this.mutations = config.mutations; this.env = config.env; this.observer = new Observer(); this.observer.notifyCB = this.__notifyComponents.bind(this); this.observer.allowMutations = false; - this.observer.observe(this.state); + this.state = this.observer.observe(config.state || {}); this.getters = {}; this._gettersCache = {}; diff --git a/tests/core/observer.test.ts b/tests/core/observer.test.ts index 92defaab..9506d70b 100644 --- a/tests/core/observer.test.ts +++ b/tests/core/observer.test.ts @@ -4,307 +4,362 @@ import { nextMicroTick } from "../helpers"; describe("observer", () => { test("properly observe objects", () => { const observer = new Observer(); - const obj: any = {}; + const obj: any = observer.observe({}); - observer.observe(obj); - expect(obj.__owl__.rev).toBe(1); + expect(typeof obj).toBe("object"); + expect(observer.revNumber(obj)).toBe(1); + expect(observer.deepRevNumber(obj)).toBe(1); expect(observer.rev).toBe(1); - const ob2: any = { a: 1 }; - observer.observe(ob2); - expect(ob2.__owl__.rev).toBe(1); - ob2.a = 2; - expect(observer.rev).toBe(2); - expect(ob2.__owl__.rev).toBe(2); + const obj2: any = observer.observe({ a: 1 }); + expect(observer.revNumber(obj2)).toBe(1); + expect(observer.revNumber(obj)).toBe(1); + expect(observer.deepRevNumber(obj)).toBe(1); + expect(observer.rev).toBe(1); - ob2.b = 3; - expect(observer.rev).toBe(2); - expect(ob2.__owl__.rev).toBe(2); + obj2.a = 2; - observer.set(ob2, "b", 4); - expect(observer.rev).toBe(3); - expect(ob2.__owl__.rev).toBe(3); + expect(observer.revNumber(obj)).toBe(1); + expect(observer.revNumber(obj2)).toBe(2); + expect(observer.rev).toBe(2); + + expect(obj2).toEqual({ + a: 2 + }); }); test("properly handle null or undefined", () => { const observer = new Observer(); - const obj: any = { a: null, b: undefined }; + const obj: any = observer.observe({ a: null, b: undefined }); - observer.observe(obj); - expect(obj.__owl__.rev).toBe(1); + expect(observer.revNumber(obj)).toBe(1); + expect(observer.deepRevNumber(obj)).toBe(1); expect(observer.rev).toBe(1); obj.a = 3; - expect(obj.__owl__.rev).toBe(2); + expect(observer.revNumber(obj)).toBe(2); + expect(observer.deepRevNumber(obj)).toBe(2); + expect(observer.rev).toBe(2); obj.b = 5; - expect(obj.__owl__.rev).toBe(3); + expect(observer.revNumber(obj)).toBe(3); + expect(observer.deepRevNumber(obj)).toBe(3); + expect(observer.rev).toBe(3); obj.a = null; obj.b = undefined; - expect(obj.__owl__.rev).toBe(5); + expect(observer.revNumber(obj)).toBe(5); + expect(observer.deepRevNumber(obj)).toBe(5); + expect(observer.rev).toBe(5); + expect(obj).toEqual({ + a: null, + b: undefined + }); }); test("can change values in array", () => { const observer = new Observer(); - const obj: any = { arr: [1, 2] }; + const obj: any = observer.observe({ arr: [1, 2] }); - observer.observe(obj); - expect(obj.arr.__owl__.rev).toBe(1); + expect(Array.isArray(obj.arr)).toBe(true); + expect(observer.revNumber(obj.arr)).toBe(1); + expect(observer.deepRevNumber(obj.arr)).toBe(1); expect(observer.rev).toBe(1); obj.arr[0] = "nope"; - expect(obj.arr.__owl__.rev).toBe(1); - expect(observer.rev).toBe(1); - - observer.set(obj.arr, 0, "yep"); - expect(obj.arr.__owl__.rev).toBe(2); + expect(observer.revNumber(obj.arr)).toBe(2); + expect(observer.deepRevNumber(obj.arr)).toBe(2); + expect(observer.revNumber(obj)).toBe(1); + expect(observer.deepRevNumber(obj)).toBe(2); expect(observer.rev).toBe(2); }); test("various object property changes", () => { const observer = new Observer(); - const obj: any = { a: 1 }; - observer.observe(obj); - expect(obj.__owl__.rev).toBe(1); + const obj: any = observer.observe({ a: 1 }); + + expect(observer.revNumber(obj)).toBe(1); + expect(observer.deepRevNumber(obj)).toBe(1); + expect(observer.rev).toBe(1); + obj.a = 2; + + expect(observer.revNumber(obj)).toBe(2); + expect(observer.deepRevNumber(obj)).toBe(2); expect(observer.rev).toBe(2); - expect(obj.__owl__.rev).toBe(2); // same value again obj.a = 2; + expect(observer.revNumber(obj)).toBe(2); + expect(observer.deepRevNumber(obj)).toBe(2); expect(observer.rev).toBe(2); - expect(obj.__owl__.rev).toBe(2); obj.a = 3; + expect(observer.revNumber(obj)).toBe(3); + expect(observer.deepRevNumber(obj)).toBe(3); expect(observer.rev).toBe(3); - expect(obj.__owl__.rev).toBe(3); }); test("properly observe arrays", () => { const observer = new Observer(); - const arr: any = []; - observer.observe(arr); - expect(arr.__owl__.rev).toBe(1); - expect(observer.rev).toBe(1); + const arr: any = observer.observe([]); + + expect(Array.isArray(arr)).toBe(true); expect(arr.length).toBe(0); + expect(observer.revNumber(arr)).toBe(1); + expect(observer.deepRevNumber(arr)).toBe(1); + expect(observer.rev).toBe(1); arr.push(1); - expect(arr.__owl__.rev).toBe(2); + expect(observer.revNumber(arr)).toBe(2); + expect(observer.deepRevNumber(arr)).toBe(2); expect(observer.rev).toBe(2); expect(arr.length).toBe(1); + expect(arr).toEqual([1]); arr.splice(1, 0, "hey"); - expect(arr.__owl__.rev).toBe(3); + expect(observer.revNumber(arr)).toBe(3); + expect(observer.deepRevNumber(arr)).toBe(3); expect(observer.rev).toBe(3); + expect(arr).toEqual([1, "hey"]); expect(arr.length).toBe(2); arr.unshift("lindemans"); - expect(arr.__owl__.rev).toBe(4); + //it generates 3 primitive operations + expect(observer.revNumber(arr)).toBe(6); + expect(observer.deepRevNumber(arr)).toBe(6); + expect(observer.rev).toBe(6); + expect(arr).toEqual(["lindemans", 1, "hey"]); + expect(arr.length).toBe(3); arr.reverse(); - expect(arr.__owl__.rev).toBe(5); + //it generates 2 primitive operations + expect(observer.revNumber(arr)).toBe(8); + expect(observer.deepRevNumber(arr)).toBe(8); + expect(observer.rev).toBe(8); + expect(arr).toEqual(["hey", 1, "lindemans"]); + expect(arr.length).toBe(3); - arr.pop(); - expect(arr.__owl__.rev).toBe(6); - - arr.shift(); - expect(arr.__owl__.rev).toBe(7); - - arr.sort(); - expect(arr.__owl__.rev).toBe(8); + arr.pop(); // one set, one delete + expect(observer.revNumber(arr)).toBe(10); + expect(observer.deepRevNumber(arr)).toBe(10); + expect(observer.rev).toBe(10); + expect(arr).toEqual(["hey", 1]); + expect(arr.length).toBe(2); + arr.shift(); // 2 sets, 1 delete + expect(observer.revNumber(arr)).toBe(13); + expect(observer.deepRevNumber(arr)).toBe(13); + expect(observer.rev).toBe(13); expect(arr).toEqual([1]); + expect(arr.length).toBe(1); }); test("object pushed into arrays are observed", () => { const observer = new Observer(); - const arr: any = []; - observer.observe(arr); + const arr: any = observer.observe([]); expect(observer.rev).toBe(1); arr.push({ kriek: 5 }); expect(observer.rev).toBe(2); - expect(arr.__owl__.rev).toBe(2); - expect(arr[0].__owl__.rev).toBe(2); + expect(observer.revNumber(arr)).toBe(2); + expect(observer.revNumber(arr[0])).toBe(2); arr[0].kriek = 6; + expect(observer.rev).toBe(3); - expect(arr.__owl__.rev).toBe(2); - expect(arr[0].__owl__.rev).toBe(3); + expect(observer.revNumber(arr)).toBe(2); + expect(observer.deepRevNumber(arr)).toBe(3); + expect(observer.revNumber(arr[0])).toBe(3); }); test("set new property on observed object", async () => { const observer = new Observer(); observer.notifyCB = jest.fn(); - const state: any = { a: 1 }; - observer.observe(state); - expect(state.__owl__.rev).toBe(1); - expect(observer.rev).toBe(1); + const state: any = observer.observe({ a: 1 }); expect(observer.notifyCB).toBeCalledTimes(0); + expect(observer.rev).toBe(1); + expect(observer.revNumber(state)).toBe(1); + + state.b = 8; - Observer.set(state, "b", 8); await nextMicroTick(); - expect(state.__owl__.rev).toBe(2); - expect(observer.rev).toBe(2); + expect(observer.notifyCB).toBeCalledTimes(1); + expect(observer.rev).toBe(2); + expect(observer.revNumber(state)).toBe(2); + expect(state.b).toBe(8); }); test("delete property from observed object", async () => { const observer = new Observer(); observer.notifyCB = jest.fn(); - const state: any = { a: 1, b: 8 }; + const state: any = observer.observe({ a: 1, b: 8 }); observer.observe(state); - expect(state.__owl__.rev).toBe(1); - expect(observer.rev).toBe(1); - expect(observer.notifyCB).toBeCalledTimes(0); - Observer.delete(state, "b"); + expect(observer.notifyCB).toBeCalledTimes(0); + expect(observer.rev).toBe(1); + expect(observer.revNumber(state)).toBe(1); + + delete state.b; await nextMicroTick(); - expect(state.__owl__.rev).toBe(2); - expect(observer.rev).toBe(2); + expect(observer.notifyCB).toBeCalledTimes(1); + expect(observer.rev).toBe(2); + expect(observer.revNumber(state)).toBe(2); + expect(state).toEqual({ a: 1 }); }); test("set element in observed array", async () => { const observer = new Observer(); observer.notifyCB = jest.fn(); - const state: any = ["a"]; - observer.observe(state); - expect(state.__owl__.rev).toBe(1); + const state: any = observer.observe(["a"]); + expect(observer.rev).toBe(1); + expect(observer.revNumber(state)).toBe(1); + expect(observer.deepRevNumber(state)).toBe(1); expect(observer.notifyCB).toBeCalledTimes(0); - Observer.set(state, 1, "b"); + state[1] = "b"; + await nextMicroTick(); - expect(state.__owl__.rev).toBe(2); + expect(observer.rev).toBe(2); + expect(observer.revNumber(state)).toBe(2); + expect(observer.deepRevNumber(state)).toBe(2); expect(observer.notifyCB).toBeCalledTimes(1); + expect(state).toEqual(["a", "b"]); }); test("properly observe arrays in object", () => { const observer = new Observer(); - const state: any = { arr: [] }; - observer.observe(state); - expect(state.arr.__owl__.rev).toBe(1); + const state: any = observer.observe({ arr: [] }); + expect(observer.rev).toBe(1); + expect(observer.revNumber(state.arr)).toBe(1); + expect(observer.deepRevNumber(state.arr)).toBe(1); expect(state.arr.length).toBe(0); state.arr.push(1); - expect(state.arr.__owl__.rev).toBe(2); expect(observer.rev).toBe(2); + expect(observer.revNumber(state.arr)).toBe(2); + expect(observer.deepRevNumber(state.arr)).toBe(2); expect(state.arr.length).toBe(1); }); test("properly observe objects in array", () => { const observer = new Observer(); - const state: any = { arr: [{ something: 1 }] }; + const state: any = observer.observe({ arr: [{ something: 1 }] }); observer.observe(state); - expect(state.arr.__owl__.rev).toBe(1); - expect(state.arr[0].__owl__.rev).toBe(1); + + expect(observer.rev).toBe(1); + expect(observer.revNumber(state.arr)).toBe(1); + expect(observer.revNumber(state.arr[0])).toBe(1); state.arr[0].something = 2; - expect(state.arr.__owl__.rev).toBe(1); - expect(state.arr[0].__owl__.rev).toBe(2); + expect(observer.rev).toBe(2); + expect(observer.revNumber(state.arr)).toBe(1); + expect(observer.revNumber(state.arr[0])).toBe(2); }); test("properly observe objects in object", () => { const observer = new Observer(); - const state: any = { a: { b: 1 } }; - observer.observe(state); - expect(state.__owl__.rev).toBe(1); - expect(state.a.__owl__.rev).toBe(1); + const state: any = observer.observe({ a: { b: 1 } }); + + expect(observer.rev).toBe(1); + expect(observer.revNumber(state)).toBe(1); + expect(observer.revNumber(state.a)).toBe(1); state.a.b = 2; - expect(state.__owl__.rev).toBe(1); - expect(state.a.__owl__.rev).toBe(2); + expect(observer.rev).toBe(2); + expect(observer.revNumber(state)).toBe(1); + expect(observer.revNumber(state.a)).toBe(2); }); test("reobserve new object values", () => { const observer = new Observer(); - const obj: any = { a: 1 }; - observer.observe(obj); - expect(obj.__owl__.rev).toBe(1); + const obj: any = observer.observe({ a: 1 }); + + expect(observer.rev).toBe(1); + expect(observer.revNumber(obj)).toBe(1); + obj.a = { b: 2 }; + expect(observer.rev).toBe(2); - expect(obj.__owl__.rev).toBe(2); - - // we start at 2 because it is the new observer rev number - expect(obj.a.__owl__.rev).toBe(2); - + expect(observer.revNumber(obj)).toBe(2); + expect(observer.revNumber(obj.a)).toBe(2); obj.a.b = 3; expect(observer.rev).toBe(3); - expect(obj.__owl__.rev).toBe(2); - expect(obj.a.__owl__.rev).toBe(3); + expect(observer.revNumber(obj)).toBe(2); + expect(observer.revNumber(obj.a)).toBe(3); }); test("deep observe misc changes", () => { const observer = new Observer(); - const state: any = { o: { a: 1 }, arr: [1], n: 13 }; - observer.observe(state); - expect(state.__owl__.rev).toBe(1); - expect(state.__owl__.deepRev).toBe(1); + const state: any = observer.observe({ o: { a: 1 }, arr: [1], n: 13 }); + expect(observer.revNumber(state)).toBe(1); + expect(observer.deepRevNumber(state)).toBe(1); state.o.a = 2; expect(observer.rev).toBe(2); - expect(state.__owl__.rev).toBe(1); - expect(state.__owl__.deepRev).toBe(2); + expect(observer.revNumber(state)).toBe(1); + expect(observer.deepRevNumber(state)).toBe(2); state.arr.push(2); - expect(state.__owl__.rev).toBe(1); - expect(state.__owl__.deepRev).toBe(3); + expect(observer.rev).toBe(3); + expect(observer.revNumber(state)).toBe(1); + expect(observer.deepRevNumber(state)).toBe(3); state.n = 155; - expect(state.__owl__.rev).toBe(2); - expect(state.__owl__.deepRev).toBe(4); + expect(observer.rev).toBe(4); + expect(observer.revNumber(state)).toBe(2); + expect(observer.deepRevNumber(state)).toBe(4); }); test("properly handle already observed state", () => { const observer = new Observer(); - const obj1: any = { a: 1 }; - const obj2: any = { b: 1 }; - observer.observe(obj1); - observer.observe(obj2); - expect(obj1.__owl__.rev).toBe(1); - expect(obj2.__owl__.rev).toBe(1); + const obj1: any = observer.observe({ a: 1 }); + const obj2: any = observer.observe({ b: 1 }); + + expect(observer.revNumber(obj1)).toBe(1); + expect(observer.revNumber(obj2)).toBe(1); obj1.a = 2; obj2.b = 3; - expect(obj1.__owl__.rev).toBe(2); - expect(obj2.__owl__.rev).toBe(2); + expect(observer.revNumber(obj1)).toBe(2); + expect(observer.revNumber(obj2)).toBe(2); obj2.b = obj1; - expect(obj1.__owl__.rev).toBe(2); - expect(obj2.__owl__.rev).toBe(3); + expect(observer.revNumber(obj1)).toBe(2); + expect(observer.revNumber(obj2)).toBe(3); }); test("can set a property more than once", () => { const observer = new Observer(); - const obj: any = {}; + const obj: any = observer.observe({}); - observer.observe(obj); - expect(obj.__owl__.rev).toBe(1); + expect(observer.revNumber(obj)).toBe(1); + expect(observer.deepRevNumber(obj)).toBe(1); expect(observer.rev).toBe(1); - expect(obj.__owl__.deepRev).toBe(1); - observer.set(obj, "aku", "always finds annoying problems"); + obj.aku = "always finds annoying problems"; + expect(observer.revNumber(obj)).toBe(2); + expect(observer.deepRevNumber(obj)).toBe(2); expect(observer.rev).toBe(2); - expect(obj.__owl__.rev).toBe(2); - expect(obj.__owl__.deepRev).toBe(2); - observer.set(obj, "aku", "always finds good problems"); + obj.aku = "always finds good problems"; + + expect(observer.revNumber(obj)).toBe(3); + expect(observer.deepRevNumber(obj)).toBe(3); expect(observer.rev).toBe(3); - expect(obj.__owl__.rev).toBe(3); - expect(obj.__owl__.deepRev).toBe(3); }); test("properly handle swapping elements", () => { const observer = new Observer(); - const obj: any = { a: { arr: [] }, b: 1 }; - observer.observe(obj); + const obj: any = observer.observe({ a: { arr: [] }, b: 1 }); // swap a and b const b = obj.b; @@ -319,9 +374,9 @@ describe("observer", () => { test("properly handle assigning observed obj containing array", () => { const observer = new Observer(); - const obj: any = { a: { arr: [], val: "test" } }; - observer.observe(obj); + const obj: any = observer.observe({ a: { arr: [], val: "test" } }); + expect(observer.rev).toBe(1); obj.a = { ...obj.a, val: "test2" }; expect(observer.rev).toBe(2); @@ -332,24 +387,25 @@ describe("observer", () => { test("accept cycles in observed state", () => { const observer = new Observer(); - const obj1: any = {}; - const obj2: any = { b: obj1, key: 1 }; + let obj1: any = {}; + let obj2: any = { b: obj1, key: 1 }; obj1.a = obj2; - observer.observe(obj1); - expect(obj1.__owl__.rev).toBe(1); - expect(obj2.__owl__.rev).toBe(1); + obj1 = observer.observe(obj1); + obj2 = obj1.a; + + expect(observer.revNumber(obj1)).toBe(1); + expect(observer.revNumber(obj2)).toBe(1); obj2.key = 3; - expect(obj1.__owl__.rev).toBe(1); - expect(obj2.__owl__.rev).toBe(2); + expect(observer.revNumber(obj1)).toBe(1); + expect(observer.revNumber(obj2)).toBe(2); }); test("call callback when state is changed", async () => { const observer = new Observer(); observer.notifyCB = jest.fn(); - const obj: any = { a: 1, b: { c: 2 }, d: [{ e: 3 }], f: 4 }; + const obj: any = observer.observe({ a: 1, b: { c: 2 }, d: [{ e: 3 }], f: 4 }); - observer.observe(obj); expect(observer.notifyCB).toBeCalledTimes(0); obj.a = 2; @@ -373,9 +429,7 @@ describe("observer", () => { test("throw error when state is mutated in object if allowMutation=false", async () => { const observer = new Observer(); observer.allowMutations = false; - const obj: any = { a: 1 }; - observer.observe(obj); - + const obj: any = observer.observe({ a: 1 }); expect(() => { obj.a = 2; }).toThrow('Observed state cannot be changed here! (key: "a", val: "2")'); @@ -384,11 +438,10 @@ describe("observer", () => { test("throw error when state is mutated in array if allowMutation=false", async () => { const observer = new Observer(); observer.allowMutations = false; - const obj: any = { a: [1] }; - observer.observe(obj); + const obj: any = observer.observe({ a: [1] }); expect(() => { obj.a.push(2); - }).toThrow("Array cannot be changed here"); + }).toThrow('Observed state cannot be changed here! (key: "1", val: "2")'); }); }); diff --git a/tests/store/connected_component.test.ts b/tests/store/connected_component.test.ts index 106dcba8..dad0c20b 100644 --- a/tests/store/connected_component.test.ts +++ b/tests/store/connected_component.test.ts @@ -1,11 +1,8 @@ -import { core } from "../../src"; import { Component, Env } from "../../src/component/component"; import { ConnectedComponent } from "../../src/store/connected_component"; import { Store } from "../../src/store/store"; import { makeTestEnv, makeTestFixture, nextTick } from "../helpers"; -const Observer = core.Observer; - describe("connecting a component to store", () => { let fixture: HTMLElement; let env: Env; @@ -644,7 +641,7 @@ describe("connecting a component to store", () => { test("connected parent/children: no rendering if child is destroyed", async () => { const mutations = { removeTodo({ state }) { - Observer.delete(state.todos, 1); + delete state.todos[1]; } }; const todos = { 1: { id: 1, title: "kikoou" } }; @@ -760,7 +757,6 @@ describe("connecting a component to store", () => { }); }); - describe("connected components and default values", () => { let fixture: HTMLElement; let env: Env; @@ -795,7 +791,7 @@ describe("connected components and default values", () => { const app = new App(env); await app.mount(fixture); - expect(fixture.innerHTML).toBe('
Hello, John
'); + expect(fixture.innerHTML).toBe("
Hello, John
"); }); test("can set default values", async () => { @@ -819,13 +815,13 @@ describe("connected components and default values", () => { const app = new App(env); await app.mount(fixture); - expect(fixture.innerHTML).toBe('
Hello, John
'); + expect(fixture.innerHTML).toBe("
Hello, John
"); await app.__updateProps({ initialRecipient: "James" }, true); - expect(fixture.innerHTML).toBe('
Hello, James
'); + expect(fixture.innerHTML).toBe("
Hello, James
"); await app.__updateProps({ initialRecipient: undefined }, true); - expect(fixture.innerHTML).toBe('
Hello, John
'); + expect(fixture.innerHTML).toBe("
Hello, John
"); }); test("can set default values (v2)", async () => { @@ -846,9 +842,9 @@ describe("connected components and default values", () => { class Message extends ConnectedComponent { static defaultProps = { showId: true }; - static mapStoreToProps = function (state, ownProps) { + static mapStoreToProps = function(state, ownProps) { return { - message: state.messages[ownProps.messageId], + message: state.messages[ownProps.messageId] }; }; } @@ -856,10 +852,10 @@ describe("connected components and default values", () => { class Thread extends ConnectedComponent { components = { Message }; static defaultProps = { showMessages: true }; - static mapStoreToProps = function (state, ownProps) { + static mapStoreToProps = function(state, ownProps) { const thread = state.threads[ownProps.threadId]; return { - thread, + thread }; }; } @@ -872,11 +868,11 @@ describe("connected components and default values", () => { const state = { threads: { 1: { - messages: [100, 101], + messages: [100, 101] }, 2: { - messages: [200], - }, + messages: [200] + } }, messages: { 100: { @@ -902,13 +898,15 @@ describe("connected components and default values", () => { const app = new App(env); await app.mount(fixture); - expect(fixture.innerHTML).toBe('
100Message100
101Message101
'); + expect(fixture.innerHTML).toBe( + "
100Message100
101Message101
" + ); await app.__updateProps({ threadId: 2 }, true); - expect(fixture.innerHTML).toBe('
200Message200
'); + expect(fixture.innerHTML).toBe("
200Message200
"); - store.commit('changeMessageContent', 200, "UpdatedMessage200"); + store.commit("changeMessageContent", 200, "UpdatedMessage200"); await nextTick(); - expect(fixture.innerHTML).toBe('
200UpdatedMessage200
'); + expect(fixture.innerHTML).toBe("
200UpdatedMessage200
"); }); }); diff --git a/tests/store/store.test.ts b/tests/store/store.test.ts index 28f125f2..b83ff3c4 100644 --- a/tests/store/store.test.ts +++ b/tests/store/store.test.ts @@ -474,12 +474,11 @@ describe("basic use", () => { describe("advanced state properties", () => { test("state in the store is reference equal after mutation", async () => { - const state = {}; const mutations = { donothing() {} }; - const store = new Store({ state, mutations }); - expect(store.state).toBe(state); + const store = new Store({ state:{}, mutations }); + const state = store.state; store.commit("donothing"); expect(store.state).toBe(state); }); diff --git a/tools/benchmarks/owl-master/app.js b/tools/benchmarks/owl-master/app.js index 709d01ed..dd4f1cb6 100644 --- a/tools/benchmarks/owl-master/app.js +++ b/tools/benchmarks/owl-master/app.js @@ -107,12 +107,11 @@ class App extends owl.Component { updateSomeMessages() { this.benchmark("update every 10th", () => { - const setState = owl.core.Observer.set; const messages = this.state.messages; for (let i = 0; i < messages.length; i += 10) { const msg = Object.assign({}, messages[i]); msg.author += "!!!"; - setState(messages, i, msg); + messages[i] = msg; } }); }