Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(index): support adding index widget with initial UI state #4359

Merged
merged 16 commits into from
May 13, 2020
Merged
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
*/
Haroenv marked this conversation as resolved.
Show resolved Hide resolved
function privateHelperSetState(
helper: AlgoliaSearchHelper,
{
state,
isPageReset,
_uiState,
}: {
state: SearchParameters;
isPageReset?: boolean;
_uiState?: IndexUiState;
}
) {
if (state !== helper.state) {
Haroenv marked this conversation as resolved.
Show resolved Hide resolved
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, {
Haroenv marked this conversation as resolved.
Show resolved Hide resolved
// @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,
Haroenv marked this conversation as resolved.
Show resolved Hide resolved
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;
Haroenv marked this conversation as resolved.
Show resolved Hide resolved
}

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