From 5ee70fe1ee0f794fccc0a7e64ca00959d6e787c4 Mon Sep 17 00:00:00 2001 From: Jeremy Zagorski Date: Sun, 4 Nov 2018 10:49:36 -0500 Subject: [PATCH] refactor(connectTo): Call the change handler after value is set Change this logic to align with Aurelia's change handling method invocation for view model properties. @bindable and @observable both call the property change method after the value is set. closes #69 BREAKING CHANGE: the property value being changed will be set and equal to the newVal parameter when its change handler is invoked. To migrate from old code, get the old value from the oldValue parameter in the method call instead of getting a value from the view model property --- src/decorator.ts | 16 ++++++++-------- test/unit/decorator.spec.ts | 26 ++++++++++---------------- 2 files changed, 18 insertions(+), 24 deletions(-) diff --git a/src/decorator.ts b/src/decorator.ts index cb56f36..0497f38 100644 --- a/src/decorator.ts +++ b/src/decorator.ts @@ -39,17 +39,17 @@ export function connectTo(settings?: ((store: Store) => Observabl } function createSelectors() { - const isSelectorObj = typeof _settings.selector === "object"; + const isSelectorObj = typeof _settings.selector === "object"; const fallbackSelector = { [_settings.target || "state"]: _settings.selector || defaultSelector }; - + return Object.entries({ ...((isSelectorObj ? _settings.selector : fallbackSelector) as MultipleSelector) }).map(([target, selector]) => ({ targets: _settings.target && isSelectorObj ? [_settings.target, target] : [target], selector, - // numbers are the starting index to slice all the change handling args, + // numbers are the starting index to slice all the change handling args, // which are prop name, new state and old state changeHandlers: { [_settings.onChanged || ""]: 1, @@ -78,16 +78,16 @@ export function connectTo(settings?: ((store: Store) => Observabl const lastTargetIdx = s.targets.length - 1; const oldState = s.targets.reduce((accu = {}, curr) => accu[curr], this); + s.targets.reduce((accu, curr, idx) => { + accu[curr] = idx === lastTargetIdx ? state : accu[curr] || {}; + return accu[curr]; + }, this); + Object.entries(s.changeHandlers).forEach(([handlerName, args]) => { if (handlerName in this) { this[handlerName](...[ s.targets[lastTargetIdx], state, oldState ].slice(args, 3)) } }); - - s.targets.reduce((accu, curr, idx) => { - accu[curr] = idx === lastTargetIdx ? state : accu[curr] || {}; - return accu[curr]; - }, this); })); if (originalSetup) { diff --git a/test/unit/decorator.spec.ts b/test/unit/decorator.spec.ts index 9eb0197..0b011bf 100644 --- a/test/unit/decorator.spec.ts +++ b/test/unit/decorator.spec.ts @@ -480,19 +480,20 @@ describe("using decorators", () => { expect(sut.stateChanged).toHaveBeenCalledWith(initialState, undefined); }); - it("should be called before assigning the new state, so there is still access to the previous state", () => { + it("should be called after assigning the new state", () => { const { initialState } = arrange(); + const oldState = { foo: "foo", bar: "bar" }; @connectTo({ onChanged: "stateChanged", selector: (store) => store.state, }) class DemoStoreConsumer { - state: DemoState; + state: DemoState = oldState; - stateChanged(state: DemoState) { - expect(sut.state).toEqual(undefined); - expect(state).toEqual(initialState); + stateChanged(newState: DemoState, oldState: DemoState) { + expect(sut.state).toBe(newState); + expect(newState).not.toBe(oldState); } } @@ -502,7 +503,6 @@ describe("using decorators", () => { it("should call the targetChanged handler on the VM, if existing, with the new and old state", () => { const { initialState } = arrange(); - let targetValOnChange = null; @connectTo({ selector: { @@ -514,7 +514,6 @@ describe("using decorators", () => { targetProp = "foobar" targetPropChanged() { - targetValOnChange = sut.targetProp; } } @@ -522,8 +521,7 @@ describe("using decorators", () => { spyOn(sut, "targetPropChanged").and.callThrough(); (sut as any).bind(); - expect(targetValOnChange).toEqual("foobar"); - expect(sut.targetProp).toEqual(initialState); + expect(sut.targetProp).toBe(initialState); expect(sut.targetPropChanged.calls.count()).toEqual(1); expect(sut.targetPropChanged).toHaveBeenCalledWith(initialState, "foobar"); expect(sut.targetPropChanged.calls.argsFor(0)[0]).toBe(initialState); @@ -531,7 +529,6 @@ describe("using decorators", () => { it("should call the propertyChanged handler on the VM, if existing, with the new and old state", () => { const { initialState } = arrange(); - let targetValOnChange = null; @connectTo({ selector: { @@ -543,7 +540,6 @@ describe("using decorators", () => { targetProp = "foobar" propertyChanged() { - targetValOnChange = sut.targetProp; } } @@ -551,9 +547,10 @@ describe("using decorators", () => { spyOn(sut, "propertyChanged").and.callThrough(); (sut as any).bind(); - expect(targetValOnChange).toEqual("foobar"); - expect(sut.targetProp).toEqual(initialState); + expect(sut.targetProp).toBe(initialState); + expect(sut.propertyChanged.calls.count()).toEqual(1); expect(sut.propertyChanged).toHaveBeenCalledWith("targetProp", initialState, "foobar"); + expect(sut.propertyChanged.calls.argsFor(0)[1]).toBe(initialState); }); it("should call all change handlers on the VM, if existing, in order and with the correct args", () => { @@ -593,7 +590,6 @@ describe("using decorators", () => { it("should call the targetOnChanged handler and not each multiple selector, if existing, with the 3 args", () => { const { initialState } = arrange(); - let targetValOnChange = null; @connectTo({ target: "foo", @@ -611,7 +607,6 @@ describe("using decorators", () => { } fooChanged() { - targetValOnChange = sut.foo.targetProp; } } @@ -620,7 +615,6 @@ describe("using decorators", () => { spyOn(sut, "targetPropChanged"); (sut as any).bind(); - expect(targetValOnChange).toEqual("foobar"); expect(sut.foo.targetProp).toEqual(initialState); expect(sut.targetPropChanged.calls.count()).toEqual(0); expect(sut.fooChanged.calls.count()).toEqual(1);