From 40e486a9eb441c7b06ab5a2f5324de88eb583779 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= Date: Mon, 29 Apr 2019 16:49:37 +0200 Subject: [PATCH] [REF] component: remove updateProps it is actually now a private method --- doc/component.md | 4 --- src/component.ts | 32 ++++++++++++---------- src/qweb.ts | 4 +-- src/store.ts | 6 ++-- tests/__snapshots__/component.test.ts.snap | 12 ++++---- tests/component.test.ts | 4 +-- 6 files changed, 30 insertions(+), 32 deletions(-) diff --git a/doc/component.md b/doc/component.md index 045103d7..dc63e293 100644 --- a/doc/component.md +++ b/doc/component.md @@ -142,10 +142,6 @@ will be used to render the component template. always return true. This is an optimization, similar to React's `shouldComponentUpdate`. Most of the time, this should not be used, but it can be useful if we are handling large number of components. -- **`updateProps(nextProps)`**: should not be called manually, except on the root - component. This method is only supposed to be called by the framework whenever - a parent is rerendered. - - **`updateEnv(nextEnv)`**: update the environment of a component and all its children. This forces a complete rerender. For example, this could be useful if we have a `isMobile` key in the environment, to decide if we want a mobile diff --git a/src/component.ts b/src/component.ts index 981d269d..15ee3109 100644 --- a/src/component.ts +++ b/src/component.ts @@ -319,14 +319,6 @@ export class Component< } } - async updateProps( - nextProps: Props, - forceUpdate: boolean = false, - patchQueue: any[], - ): Promise { - const shouldUpdate = forceUpdate || this.shouldUpdate(nextProps); - return shouldUpdate ? this._updateProps(nextProps, patchQueue) : Promise.resolve(); - } set(target: any, key: string | number, value: any) { this.__owl__.observer!.set(target, key, value); @@ -336,10 +328,17 @@ export class Component< // Private //-------------------------------------------------------------------------- - async _updateProps(nextProps: Props, patchQueue: any[]): Promise { - await this.willUpdateProps(nextProps); - this.props = nextProps; - await this.render(false, patchQueue); + async _updateProps( + nextProps: Props, + forceUpdate: boolean = false, + patchQueue: any[] + ): Promise { + const shouldUpdate = forceUpdate || this.shouldUpdate(nextProps); + if (shouldUpdate) { + await this.willUpdateProps(nextProps); + this.props = nextProps; + await this.render(false, patchQueue); + } } _patch(vnode) { @@ -370,9 +369,12 @@ export class Component< return this.__owl__.renderPromise; } - async _render(force: boolean = false, patchQueue: any[] = []): Promise { + async _render( + force: boolean = false, + patchQueue: any[] = [] + ): Promise { if (this.__owl__.isMounted) { - patchQueue.push(this) + patchQueue.push(this); } this.__owl__.renderId++; const promises: Promise[] = []; @@ -384,7 +386,7 @@ export class Component< promises, handlers: this.__owl__.boundHandlers, forceUpdate: force, - patchQueue, + patchQueue }); if (this.__owl__.observer) { this.__owl__.observer.allowMutations = true; diff --git a/src/qweb.ts b/src/qweb.ts index 4cc34316..1ca39405 100644 --- a/src/qweb.ts +++ b/src/qweb.ts @@ -1060,7 +1060,7 @@ const widgetDirective: Directive = { ctx.addIf(`w${widgetID} && w${widgetID}.__owl__.renderPromise`); ctx.addIf(`w${widgetID}.__owl__.isStarted`); ctx.addLine( - `def${defID} = w${widgetID}.updateProps(props${widgetID}, extra.forceUpdate, extra.patchQueue);` + `def${defID} = w${widgetID}._updateProps(props${widgetID}, extra.forceUpdate, extra.patchQueue);` ); ctx.addElse(); ctx.addLine(`isNew${widgetID} = true`); @@ -1076,7 +1076,7 @@ const widgetDirective: Directive = { ctx.addIf(`!def${defID}`); ctx.addIf(`w${widgetID}`); ctx.addLine( - `def${defID} = w${widgetID}.updateProps(props${widgetID}, extra.forceUpdate, extra.patchQueue);` + `def${defID} = w${widgetID}._updateProps(props${widgetID}, extra.forceUpdate, extra.patchQueue);` ); ctx.addElse(); ctx.addLine( diff --git a/src/store.ts b/src/store.ts index c9e0b6ec..abe10da3 100644 --- a/src/store.ts +++ b/src/store.ts @@ -189,7 +189,7 @@ export function connect(mapStateToProps, options: any = {}) { } if (didChange) { this.__owl__.currentStoreProps = storeProps; - this.updateProps(ownProps, false); + this._updateProps(ownProps, false); } }); super.mounted(); @@ -198,7 +198,7 @@ export function connect(mapStateToProps, options: any = {}) { this.env.store.off("update", this); super.willUnmount(); } - updateProps(nextProps, forceUpdate) { + _updateProps(nextProps, forceUpdate) { if (this.__owl__.ownProps !== nextProps) { this.__owl__.currentStoreProps = mapStateToProps( this.env.store.state, @@ -211,7 +211,7 @@ export function connect(mapStateToProps, options: any = {}) { nextProps, this.__owl__.currentStoreProps ); - return super.updateProps(mergedProps, forceUpdate); + return super._updateProps(mergedProps, forceUpdate); } }; }; diff --git a/tests/__snapshots__/component.test.ts.snap b/tests/__snapshots__/component.test.ts.snap index 3c092a63..03ea5d46 100644 --- a/tests/__snapshots__/component.test.ts.snap +++ b/tests/__snapshots__/component.test.ts.snap @@ -30,7 +30,7 @@ exports[`composition sub widgets with some state rendered in a loop 1`] = ` let isNew7 = !w7; if (w7 && w7.__owl__.renderPromise) { if (w7.__owl__.isStarted) { - def6 = w7.updateProps(props7, extra.forceUpdate, extra.patchQueue); + def6 = w7._updateProps(props7, extra.forceUpdate, extra.patchQueue); } else { isNew7 = true if (props7 === w7.__owl__.renderProps) { @@ -43,7 +43,7 @@ exports[`composition sub widgets with some state rendered in a loop 1`] = ` } if (!def6) { if (w7) { - def6 = w7.updateProps(props7, extra.forceUpdate, extra.patchQueue); + def6 = w7._updateProps(props7, extra.forceUpdate, extra.patchQueue); } else { w7 = new context.widgets['ChildWidget'](owner, props7); context.__owl__.cmap[key8] = w7.__owl__.id; @@ -78,7 +78,7 @@ exports[`random stuff/miscellaneous snapshotting compiled code 1`] = ` let isNew4 = !w4; if (w4 && w4.__owl__.renderPromise) { if (w4.__owl__.isStarted) { - def3 = w4.updateProps(props4, extra.forceUpdate, extra.patchQueue); + def3 = w4._updateProps(props4, extra.forceUpdate, extra.patchQueue); } else { isNew4 = true if (props4 === w4.__owl__.renderProps) { @@ -91,7 +91,7 @@ exports[`random stuff/miscellaneous snapshotting compiled code 1`] = ` } if (!def3) { if (w4) { - def3 = w4.updateProps(props4, extra.forceUpdate, extra.patchQueue); + def3 = w4._updateProps(props4, extra.forceUpdate, extra.patchQueue); } else { w4 = new context.widgets['child'](owner, props4); context.__owl__.cmap[key5] = w4.__owl__.id; @@ -124,7 +124,7 @@ exports[`random stuff/miscellaneous t-props should not be undefined (snapshottin let isNew4 = !w4; if (w4 && w4.__owl__.renderPromise) { if (w4.__owl__.isStarted) { - def3 = w4.updateProps(props4, extra.forceUpdate, extra.patchQueue); + def3 = w4._updateProps(props4, extra.forceUpdate, extra.patchQueue); } else { isNew4 = true if (props4 === w4.__owl__.renderProps) { @@ -137,7 +137,7 @@ exports[`random stuff/miscellaneous t-props should not be undefined (snapshottin } if (!def3) { if (w4) { - def3 = w4.updateProps(props4, extra.forceUpdate, extra.patchQueue); + def3 = w4._updateProps(props4, extra.forceUpdate, extra.patchQueue); } else { w4 = new context.widgets['child'](owner, props4); context.__owl__.cmap[4] = w4.__owl__.id; diff --git a/tests/component.test.ts b/tests/component.test.ts index e8e32c1f..370c6146 100644 --- a/tests/component.test.ts +++ b/tests/component.test.ts @@ -1485,8 +1485,8 @@ describe("async rendering", () => { class ChildA extends Widget { inlineTemplate = `a`; - updateProps(props, forceUpdate, fiber): Promise { - return defA.then(() => super.updateProps(props, forceUpdate, fiber)); + _updateProps(props, forceUpdate, fiber): Promise { + return defA.then(() => super._updateProps(props, forceUpdate, fiber)); } } class ChildB extends Widget {