Skip to content

Commit

Permalink
feat(index): support adding index widget with initial UI state (#4359)
Browse files Browse the repository at this point in the history
1. when hydrating (server or client) we add widgets, these shouldn't search yet, until started
since every added widget calls scheduleSearch if init has happened, this used to do a search, even before start was called, and thus with not yet the correct state

2. make sure local ui state keeps "not yet mounted" widgets on change
e.g. initialUiState contains a few widgets, if you mount those one by one, after init on its index, each widget would cause a change event on the helper, which used to erase non-valid state, since it starts from an empty ui state. This is now fixed by using the previous local ui state as base, but a special provision for isPageReset

3. prevent mutation in escapeHits
It's okay to do results.hits = ... inside connectHits, since that overrides just the value on SearchResults, however if we mutate within the inner hits, we are actually mutating _rawResults, which isn't what we want to achieve for server side rendering, since _rawResults gets serialised.

Further changes are just minor for tests
  • Loading branch information
Haroenv authored May 13, 2020
1 parent 4a00fa6 commit 5ff4c83
Show file tree
Hide file tree
Showing 6 changed files with 208 additions and 27 deletions.
4 changes: 3 additions & 1 deletion src/lib/InstantSearch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -483,7 +483,9 @@ See ${createDocumentationLink({
}

public scheduleSearch = defer(() => {
this.mainHelper!.search();
if (this.started) {
this.mainHelper!.search();
}
});

public scheduleRender = defer(() => {
Expand Down
22 changes: 22 additions & 0 deletions src/lib/__tests__/escape-highlight-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -246,4 +246,26 @@ describe('escapeHits()', () => {

expect(hits).toEqual(output);
});

it('should not mutate the hit', () => {
const hit = {
_highlightResult: {
foobar: {
value: '<script>__ais-highlight__foo__/ais-highlight__</script>',
},
},
};

const hits = [hit];

escapeHits(hits);

expect(hit).toEqual({
_highlightResult: {
foobar: {
value: '<script>__ais-highlight__foo__/ais-highlight__</script>',
},
},
});
});
});
12 changes: 7 additions & 5 deletions src/lib/escape-highlight.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,15 @@ function recursiveEscape(input: any): any {

export default function escapeHits(hits: Hit[]): Hit[] {
if ((hits as any).__escaped === undefined) {
hits = hits.map(hit => {
if (hit._highlightResult) {
hit._highlightResult = recursiveEscape(hit._highlightResult);
// We don't override the value on hit because it will mutate the raw results
// instead we make a shallow copy and we assign the escaped values on it.
hits = hits.map(({ _highlightResult, _snippetResult, ...hit }) => {
if (_highlightResult) {
hit._highlightResult = recursiveEscape(_highlightResult);
}

if (hit._snippetResult) {
hit._snippetResult = recursiveEscape(hit._snippetResult);
if (_snippetResult) {
hit._snippetResult = recursiveEscape(_snippetResult);
}

return hit;
Expand Down
109 changes: 109 additions & 0 deletions src/widgets/index/__tests__/index-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ describe('index', () => {
}),
getWidgetState(uiState) {
return {
...uiState,
configure: {
...uiState.configure,
...params,
Expand Down Expand Up @@ -255,6 +256,66 @@ See documentation: https://www.algolia.com/doc/api-reference/widgets/index-widge
});
});

it('forwards initial UiState to inner indices', () => {
const instance = index({ indexName: 'indexName' });
const instantSearchInstance = createInstantSearch({
_initialUiState: {
indexName: {
query: 'xxx',
},
two: {
query: 'inner',
},
},
});
const inner = index({ indexName: 'two' });
jest.spyOn(inner, 'init');

const widgets = [createSearchBox(), createPagination(), inner];
const innerWidgets = [createSearchBox()];

instance.init(
createInitOptions({
instantSearchInstance,
parent: null,
})
);

widgets.forEach(widget => {
expect(widget.init).toHaveBeenCalledTimes(0);
});

instance.addWidgets(widgets);

widgets.forEach(widget => {
expect(widget.init).toHaveBeenCalledTimes(1);
expect(widget.init).toHaveBeenCalledWith({
instantSearchInstance,
parent: instance,
uiState: {
indexName: {
query: 'xxx',
},
two: {
query: 'inner',
},
},
helper: instance.getHelper(),
state: instance.getHelper()!.state,
templatesConfig: instantSearchInstance.templatesConfig,
createURL: expect.any(Function),
});
});

inner.addWidgets(innerWidgets);

expect(inner.getWidgetState({})).toEqual({
two: {
query: 'inner',
},
});
});

it('schedules a search to take the added widgets into account', () => {
const instance = index({ indexName: 'indexName' });
const instantSearchInstance = createInstantSearch({
Expand Down Expand Up @@ -1783,6 +1844,54 @@ See documentation: https://www.algolia.com/doc/api-reference/widgets/index-widge
});
});
});

it('uiState on inner index does not get erased on addWidget', () => {
const level0 = index({ indexName: 'level0IndexName' });

const searchClient = createSearchClient();
const mainHelper = algoliasearchHelper(searchClient, '', {});
const instantSearchInstance = createInstantSearch({
mainHelper,
_initialUiState: {
level0IndexName: {
query: 'something',
},
},
});

instantSearchInstance.mainIndex.init(
createInitOptions({ instantSearchInstance, parent: null })
);

instantSearchInstance.mainIndex.addWidgets([level0]);

level0.init(
createInitOptions({
instantSearchInstance,
parent: instantSearchInstance.mainIndex,
})
);

level0.addWidgets([
createConfigure({
distinct: false,
}),
]);

level0.addWidgets([createSearchBox()]);

expect(level0.getHelper()!.state.query).toBe('something');

expect(instantSearchInstance.mainIndex.getWidgetState({})).toEqual({
indexName: {},
level0IndexName: {
configure: {
distinct: false,
},
query: 'something',
},
});
});
});

describe('render', () => {
Expand Down
76 changes: 62 additions & 14 deletions src/widgets/index/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import algoliasearchHelper, {
PlainSearchParameters,
SearchParameters,
SearchResults,
AlgoliaSearchHelper,
} from 'algoliasearch-helper';
import {
InstantSearch,
Expand Down Expand Up @@ -68,9 +69,39 @@ export function isIndexWidget(widget: Widget): widget is Index {
return widget.$$type === 'ais.index';
}

/**
* This is the same content as helper._change / setState, but allowing for extra
* UiState to be synchronized.
* see: https://github.com/algolia/algoliasearch-helper-js/blob/6b835ffd07742f2d6b314022cce6848f5cfecd4a/src/algoliasearch.helper.js#L1311-L1324
*/
function privateHelperSetState(
helper: AlgoliaSearchHelper,
{
state,
isPageReset,
_uiState,
}: {
state: SearchParameters;
isPageReset?: boolean;
_uiState?: IndexUiState;
}
) {
if (state !== helper.state) {
helper.state = state;

helper.emit('change', {
state: helper.state,
results: helper.lastResults,
isPageReset,
_uiState,
});
}
}

function getLocalWidgetsState(
widgets: Widget[],
widgetStateOptions: WidgetStateOptions
widgetStateOptions: WidgetStateOptions,
initialUiState: IndexUiState = {}
): IndexUiState {
return widgets
.filter(widget => !isIndexWidget(widget))
Expand All @@ -80,7 +111,7 @@ function getLocalWidgetsState(
}

return widget.getWidgetState(uiState, widgetStateOptions);
}, {});
}, initialUiState);
}

function getLocalWidgetsSearchParameters(
Expand Down Expand Up @@ -110,8 +141,11 @@ function resetPageFromWidgets(widgets: Widget[]): void {
indexWidgets.forEach(widget => {
const widgetHelper = widget.getHelper()!;

// @ts-ignore @TODO: remove "ts-ignore" once `resetPage()` is typed in the helper
widgetHelper.setState(widgetHelper.state.resetPage());
privateHelperSetState(widgetHelper, {
// @ts-ignore @TODO: remove "ts-ignore" once `resetPage()` is typed in the helper
state: widgetHelper.state.resetPage(),
isPageReset: true,
});

resetPageFromWidgets(widget.getWidgets());
});
Expand Down Expand Up @@ -213,19 +247,20 @@ const index = (props: IndexProps): Index => {
localWidgets = localWidgets.concat(widgets);

if (localInstantSearchInstance && Boolean(widgets.length)) {
helper!.setState(
getLocalWidgetsSearchParameters(localWidgets, {
privateHelperSetState(helper!, {
state: getLocalWidgetsSearchParameters(localWidgets, {
uiState: localUiState,
initialSearchParameters: helper!.state,
})
);
}),
_uiState: localUiState,
});

widgets.forEach(widget => {
if (localInstantSearchInstance && widget.init) {
widget.init({
helper: helper!,
parent: this,
uiState: {},
uiState: localInstantSearchInstance._initialUiState,
instantSearchInstance: localInstantSearchInstance,
state: helper!.state,
templatesConfig: localInstantSearchInstance.templatesConfig,
Expand Down Expand Up @@ -402,11 +437,24 @@ const index = (props: IndexProps): Index => {
// widgets. When the subscription happens before the `init` step, the (static)
// configuration of the widget is pushed in the URL. That's what we want to avoid.
// https://github.com/algolia/instantsearch.js/pull/994/commits/4a672ae3fd78809e213de0368549ef12e9dc9454
helper.on('change', ({ state }) => {
localUiState = getLocalWidgetsState(localWidgets, {
searchParameters: state,
helper: helper!,
});
helper.on('change', event => {
const { state, isPageReset } = event;

// @ts-ignore _uiState comes from privateHelperSetState and thus isn't typed on the helper event
const _uiState = event._uiState;

if (isPageReset) {
localUiState.page = undefined;
}

localUiState = getLocalWidgetsState(
localWidgets,
{
searchParameters: state,
helper: helper!,
},
_uiState || {}
);

// We don't trigger an internal change when controlled because it
// becomes the responsibility of `setUiState`.
Expand Down
12 changes: 5 additions & 7 deletions test/mock/createInstantSearch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,19 +12,17 @@ export const createInstantSearch = (
const mainHelper = algoliasearchHelper(searchClient, indexName, {});
const mainIndex = index({ indexName });

let started = false;

return {
indexName,
mainIndex,
mainHelper,
client: searchClient,
started,
start: () => {
started = true;
started: false,
start() {
this.started = true;
},
dispose: () => {
started = false;
dispose() {
this.started = false;
},
refresh: jest.fn(),
helper: mainHelper, // @TODO: use the Helper from the index once the RoutingManger uses the index
Expand Down

0 comments on commit 5ff4c83

Please sign in to comment.