From 0e38943eeeaa9dd8ee9cac7ad0d06447297e8745 Mon Sep 17 00:00:00 2001 From: Haroen Viaene Date: Mon, 24 Jan 2022 16:50:02 +0100 Subject: [PATCH 1/4] feat(dynamicWidgets): prevent calling of "render" when widgets are not yet added --- .../connectors/dynamic-widgets/connectDynamicWidgets.ts | 6 +++++- packages/instantsearch.js/src/types/render-state.ts | 4 +++- packages/instantsearch.js/src/widgets/index/index.ts | 7 +++++++ 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/packages/instantsearch.js/src/connectors/dynamic-widgets/connectDynamicWidgets.ts b/packages/instantsearch.js/src/connectors/dynamic-widgets/connectDynamicWidgets.ts index 74ec8e74f8..f13ad12d16 100644 --- a/packages/instantsearch.js/src/connectors/dynamic-widgets/connectDynamicWidgets.ts +++ b/packages/instantsearch.js/src/connectors/dynamic-widgets/connectDynamicWidgets.ts @@ -212,9 +212,13 @@ const connectDynamicWidgets: DynamicWidgetsConnector = ); }, getRenderState(renderState, renderOptions) { + const dynamicWidgets = this.getWidgetRenderState(renderOptions); return { ...renderState, - dynamicWidgets: this.getWidgetRenderState(renderOptions), + // if we are in a "has results, but only just mounted widgets" state, add a flag + // in other cases the flag isn't set and we *do* render the other widgets + PREVENT_RENDER: renderState.PREVENT_RENDER && true, + dynamicWidgets, }; }, getWidgetRenderState({ results, state }) { diff --git a/packages/instantsearch.js/src/types/render-state.ts b/packages/instantsearch.js/src/types/render-state.ts index 4064993dee..1f531bc178 100644 --- a/packages/instantsearch.js/src/types/render-state.ts +++ b/packages/instantsearch.js/src/types/render-state.ts @@ -55,8 +55,10 @@ type ConnectorRenderStates = AnswersWidgetDescription['indexRenderState'] & type WidgetRenderStates = AnalyticsWidgetDescription['indexRenderState'] & PlacesWidgetDescription['indexRenderState']; +type GlobalRenderStates = { PREVENT_RENDER: boolean }; + export type IndexRenderState = Partial< - ConnectorRenderStates & WidgetRenderStates + ConnectorRenderStates & WidgetRenderStates & GlobalRenderStates >; export type RenderState = { diff --git a/packages/instantsearch.js/src/widgets/index/index.ts b/packages/instantsearch.js/src/widgets/index/index.ts index 5998862b1d..51cf9fd51e 100644 --- a/packages/instantsearch.js/src/widgets/index/index.ts +++ b/packages/instantsearch.js/src/widgets/index/index.ts @@ -591,6 +591,13 @@ const index = (widgetParams: IndexWidgetParams): IndexWidget => { } }); + // a widget that causes the current state to be incomplete (dynamic widgets) + // can prevent following widgets from rendering this pass by setting the + // PREVENT_RENDER flag to "true" + if (instantSearchInstance.renderState[this.getIndexId()].PREVENT_RENDER) { + return; + } + localWidgets.forEach((widget) => { // At this point, all the variables used below are set. Both `helper` // and `derivedHelper` have been created at the `init` step. The attribute From 8ef25bf6725679e7b731d1ed61c7e4daea10c81d Mon Sep 17 00:00:00 2001 From: Haroen Viaene Date: Mon, 24 Jan 2022 16:57:49 +0100 Subject: [PATCH 2/4] try of the actual condition --- .../dynamic-widgets/connectDynamicWidgets.ts | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/packages/instantsearch.js/src/connectors/dynamic-widgets/connectDynamicWidgets.ts b/packages/instantsearch.js/src/connectors/dynamic-widgets/connectDynamicWidgets.ts index f13ad12d16..a0323ede21 100644 --- a/packages/instantsearch.js/src/connectors/dynamic-widgets/connectDynamicWidgets.ts +++ b/packages/instantsearch.js/src/connectors/dynamic-widgets/connectDynamicWidgets.ts @@ -215,9 +215,13 @@ const connectDynamicWidgets: DynamicWidgetsConnector = const dynamicWidgets = this.getWidgetRenderState(renderOptions); return { ...renderState, - // if we are in a "has results, but only just mounted widgets" state, add a flag - // in other cases the flag isn't set and we *do* render the other widgets - PREVENT_RENDER: renderState.PREVENT_RENDER && true, + PREVENT_RENDER: + renderState.PREVENT_RENDER && + // if we are in a "has results, but only just mounted widgets" state, add a flag + // in other cases the flag isn't set and we *do* render the other widgets + // TODO: improve this condition, reordering is allowed, we just want to make sure it's the same items + renderState.dynamicWidgets?.attributesToRender.join('__') !== + dynamicWidgets?.attributesToRender.join('__'), dynamicWidgets, }; }, From c4d541891f724d4c300b0e42856fd2c96ef36c32 Mon Sep 17 00:00:00 2001 From: Haroen Viaene Date: Mon, 24 Jan 2022 17:29:29 +0100 Subject: [PATCH 3/4] better condition, still messy though --- .../connectors/dynamic-widgets/connectDynamicWidgets.ts | 3 ++- packages/instantsearch.js/src/widgets/index/index.ts | 8 ++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/instantsearch.js/src/connectors/dynamic-widgets/connectDynamicWidgets.ts b/packages/instantsearch.js/src/connectors/dynamic-widgets/connectDynamicWidgets.ts index a0323ede21..490672b891 100644 --- a/packages/instantsearch.js/src/connectors/dynamic-widgets/connectDynamicWidgets.ts +++ b/packages/instantsearch.js/src/connectors/dynamic-widgets/connectDynamicWidgets.ts @@ -211,12 +211,13 @@ const connectDynamicWidgets: DynamicWidgetsConnector = }) ); }, + rendersOnPrevented: true, getRenderState(renderState, renderOptions) { const dynamicWidgets = this.getWidgetRenderState(renderOptions); return { ...renderState, PREVENT_RENDER: - renderState.PREVENT_RENDER && + renderState.PREVENT_RENDER !== false && // if we are in a "has results, but only just mounted widgets" state, add a flag // in other cases the flag isn't set and we *do* render the other widgets // TODO: improve this condition, reordering is allowed, we just want to make sure it's the same items diff --git a/packages/instantsearch.js/src/widgets/index/index.ts b/packages/instantsearch.js/src/widgets/index/index.ts index 51cf9fd51e..4fe103a223 100644 --- a/packages/instantsearch.js/src/widgets/index/index.ts +++ b/packages/instantsearch.js/src/widgets/index/index.ts @@ -594,9 +594,8 @@ const index = (widgetParams: IndexWidgetParams): IndexWidget => { // a widget that causes the current state to be incomplete (dynamic widgets) // can prevent following widgets from rendering this pass by setting the // PREVENT_RENDER flag to "true" - if (instantSearchInstance.renderState[this.getIndexId()].PREVENT_RENDER) { - return; - } + const renderPrevented = + instantSearchInstance.renderState[this.getIndexId()].PREVENT_RENDER; localWidgets.forEach((widget) => { // At this point, all the variables used below are set. Both `helper` @@ -606,7 +605,8 @@ const index = (widgetParams: IndexWidgetParams): IndexWidget => { // be delayed. The render is triggered for the complete tree but some parts do // not have results yet. - if (widget.render) { + // the widget can make itself rendering, even if the rest is prevented + if (widget.render && (!renderPrevented || widget.rendersOnPrevented)) { widget.render(createRenderArgs(instantSearchInstance, this)); } }); From 00d1713685fb255597957115a463e179a295acee Mon Sep 17 00:00:00 2001 From: Haroen Viaene Date: Thu, 27 Jan 2022 13:08:28 +0100 Subject: [PATCH 4/4] make other tests pass --- .../__tests__/connectDynamicWidgets-test.ts | 2 ++ .../dynamic-widgets/connectDynamicWidgets.ts | 17 ++++++++++------- packages/instantsearch.js/src/types/widget.ts | 6 ++++++ .../instantsearch.js/src/widgets/index/index.ts | 6 +++--- 4 files changed, 21 insertions(+), 10 deletions(-) diff --git a/packages/instantsearch.js/src/connectors/dynamic-widgets/__tests__/connectDynamicWidgets-test.ts b/packages/instantsearch.js/src/connectors/dynamic-widgets/__tests__/connectDynamicWidgets-test.ts index aa245f586e..7f2f5f91e9 100644 --- a/packages/instantsearch.js/src/connectors/dynamic-widgets/__tests__/connectDynamicWidgets-test.ts +++ b/packages/instantsearch.js/src/connectors/dynamic-widgets/__tests__/connectDynamicWidgets-test.ts @@ -721,6 +721,7 @@ describe('connectDynamicWidgets', () => { expect( dynamicWidgets.getRenderState(existingRenderState, createInitOptions()) ).toEqual({ + PREVENT_RENDER: true, dynamicWidgets: { attributesToRender: [], widgetParams, @@ -748,6 +749,7 @@ describe('connectDynamicWidgets', () => { createRenderOptions() ) ).toEqual({ + PREVENT_RENDER: true, dynamicWidgets: { attributesToRender: ['test1'], widgetParams, diff --git a/packages/instantsearch.js/src/connectors/dynamic-widgets/connectDynamicWidgets.ts b/packages/instantsearch.js/src/connectors/dynamic-widgets/connectDynamicWidgets.ts index 490672b891..82b78aa8d6 100644 --- a/packages/instantsearch.js/src/connectors/dynamic-widgets/connectDynamicWidgets.ts +++ b/packages/instantsearch.js/src/connectors/dynamic-widgets/connectDynamicWidgets.ts @@ -213,16 +213,19 @@ const connectDynamicWidgets: DynamicWidgetsConnector = }, rendersOnPrevented: true, getRenderState(renderState, renderOptions) { + const { PREVENT_RENDER = true } = renderState; const dynamicWidgets = this.getWidgetRenderState(renderOptions); + + // if we are in a "has results, but only just mounted widgets" state, add a flag + // in other cases the flag isn't set and we *do* render the other widgets + // TODO: improve this condition, reordering is allowed, we just want to make sure it's the same items + const willChangeWidgets = + renderState.dynamicWidgets?.attributesToRender.join('__') !== + dynamicWidgets?.attributesToRender.join('__'); + return { ...renderState, - PREVENT_RENDER: - renderState.PREVENT_RENDER !== false && - // if we are in a "has results, but only just mounted widgets" state, add a flag - // in other cases the flag isn't set and we *do* render the other widgets - // TODO: improve this condition, reordering is allowed, we just want to make sure it's the same items - renderState.dynamicWidgets?.attributesToRender.join('__') !== - dynamicWidgets?.attributesToRender.join('__'), + PREVENT_RENDER: PREVENT_RENDER !== false && willChangeWidgets, dynamicWidgets, }; }, diff --git a/packages/instantsearch.js/src/types/widget.ts b/packages/instantsearch.js/src/types/widget.ts index 653df1560a..d9833fc3c0 100644 --- a/packages/instantsearch.js/src/types/widget.ts +++ b/packages/instantsearch.js/src/types/widget.ts @@ -245,6 +245,12 @@ type RequiredRenderStateLifeCycle< >, renderOptions: InitOptions | RenderOptions ) => IndexRenderState & TWidgetDescription['indexRenderState']; + + /** + * If PREVENT_RENDER is set in the render state, still render this widget. + * This is only useful for cases where this widget is the one setting PREVENT_RENDER + */ + rendersOnPrevented?: boolean; }; type RenderStateLifeCycle< diff --git a/packages/instantsearch.js/src/widgets/index/index.ts b/packages/instantsearch.js/src/widgets/index/index.ts index 4fe103a223..ae999cea54 100644 --- a/packages/instantsearch.js/src/widgets/index/index.ts +++ b/packages/instantsearch.js/src/widgets/index/index.ts @@ -594,8 +594,8 @@ const index = (widgetParams: IndexWidgetParams): IndexWidget => { // a widget that causes the current state to be incomplete (dynamic widgets) // can prevent following widgets from rendering this pass by setting the // PREVENT_RENDER flag to "true" - const renderPrevented = - instantSearchInstance.renderState[this.getIndexId()].PREVENT_RENDER; + const { PREVENT_RENDER = false } = + instantSearchInstance.renderState?.[this.getIndexId()] ?? {}; localWidgets.forEach((widget) => { // At this point, all the variables used below are set. Both `helper` @@ -606,7 +606,7 @@ const index = (widgetParams: IndexWidgetParams): IndexWidget => { // not have results yet. // the widget can make itself rendering, even if the rest is prevented - if (widget.render && (!renderPrevented || widget.rendersOnPrevented)) { + if (widget.render && (!PREVENT_RENDER || widget.rendersOnPrevented)) { widget.render(createRenderArgs(instantSearchInstance, this)); } });