From d1094f647a527337e7e192df5f4f5f56e91a4fbd Mon Sep 17 00:00:00 2001 From: Aaron Bohy Date: Thu, 6 Jun 2019 09:08:51 +0200 Subject: [PATCH] [IMP] observer: add set and delete static functions This rev. add a 'set' and 'delete' static functions to the Observer. They allow to set or delete (only for objects) properties on observed objects or arrays. With this, the 'set' functions in Store and Component are no longer necessary, so they have been removed. Closes #138 --- doc/component.md | 5 ----- doc/observer.md | 38 ++++++++++++++----------------- src/component.ts | 8 ------- src/observer.ts | 39 ++++++++++++++++++++++++++++++-- src/store.ts | 5 +---- tests/observer.test.ts | 51 ++++++++++++++++++++++++++++++++++++++++++ tests/store.test.ts | 18 --------------- 7 files changed, 106 insertions(+), 58 deletions(-) diff --git a/doc/component.md b/doc/component.md index a2c70bb7..a53fea97 100644 --- a/doc/component.md +++ b/doc/component.md @@ -146,11 +146,6 @@ find a template with the component name (or one of its ancestor). if we have a `isMobile` key in the environment, to decide if we want a mobile interface or a destkop one. -- **`set(target, key, value)`**. This method is necessary in some cases when we - need to modify the state of the component in a way that is not visible to the - observer (see [observer's technical limitations](observer.md#technical-limitations)). - For example, if we need to add a key to the state. - - **`destroy()`**. As its name suggests, this method will remove the component, and perform all necessary cleanup, such as unmounting the component, its children, removing the parent/children relationship. This method should almost never be diff --git a/doc/observer.md b/doc/observer.md index 35d50d45..48f61353 100644 --- a/doc/observer.md +++ b/doc/observer.md @@ -20,35 +20,31 @@ obj.a.b = 2; ## Technical Limitations Since the observer uses getters and setters, it is actually unable to react to -changes in two situations: +changes in three situations: -- adding a key to an object: +- adding a key to an object +- deleting a key from an object +- modifying an array by setting a new value at a given index + +In those situations, we need a way to tell the observer that something happened. +This can be done by using the `set` and `delete` (only for objects) static +methods of the `Observer`. ```javascript const observer = new owl.Observer(); const obj = { a: 1 }; observer.observe(obj); - obj.b = 2; // will do nothing + obj.b = 2; // won't notify the change + owl.Observer.set(obj, "b", 2); // will notify the change + + delete obj.b; // won't notify the change + owl.Observer.delete(obj, "b"); // will notify the change ``` - In that case, we need a way to tell the observer that something happened. - This can be done by using the `set` method: - - ```javascript - observer.set(obj, "b", 2); - ``` - -- modifying an array by setting a new value at a given index: - ```javascript const observer = new owl.Observer(); - const obj = { todos: [{ id: 1, text: "todo" }] }; - observer.observe(obj); - obj[0] = { id: 2, text: "othertodo" }; // will do nothing, and obj[0] is not observed - ``` - - In that case, the solution is the same, we can simply use the `set` method: - - ```javascript - observer.set(obj, 0, { id: 2, text: "othertodo" }); + const arr = ["a"]; + observer.observe(arr); + arr[0] = "b"; // won't notify the change + owl.Observer.set(arr, 0, "b"); // will notify the change ``` diff --git a/src/component.ts b/src/component.ts index 4600d282..8c5614ad 100644 --- a/src/component.ts +++ b/src/component.ts @@ -340,14 +340,6 @@ export class Component { } } - /** - * Sets a key (from the state) to a specific value. This is mostly useful to - * work around the limitation in observed value with new keys. - */ - set(target: any, key: string | number, value: any) { - this.__owl__.observer!.set(target, key, value); - } - /** * Emit a custom event of type 'eventType' with the given 'payload' on the * component's el, if it exists. However, note that the event will only bubble diff --git a/src/observer.ts b/src/observer.ts index efb1f67c..c5c665e2 100644 --- a/src/observer.ts +++ b/src/observer.ts @@ -76,6 +76,24 @@ export class Observer { 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); + } + notifyCB() {} notifyChange() { this.dirty = true; @@ -120,8 +138,19 @@ export class Observer { this.notifyChange(); } + 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 }; + 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]); @@ -129,7 +158,12 @@ export class Observer { } _observeArr(arr: Array, parent?: any) { - (arr).__owl__ = { rev: this.rev, deepRev: this.rev, parent }; + (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; @@ -145,6 +179,7 @@ export class Observer { ) { var self = this; Object.defineProperty(obj, key, { + configurable: true, enumerable: true, get() { return value; diff --git a/src/store.ts b/src/store.ts index 35d1e462..853d0231 100644 --- a/src/store.ts +++ b/src/store.ts @@ -21,7 +21,7 @@ import { Observer } from "./observer"; // Store Definition //------------------------------------------------------------------------------ -type Mutation = ({ state, commit, set, getters }, payload: any) => void; +type Mutation = ({ state, commit, getters }, payload: any) => void; type Action = ({ commit, state, dispatch, env, getters }, payload: any) => void; type Getter = ({ state, getters }, payload) => any; @@ -46,7 +46,6 @@ export class Store extends EventBus { debug: boolean; env: any; observer: Observer; - set: any; getters: { [name: string]: (payload?) => any }; constructor(config: StoreConfig, options: StoreOption = {}) { @@ -65,7 +64,6 @@ export class Store extends EventBus { if (this.debug) { this.history.push({ state: this.state }); } - this.set = this.observer.set.bind(this.observer); for (let entry of Object.entries(config.getters || {})) { const name: string = entry[0]; @@ -110,7 +108,6 @@ export class Store extends EventBus { { commit: this.commit.bind(this), state: this.state, - set: this.set, getters: this.getters }, payload diff --git a/tests/observer.test.ts b/tests/observer.test.ts index f1e15144..70b3cdbd 100644 --- a/tests/observer.test.ts +++ b/tests/observer.test.ts @@ -134,6 +134,57 @@ describe("observer", () => { expect(arr[0].__owl__.rev).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); + expect(observer.notifyCB).toBeCalledTimes(0); + + Observer.set(state, "b", 8); + await nextMicroTick(); + expect(state.__owl__.rev).toBe(2); + expect(observer.rev).toBe(2); + expect(observer.notifyCB).toBeCalledTimes(1); + 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 }; + observer.observe(state); + expect(state.__owl__.rev).toBe(1); + expect(observer.rev).toBe(1); + expect(observer.notifyCB).toBeCalledTimes(0); + + Observer.delete(state, "b"); + await nextMicroTick(); + expect(state.__owl__.rev).toBe(2); + expect(observer.rev).toBe(2); + expect(observer.notifyCB).toBeCalledTimes(1); + 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); + expect(observer.rev).toBe(1); + expect(observer.notifyCB).toBeCalledTimes(0); + + Observer.set(state, 1, "b"); + await nextMicroTick(); + expect(state.__owl__.rev).toBe(2); + expect(observer.rev).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: [] }; diff --git a/tests/store.test.ts b/tests/store.test.ts index 2a0a81f4..88f12f82 100644 --- a/tests/store.test.ts +++ b/tests/store.test.ts @@ -152,24 +152,6 @@ describe("basic use", () => { store.dispatch("someaction"); }); - test("set function is given to mutations", async () => { - let updateCounter = 0; - const state = { bertinchamps: "brune" }; - const mutations = { - addInfo({ state, set }) { - set(state, "chouffe", "blonde"); - } - }; - const store = new Store({ state, mutations, actions: {} }); - store.on("update", null, () => updateCounter++); - - expect(updateCounter).toBe(0); - store.commit("addInfo"); - await nextMicroTick(); - expect(updateCounter).toBe(1); - expect(store.state).toEqual({ bertinchamps: "brune", chouffe: "blonde" }); - }); - test("can have getters from store", async () => { const state = { beers: {