From a5e3bd198abc55075c41f0edae2c3f0c3e489ea6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Chalifour?= Date: Mon, 23 Sep 2019 13:55:53 -0700 Subject: [PATCH 1/6] fix(index): warn for inconsistent UI state in development mode --- src/lib/__tests__/InstantSearch-test.js | 58 ++++++++++++++ src/types/widget.ts | 26 +++++- src/widgets/index/index.ts | 101 ++++++++++++++++++++++++ 3 files changed, 184 insertions(+), 1 deletion(-) diff --git a/src/lib/__tests__/InstantSearch-test.js b/src/lib/__tests__/InstantSearch-test.js index 4b901c930a..05ed6ec12d 100644 --- a/src/lib/__tests__/InstantSearch-test.js +++ b/src/lib/__tests__/InstantSearch-test.js @@ -1203,3 +1203,61 @@ describe('refresh', () => { expect(searchClient.search).toHaveBeenCalledTimes(2); }); }); + +describe('UI state', () => { + it('warns if UI state contains unmounted widgets in development mode', () => { + const searchClient = createSearchClient(); + const search = new InstantSearch({ + indexName: 'indexName', + searchClient, + initialUiState: { + indexName: { + query: 'First query', + page: 3, + refinementList: { + brand: ['Apple'], + }, + hierarchicalMenu: { + categories: 'Mobile', + }, + }, + }, + }); + + const searchBox = connectSearchBox(() => null)({}); + const customWidget = { render() {} }; + + search.addWidgets([searchBox, customWidget]); + + expect(() => { + search.start(); + }) + .toWarnDev(`[InstantSearch.js]: The UI state for the index "indexName" is not consistent with the widgets mounted. + +This can happen when the UI state is specified via \`initialUiState\` or \`routing\` but that the widgets responsible for this state were not added. This results in query parameters not being sent to the API. + +To fully reflect the state, some widgets need to be added to the index "indexName": + +- \`page\` needs one of these widgets: "pagination", "infiniteHits" +- \`refinementList\` needs one of these widgets: "refinementList" +- \`hierarchicalMenu\` needs one of these widgets: "hierarchicalMenu" + +If you do not wish to display widgets but still want to support their search parameters, you can mount "virtual widgets" that don't render anything: + +\`\`\` +const virtualPagination = connectPagination(() => null); +const virtualRefinementList = connectRefinementList(() => null); +const virtualHierarchicalMenu = connectHierarchicalMenu(() => null); + +search.addWidgets([ + virtualPagination({ /* ... */ }), + virtualRefinementList({ /* ... */ }), + virtualHierarchicalMenu({ /* ... */ }) +]); +\`\`\` + +If you're using custom widgets that do set these query parameters, we recommend using connectors instead. + +See https://www.algolia.com/doc/guides/building-search-ui/widgets/customize-an-existing-widget/js/#customize-the-complete-ui-of-the-widgets`); + }); +}); diff --git a/src/types/widget.ts b/src/types/widget.ts index 76bf782b0b..0b398a2313 100644 --- a/src/types/widget.ts +++ b/src/types/widget.ts @@ -115,7 +115,31 @@ export type UiState = { * have at least a `render` or a `init` function. */ export interface Widget { - $$type?: string; + $$type?: + | 'ais.autocomplete' + | 'ais.breadcrumb' + | 'ais.clearRefinements' + | 'ais.configure' + | 'ais.currentRefinements' + | 'ais.geoSearch' + | 'ais.hierarchicalMenu' + | 'ais.hits' + | 'ais.hitsPerPage' + | 'ais.index' + | 'ais.infiniteHits' + | 'ais.menu' + | 'ais.numericMenu' + | 'ais.pagination' + | 'ais.poweredBy' + | 'ais.queryRules' + | 'ais.range' + | 'ais.ratingMenu' + | 'ais.refinementList' + | 'ais.searchBox' + | 'ais.sortBy' + | 'ais.stats' + | 'ais.toggleRefinement' + | 'ais.voiceSearch'; /** * Called once before the first search */ diff --git a/src/widgets/index/index.ts b/src/widgets/index/index.ts index 47af816b1e..307bdf9ec9 100644 --- a/src/widgets/index/index.ts +++ b/src/widgets/index/index.ts @@ -21,6 +21,8 @@ import { createDocumentationMessageGenerator, resolveSearchParameters, mergeSearchParameters, + warning, + capitalize, } from '../../lib/utils'; const withUsage = createDocumentationMessageGenerator({ @@ -342,6 +344,105 @@ const index = (props: IndexProps): Index => { // it at the index level because it's either: all of them or none of them // that are stalled. The queries are performed into a single network request. instantSearchInstance.scheduleStalledRender(); + + if (__DEV__) { + type StateToWidgets = { + [TParameter in keyof IndexUiState]: Array; + }; + + const stateToWidgetsMap: StateToWidgets = { + query: ['ais.searchBox', 'ais.autocomplete', 'ais.voiceSearch'], + refinementList: ['ais.refinementList'], + menu: ['ais.menu'], + hierarchicalMenu: ['ais.hierarchicalMenu'], + numericMenu: ['ais.numericMenu'], + ratingMenu: ['ais.ratingMenu'], + range: ['ais.range'], + toggle: ['ais.toggleRefinement'], + geoSearch: ['ais.geoSearch'], + sortBy: ['ais.sortBy'], + page: ['ais.pagination', 'ais.infiniteHits'], + hitsPerPage: ['ais.hitsPerPage'], + configure: ['ais.configure'], + }; + + const mountedWidgets = this.getWidgets() + .map(widget => widget.$$type) + .filter(Boolean); + + const missingWidgets = Object.keys(localUiState).reduce< + Array> + >((acc, parameter) => { + const requiredWidgets: Array = + stateToWidgetsMap[parameter]; + + if ( + requiredWidgets.every( + requiredWidget => !mountedWidgets.includes(requiredWidget) + ) + ) { + acc.push({ + [parameter]: stateToWidgetsMap[parameter], + }); + } + + return acc; + }, []); + + warning( + missingWidgets.length === 0, + `The UI state for the index "${this.getIndexId()}" is not consistent with the widgets mounted. + +This can happen when the UI state is specified via \`initialUiState\` or \`routing\` but that the widgets responsible for this state were not added. This results in query parameters not being sent to the API. + +To fully reflect the state, some widgets need to be added to the index "${this.getIndexId()}": + +${missingWidgets + .map(widget => { + const stateParameter = Object.keys(widget)[0]; + const neededWidgets = stateToWidgetsMap[stateParameter] + .map( + (widgetIdentifier: string) => `"${widgetIdentifier.split('ais.')[1]}"` + ) + .join(', '); + + return `- \`${stateParameter}\` needs one of these widgets: ${neededWidgets}`; + }) + .join('\n')} + +If you do not wish to display widgets but still want to support their search parameters, you can mount "virtual widgets" that don't render anything: + +\`\`\` +${missingWidgets + .map(widget => { + const stateParameter = Object.keys(widget)[0]; + const capitalizedWidget = capitalize( + stateToWidgetsMap[stateParameter][0].split('ais.')[1] + ); + + return `const virtual${capitalizedWidget} = connect${capitalizedWidget}(() => null);`; + }) + .join('\n')} + +search.addWidgets([ + ${missingWidgets + .map(widget => { + const stateParameter = Object.keys(widget)[0]; + const capitalizedWidget = capitalize( + stateToWidgetsMap[stateParameter][0].split('ais.')[1] + ); + + return `virtual${capitalizedWidget}({ /* ... */ })`; + }) + .join(',\n ')} +]); +\`\`\` + +If you're using custom widgets that do set these query parameters, we recommend using connectors instead. + +See https://www.algolia.com/doc/guides/building-search-ui/widgets/customize-an-existing-widget/js/#customize-the-complete-ui-of-the-widgets` + ); + } }); derivedHelper.on('result', () => { From 646100990fee9e69a24c7dd77d9622dabe47ddc8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Chalifour?= Date: Thu, 26 Sep 2019 11:10:14 -0700 Subject: [PATCH 2/6] refactor: update logic to trigger warning --- src/widgets/index/index.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/widgets/index/index.ts b/src/widgets/index/index.ts index 307bdf9ec9..2e5ddd1f32 100644 --- a/src/widgets/index/index.ts +++ b/src/widgets/index/index.ts @@ -377,8 +377,8 @@ const index = (props: IndexProps): Index => { stateToWidgetsMap[parameter]; if ( - requiredWidgets.every( - requiredWidget => !mountedWidgets.includes(requiredWidget) + !requiredWidgets.some(requiredWidget => + mountedWidgets.includes(requiredWidget) ) ) { acc.push({ From 27240320507301fc25779190ac7eddcf80535f36 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Chalifour?= Date: Thu, 26 Sep 2019 11:10:36 -0700 Subject: [PATCH 3/6] fix: add "those" in warning --- src/lib/__tests__/InstantSearch-test.js | 2 +- src/widgets/index/index.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lib/__tests__/InstantSearch-test.js b/src/lib/__tests__/InstantSearch-test.js index 05ed6ec12d..4f23ecca8c 100644 --- a/src/lib/__tests__/InstantSearch-test.js +++ b/src/lib/__tests__/InstantSearch-test.js @@ -1234,7 +1234,7 @@ describe('UI state', () => { }) .toWarnDev(`[InstantSearch.js]: The UI state for the index "indexName" is not consistent with the widgets mounted. -This can happen when the UI state is specified via \`initialUiState\` or \`routing\` but that the widgets responsible for this state were not added. This results in query parameters not being sent to the API. +This can happen when the UI state is specified via \`initialUiState\` or \`routing\` but that the widgets responsible for this state were not added. This results in those query parameters not being sent to the API. To fully reflect the state, some widgets need to be added to the index "indexName": diff --git a/src/widgets/index/index.ts b/src/widgets/index/index.ts index 2e5ddd1f32..8f4d4b591c 100644 --- a/src/widgets/index/index.ts +++ b/src/widgets/index/index.ts @@ -393,7 +393,7 @@ const index = (props: IndexProps): Index => { missingWidgets.length === 0, `The UI state for the index "${this.getIndexId()}" is not consistent with the widgets mounted. -This can happen when the UI state is specified via \`initialUiState\` or \`routing\` but that the widgets responsible for this state were not added. This results in query parameters not being sent to the API. +This can happen when the UI state is specified via \`initialUiState\` or \`routing\` but that the widgets responsible for this state were not added. This results in those query parameters not being sent to the API. To fully reflect the state, some widgets need to be added to the index "${this.getIndexId()}": From 0ffe81e8c3ecad6c14868b6b7b4c60b763b69b6c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Chalifour?= Date: Mon, 30 Sep 2019 13:05:18 +0200 Subject: [PATCH 4/6] refactor: change data structure to tuples --- src/lib/__tests__/InstantSearch-test.js | 8 +++- src/widgets/index/index.ts | 54 ++++++++++++++----------- 2 files changed, 38 insertions(+), 24 deletions(-) diff --git a/src/lib/__tests__/InstantSearch-test.js b/src/lib/__tests__/InstantSearch-test.js index 4f23ecca8c..549cea29ff 100644 --- a/src/lib/__tests__/InstantSearch-test.js +++ b/src/lib/__tests__/InstantSearch-test.js @@ -1220,6 +1220,9 @@ describe('UI state', () => { hierarchicalMenu: { categories: 'Mobile', }, + range: { + price: '100:200', + }, }, }, }); @@ -1241,6 +1244,7 @@ To fully reflect the state, some widgets need to be added to the index "indexNam - \`page\` needs one of these widgets: "pagination", "infiniteHits" - \`refinementList\` needs one of these widgets: "refinementList" - \`hierarchicalMenu\` needs one of these widgets: "hierarchicalMenu" +- \`range\` needs one of these widgets: "rangeInput", "rangeSlider" If you do not wish to display widgets but still want to support their search parameters, you can mount "virtual widgets" that don't render anything: @@ -1248,11 +1252,13 @@ If you do not wish to display widgets but still want to support their search par const virtualPagination = connectPagination(() => null); const virtualRefinementList = connectRefinementList(() => null); const virtualHierarchicalMenu = connectHierarchicalMenu(() => null); +const virtualRange = connectRange(() => null); search.addWidgets([ virtualPagination({ /* ... */ }), virtualRefinementList({ /* ... */ }), - virtualHierarchicalMenu({ /* ... */ }) + virtualHierarchicalMenu({ /* ... */ }), + virtualRange({ /* ... */ }) ]); \`\`\` diff --git a/src/widgets/index/index.ts b/src/widgets/index/index.ts index 8f4d4b591c..d9b249376a 100644 --- a/src/widgets/index/index.ts +++ b/src/widgets/index/index.ts @@ -346,6 +346,18 @@ const index = (props: IndexProps): Index => { instantSearchInstance.scheduleStalledRender(); if (__DEV__) { + // Some connectors are responsible for multiple widgets so we need + // to map them. + function getWidgetNames(connectorName: string): string[] { + switch (connectorName) { + case 'range': + return ['rangeInput', 'rangeSlider']; + + default: + return [connectorName]; + } + } + type StateToWidgets = { [TParameter in keyof IndexUiState]: Array; }; @@ -370,8 +382,10 @@ const index = (props: IndexProps): Index => { .map(widget => widget.$$type) .filter(Boolean); + type MissingWidgets = Array<[string, Array]>; + const missingWidgets = Object.keys(localUiState).reduce< - Array> + MissingWidgets >((acc, parameter) => { const requiredWidgets: Array = stateToWidgetsMap[parameter]; @@ -381,9 +395,13 @@ const index = (props: IndexProps): Index => { mountedWidgets.includes(requiredWidget) ) ) { - acc.push({ - [parameter]: stateToWidgetsMap[parameter], - }); + acc.push([ + parameter, + stateToWidgetsMap[parameter].map( + (widgetIdentifier: string) => + widgetIdentifier.split('ais.')[1] + ), + ]); } return acc; @@ -398,15 +416,11 @@ This can happen when the UI state is specified via \`initialUiState\` or \`routi To fully reflect the state, some widgets need to be added to the index "${this.getIndexId()}": ${missingWidgets - .map(widget => { - const stateParameter = Object.keys(widget)[0]; - const neededWidgets = stateToWidgetsMap[stateParameter] - .map( - (widgetIdentifier: string) => `"${widgetIdentifier.split('ais.')[1]}"` - ) - .join(', '); - - return `- \`${stateParameter}\` needs one of these widgets: ${neededWidgets}`; + .map(([stateParameter, widgets]) => { + return `- \`${stateParameter}\` needs one of these widgets: ${([] as string[]) + .concat(...widgets.map(name => getWidgetNames(name!))) + .map((name: string) => `"${name}"`) + .join(', ')}`; }) .join('\n')} @@ -414,11 +428,8 @@ If you do not wish to display widgets but still want to support their search par \`\`\` ${missingWidgets - .map(widget => { - const stateParameter = Object.keys(widget)[0]; - const capitalizedWidget = capitalize( - stateToWidgetsMap[stateParameter][0].split('ais.')[1] - ); + .map(([_stateParameter, widgets]) => { + const capitalizedWidget = capitalize(widgets[0]!); return `const virtual${capitalizedWidget} = connect${capitalizedWidget}(() => null);`; }) @@ -426,11 +437,8 @@ ${missingWidgets search.addWidgets([ ${missingWidgets - .map(widget => { - const stateParameter = Object.keys(widget)[0]; - const capitalizedWidget = capitalize( - stateToWidgetsMap[stateParameter][0].split('ais.')[1] - ); + .map(([_stateParameter, widgets]) => { + const capitalizedWidget = capitalize(widgets[0]!); return `virtual${capitalizedWidget}({ /* ... */ })`; }) From 01de0f9eab34a99b9af13abe71452ab69f00b0a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Chalifour?= Date: Mon, 30 Sep 2019 14:42:37 +0200 Subject: [PATCH 5/6] chore: disable ESLint rule for no-inner-declarations in DEV branch --- src/widgets/index/index.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/widgets/index/index.ts b/src/widgets/index/index.ts index d9b249376a..16e776b6e1 100644 --- a/src/widgets/index/index.ts +++ b/src/widgets/index/index.ts @@ -348,6 +348,7 @@ const index = (props: IndexProps): Index => { if (__DEV__) { // Some connectors are responsible for multiple widgets so we need // to map them. + // eslint-disable-next-line no-inner-declarations function getWidgetNames(connectorName: string): string[] { switch (connectorName) { case 'range': From ee80795527f9c50d53c82243bffb2e6f881468f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Chalifour?= Date: Mon, 30 Sep 2019 14:46:46 +0200 Subject: [PATCH 6/6] fix: add menu widgets --- src/lib/__tests__/InstantSearch-test.js | 8 +++++++- src/widgets/index/index.ts | 3 +++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/lib/__tests__/InstantSearch-test.js b/src/lib/__tests__/InstantSearch-test.js index 549cea29ff..e757f24175 100644 --- a/src/lib/__tests__/InstantSearch-test.js +++ b/src/lib/__tests__/InstantSearch-test.js @@ -1223,6 +1223,9 @@ describe('UI state', () => { range: { price: '100:200', }, + menu: { + category: 'Hardware', + }, }, }, }); @@ -1245,6 +1248,7 @@ To fully reflect the state, some widgets need to be added to the index "indexNam - \`refinementList\` needs one of these widgets: "refinementList" - \`hierarchicalMenu\` needs one of these widgets: "hierarchicalMenu" - \`range\` needs one of these widgets: "rangeInput", "rangeSlider" +- \`menu\` needs one of these widgets: "menu", "menuSelect" If you do not wish to display widgets but still want to support their search parameters, you can mount "virtual widgets" that don't render anything: @@ -1253,12 +1257,14 @@ const virtualPagination = connectPagination(() => null); const virtualRefinementList = connectRefinementList(() => null); const virtualHierarchicalMenu = connectHierarchicalMenu(() => null); const virtualRange = connectRange(() => null); +const virtualMenu = connectMenu(() => null); search.addWidgets([ virtualPagination({ /* ... */ }), virtualRefinementList({ /* ... */ }), virtualHierarchicalMenu({ /* ... */ }), - virtualRange({ /* ... */ }) + virtualRange({ /* ... */ }), + virtualMenu({ /* ... */ }) ]); \`\`\` diff --git a/src/widgets/index/index.ts b/src/widgets/index/index.ts index 16e776b6e1..edabb80a8a 100644 --- a/src/widgets/index/index.ts +++ b/src/widgets/index/index.ts @@ -354,6 +354,9 @@ const index = (props: IndexProps): Index => { case 'range': return ['rangeInput', 'rangeSlider']; + case 'menu': + return ['menu', 'menuSelect']; + default: return [connectorName]; }