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: {