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 overidde the value on hit because it will mutate the raw results
Haroenv marked this conversation as resolved.
Show resolved Hide resolved
// instead we make a shallow copy and we assigin the escaped values on it.
Haroenv marked this conversation as resolved.
Show resolved Hide resolved
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
52 changes: 52 additions & 0 deletions src/widgets/index/__tests__/index-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1783,6 +1783,58 @@ See documentation: https://www.algolia.com/doc/api-reference/widgets/index-widge
});
});
});

it('uiState on inner index does not get erased on "change"', () => {
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.addWidgets([
createConfigure({
distinct: false,
}),
createSearchBox(),
]),
]);

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

// Setting a page is considered as a change
level0
.getHelper()!
.setQueryParameter('page', 4)
.search();

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

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

describe('render', () => {
Expand Down
31 changes: 22 additions & 9 deletions src/widgets/index/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,8 @@ export function isIndexWidget(widget: Widget): widget is Index {

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

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

function getLocalWidgetsSearchParameters(
Expand Down Expand Up @@ -110,8 +111,12 @@ 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());
// @ts-ignore remove once resetPage is a helper method
widgetHelper._change({
// @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 @@ -225,7 +230,10 @@ const index = (props: IndexProps): Index => {
widget.init({
helper: helper!,
parent: this,
uiState: {},
// Only index widget uses this key. This means that the initial value
// is sufficient, since that index didn't yet exist. If we see wrong
// behavior with this, use mainIndex.getWidgetState()
Haroenv marked this conversation as resolved.
Show resolved Hide resolved
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 @@ -353,6 +361,7 @@ const index = (props: IndexProps): Index => {
// listener on `change` below, once `init` is done.
helper.on('change', ({ isPageReset }) => {
if (isPageReset) {
localUiState.page = undefined;
Haroenv marked this conversation as resolved.
Show resolved Hide resolved
resetPageFromWidgets(localWidgets);
}
});
Expand Down Expand Up @@ -403,10 +412,14 @@ const index = (props: IndexProps): Index => {
// 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!,
});
localUiState = getLocalWidgetsState(
localWidgets,
{
searchParameters: state,
helper: helper!,
},
localUiState
Haroenv marked this conversation as resolved.
Show resolved Hide resolved
);

// 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