diff --git a/src/store/connected_component.ts b/src/store/connected_component.ts index d4a99163..347c297b 100644 --- a/src/store/connected_component.ts +++ b/src/store/connected_component.ts @@ -56,6 +56,7 @@ export class ConnectedComponent extends Component const observer = store.observer; const revFn = this.deep ? observer.deepRevNumber : observer.revNumber; (this.__owl__ as any).store = store; + (this.__owl__ as any).ownProps = this.props; (this.__owl__ as any).revFn = revFn.bind(observer); (this.__owl__ as any).storeHash = this.hashFunction(this.storeProps, { prevStoreProps: this.storeProps @@ -81,13 +82,44 @@ export class ConnectedComponent extends Component super.__destroy(parent); } + async render(force: boolean = false) { + this.__updateStoreProps(this.props); + + // this is quite technical, so this deserves some explanation. + // When we have a connected component, it can be updated for 3 reasons: + // - some internal state changes (this will go through this method) + // - some props changes (if a parent is changed and need to rerender itself) + // - a store update + // + // It is possible (with connected component and parent) to have the following + // situation: the parent component is rendered first (from its state change), + // then immediately after, it is rendered (from store update). Then, if the + // __checkUpdate method is immediately over, the children component will + // be rendered again by the store update, even though it is supposed to be + // destroyed by the first rendering. + // + // So, the solution is to keep the information that there is a current + // rendering occuring with the same store state, the same props, and return + // that in the __checkUpdate method. To do this, we use the renderPromise + // deferred, which is not used by the component system once the + // component is ready, so we can use it for our own purpose. + (this.__owl__ as any).renderPromise = super.render(force); + return (this.__owl__ as any).renderPromise; + } + async __updateProps(nextProps: P, f, p, s, v) { this.__updateStoreProps(nextProps); return super.__updateProps(nextProps, f, p, s, v); } __updateStoreProps(nextProps): boolean { - const store = (this.__owl__ as any).store; + const __owl__ = this.__owl__ as any; + const store = __owl__.store; + const observer = store.observer; + if (observer.rev === __owl__.rev && nextProps === __owl__.ownProps) { + return false; + } + const storeProps = (this.constructor).mapStoreToProps( store.state, nextProps, @@ -97,23 +129,21 @@ export class ConnectedComponent extends Component const storeHash = this.hashFunction(storeProps, options); this.storeProps = storeProps; let didChange = options.didChange; - if (storeHash !== (this.__owl__ as any).storeHash) { - (this.__owl__ as any).storeHash = storeHash; + if (storeHash !== __owl__.storeHash) { + __owl__.storeHash = storeHash; didChange = true; } - (this.__owl__ as any).rev = store.observer.rev; + __owl__.rev = store.observer.rev; + __owl__.ownProps = nextProps; return didChange; } async __checkUpdate() { - const observer = (this.__owl__ as any).store.observer; - if (observer.rev === (this.__owl__ as any).rev) { - // update was already done by updateProps, from parent - return; - } const didChange = this.__updateStoreProps(this.props); if (didChange) { return this.render(); } + // see note in render method + return (this.__owl__ as any).renderPromise; } } diff --git a/tests/store/__snapshots__/connected_component.test.ts.snap b/tests/store/__snapshots__/connected_component.test.ts.snap index 1754f5d5..a75eee66 100644 --- a/tests/store/__snapshots__/connected_component.test.ts.snap +++ b/tests/store/__snapshots__/connected_component.test.ts.snap @@ -15,3 +15,7 @@ exports[`connecting a component to store deep and shallow connecting a component exports[`connecting a component to store deep and shallow connecting a component 3`] = `"
Bertinchamps
"`; exports[`connecting a component to store deep and shallow connecting a component 4`] = `"
Kasteel
"`; + +exports[`various scenarios scenarios with async store updates and some components events 1`] = `"
Attachment 100Name: text.txt
"`; + +exports[`various scenarios scenarios with async store updates and some components events 2`] = `"
"`; diff --git a/tests/store/connected_component.test.ts b/tests/store/connected_component.test.ts index 0297fc5a..4cc36723 100644 --- a/tests/store/connected_component.test.ts +++ b/tests/store/connected_component.test.ts @@ -1094,3 +1094,85 @@ describe("connected components and default values", () => { expect(fixture.innerHTML).toBe("
1
"); }); }); + +describe("various scenarios", () => { + let fixture: HTMLElement; + let env: Env; + + beforeEach(() => { + fixture = makeTestFixture(); + env = makeTestEnv(); + }); + + afterEach(() => { + fixture.remove(); + }); + + test("scenarios with async store updates and some components events", async () => { + const actions = { + async deleteAttachment({ state }) { + await Promise.resolve(); + delete state.attachments[100]; + state.messages[10].attachmentIds = []; + } + }; + const state = { + attachments: { + 100: { + id: 100, + name: "text.txt" + } + }, + messages: { + 10: { + attachmentIds: [100], + id: 10 + } + } + }; + const store = new Store({ actions, state }); + + env.qweb.addTemplates(` + +
+ + +
+
+ Attachment + Name: +
+
+ `); + class Attachment extends ConnectedComponent { + static mapStoreToProps(state, ownProps) { + return { + name: state.attachments[ownProps.id].name + }; + } + } + class Message extends ConnectedComponent { + static mapStoreToProps(state) { + return { + attachmentIds: state.messages[10].attachmentIds + }; + } + components = { Attachment }; + state = { isAttachmentDeleted: false }; + doStuff() { + this.dispatch("deleteAttachment", 100); + this.state.isAttachmentDeleted = true; + } + } + + (env).store = store; + const message = new Message(env); + await message.mount(fixture); + + expect(fixture.innerHTML).toMatchSnapshot(); + + fixture.querySelector("button")!.click(); + await nextTick(); + expect(fixture.innerHTML).toMatchSnapshot(); + }); +});