Skip to content

Commit 0f5d688

Browse files
Haroenvfrancoischalifour
andauthoredMar 30, 2021
fix(setUiState): make sure previous ui state is stored (#4699)
* fix(setUiState): make sure previous ui state is stored currently if you call setUiState, the localUiState in index widgets doesn't get updated to prevent middlewares being called multiple times, but that means if you call setUiState multiple times, the local state doesn't change * fix typo * proposed fix * make it async * tests * just use * Apply suggestions from code review Co-authored-by: François Chalifour <francoischalifour@users.noreply.github.com> Co-authored-by: François Chalifour <francoischalifour@users.noreply.github.com>
1 parent f3213b2 commit 0f5d688

File tree

5 files changed

+106
-16
lines changed

5 files changed

+106
-16
lines changed
 

‎src/lib/InstantSearch.ts

+3-4
Original file line numberDiff line numberDiff line change
@@ -556,7 +556,7 @@ See ${createDocumentationLink({
556556
});
557557
}
558558

559-
indexWidget.getHelper()!.overrideStateWithoutTriggeringChangeEvent(
559+
indexWidget.getHelper()!.setState(
560560
indexWidget.getWidgetSearchParameters(indexWidget.getHelper()!.state, {
561561
uiState: nextUiState[indexWidget.getIndexId()],
562562
})
@@ -571,18 +571,17 @@ See ${createDocumentationLink({
571571
setIndexHelperState(this.mainIndex);
572572

573573
this.scheduleSearch();
574-
this.onInternalStateChange();
575574
}
576575

577-
public onInternalStateChange = () => {
576+
public onInternalStateChange = defer(() => {
578577
const nextUiState = this.mainIndex.getWidgetUiState({});
579578

580579
this.middleware.forEach(m => {
581580
m.onStateChange({
582581
uiState: nextUiState,
583582
});
584583
});
585-
};
584+
});
586585

587586
public createURL(nextState: UiState = {}): string {
588587
if (!this.started) {

‎src/lib/__tests__/InstantSearch-test.js

+79-8
Original file line numberDiff line numberDiff line change
@@ -1409,7 +1409,7 @@ describe('refresh', () => {
14091409
});
14101410

14111411
describe('use', () => {
1412-
it('hooks middleware into the lifecycle before the instance starts', () => {
1412+
it('hooks middleware into the lifecycle before the instance starts', async () => {
14131413
const searchClient = createSearchClient();
14141414
const search = new InstantSearch({
14151415
indexName: 'indexName',
@@ -1434,7 +1434,7 @@ describe('use', () => {
14341434
const middleware = jest.fn(() => middlewareSpy);
14351435

14361436
search.addWidgets([searchBox]);
1437-
search.EXPERIMENTAL_use(middleware);
1437+
search.use(middleware);
14381438

14391439
expect(middleware).toHaveBeenCalledTimes(1);
14401440
expect(middleware).toHaveBeenCalledWith({ instantSearchInstance: search });
@@ -1450,11 +1450,15 @@ describe('use', () => {
14501450
// Checks that `mainIndex.init` was called before subscribing the middleware.
14511451
expect(widgetsInitCallOrder).toBeLessThan(middlewareSubscribeCallOrder);
14521452

1453+
await runAllMicroTasks();
1454+
14531455
expect(middlewareSpy.subscribe).toHaveBeenCalledTimes(1);
14541456
expect(middlewareSpy.onStateChange).toHaveBeenCalledTimes(0);
14551457

14561458
button.click();
14571459

1460+
await runAllMicroTasks();
1461+
14581462
expect(middlewareSpy.onStateChange).toHaveBeenCalledTimes(1);
14591463
expect(middlewareSpy.onStateChange).toHaveBeenCalledWith({
14601464
uiState: {
@@ -1466,6 +1470,8 @@ describe('use', () => {
14661470

14671471
search.dispose();
14681472

1473+
await runAllMicroTasks();
1474+
14691475
expect(middlewareSpy.onStateChange).toHaveBeenCalledTimes(2);
14701476
expect(middlewareSpy.onStateChange).toHaveBeenCalledWith({
14711477
uiState: {
@@ -1475,7 +1481,7 @@ describe('use', () => {
14751481
expect(middlewareSpy.unsubscribe).toHaveBeenCalledTimes(1);
14761482
});
14771483

1478-
it('hooks middleware into the lifecycle after the instance starts', () => {
1484+
it('hooks middleware into the lifecycle after the instance starts', async () => {
14791485
const searchClient = createSearchClient();
14801486
const search = new InstantSearch({
14811487
indexName: 'indexName',
@@ -1503,15 +1509,15 @@ describe('use', () => {
15031509
const middlewareAfterStart = jest.fn(() => middlewareAfterStartSpy);
15041510

15051511
search.addWidgets([searchBox({})]);
1506-
search.EXPERIMENTAL_use(middlewareBeforeStart);
1512+
search.use(middlewareBeforeStart);
15071513
search.start();
15081514

15091515
expect(middlewareBeforeStart).toHaveBeenCalledTimes(1);
15101516
expect(middlewareBeforeStart).toHaveBeenCalledWith({
15111517
instantSearchInstance: search,
15121518
});
15131519

1514-
search.EXPERIMENTAL_use(middlewareAfterStart);
1520+
search.use(middlewareAfterStart);
15151521

15161522
// The first middleware should still have been only called once
15171523
expect(middlewareBeforeStart).toHaveBeenCalledTimes(1);
@@ -1520,6 +1526,8 @@ describe('use', () => {
15201526
instantSearchInstance: search,
15211527
});
15221528

1529+
await runAllMicroTasks();
1530+
15231531
// The first middleware subscribe function should have been only called once
15241532
expect(middlewareBeforeStartSpy.subscribe).toHaveBeenCalledTimes(1);
15251533
expect(middlewareAfterStartSpy.subscribe).toHaveBeenCalledTimes(1);
@@ -1528,6 +1536,8 @@ describe('use', () => {
15281536

15291537
button.click();
15301538

1539+
await runAllMicroTasks();
1540+
15311541
expect(middlewareBeforeStartSpy.onStateChange).toHaveBeenCalledTimes(1);
15321542
expect(middlewareAfterStartSpy.onStateChange).toHaveBeenCalledTimes(1);
15331543
expect(middlewareBeforeStartSpy.onStateChange).toHaveBeenCalledWith({
@@ -1547,6 +1557,8 @@ describe('use', () => {
15471557

15481558
search.dispose();
15491559

1560+
await runAllMicroTasks();
1561+
15501562
expect(middlewareBeforeStartSpy.onStateChange).toHaveBeenCalledTimes(2);
15511563
expect(middlewareAfterStartSpy.onStateChange).toHaveBeenCalledTimes(2);
15521564
expect(middlewareBeforeStartSpy.onStateChange).toHaveBeenCalledWith({
@@ -1603,7 +1615,7 @@ See documentation: https://www.algolia.com/doc/api-reference/widgets/instantsear
16031615
expect(searchClient.search).toHaveBeenCalledTimes(2);
16041616
});
16051617

1606-
test('notifies all middleware', () => {
1618+
test('notifies all middlewares', async () => {
16071619
const searchClient = createSearchClient();
16081620
const search = new InstantSearch({
16091621
indexName: 'indexName',
@@ -1618,13 +1630,16 @@ See documentation: https://www.algolia.com/doc/api-reference/widgets/instantsear
16181630
};
16191631
};
16201632

1621-
search.EXPERIMENTAL_use(middleware);
1633+
search.use(middleware);
16221634
search.start();
16231635
expect(onMiddlewareStateChange).toHaveBeenCalledTimes(0);
16241636

16251637
search.setUiState({
16261638
indexName: {},
16271639
});
1640+
1641+
await runAllMicroTasks();
1642+
16281643
expect(onMiddlewareStateChange).toHaveBeenCalledTimes(1);
16291644
expect(onMiddlewareStateChange).toHaveBeenCalledWith({
16301645
uiState: {
@@ -1633,6 +1648,62 @@ See documentation: https://www.algolia.com/doc/api-reference/widgets/instantsear
16331648
});
16341649
});
16351650

1651+
test('notifies all middlewares in multi-index when called multiple times', async () => {
1652+
const searchClient = createSearchClient();
1653+
const search = new InstantSearch({
1654+
indexName: 'indexName',
1655+
searchClient,
1656+
});
1657+
1658+
search.addWidgets([
1659+
connectSearchBox(() => {})({}),
1660+
index({ indexName: 'test' }),
1661+
]);
1662+
1663+
const onMiddlewareStateChange = jest.fn();
1664+
const middleware = () => {
1665+
return {
1666+
subscribe() {},
1667+
unsubscribe() {},
1668+
onStateChange: onMiddlewareStateChange,
1669+
};
1670+
};
1671+
1672+
search.use(middleware);
1673+
search.start();
1674+
expect(onMiddlewareStateChange).toHaveBeenCalledTimes(0);
1675+
1676+
search.setUiState({
1677+
indexName: {},
1678+
test: {},
1679+
});
1680+
1681+
await runAllMicroTasks();
1682+
1683+
expect(onMiddlewareStateChange).toHaveBeenCalledTimes(1);
1684+
expect(onMiddlewareStateChange).toHaveBeenCalledWith({
1685+
uiState: {
1686+
indexName: {},
1687+
test: {},
1688+
},
1689+
});
1690+
1691+
search.setUiState({
1692+
indexName: { query: 'test' },
1693+
test: {},
1694+
});
1695+
1696+
await runAllMicroTasks();
1697+
1698+
expect(onMiddlewareStateChange).toHaveBeenCalledTimes(2);
1699+
expect(onMiddlewareStateChange).toHaveBeenCalledWith({
1700+
uiState: {
1701+
indexName: { query: 'test' },
1702+
test: {},
1703+
},
1704+
});
1705+
});
1706+
16361707
test('with object form sets indices state', async () => {
16371708
const searchClient = createSearchClient();
16381709
const search = new InstantSearch({
@@ -1902,7 +1973,7 @@ describe('onStateChange', () => {
19021973
};
19031974

19041975
search.addWidgets([searchBox({})]);
1905-
search.EXPERIMENTAL_use(middleware);
1976+
search.use(middleware);
19061977
search.start();
19071978

19081979
expect(middlewareOnStateChange).toHaveBeenCalledTimes(0);

‎src/lib/__tests__/RoutingManager-test.ts

+17-1
Original file line numberDiff line numberDiff line change
@@ -138,15 +138,19 @@ describe('RoutingManager', () => {
138138

139139
search.start();
140140

141-
search.once('render', () => {
141+
search.once('render', async () => {
142142
// initialization is done at this point
143143
expect(widget.render).toHaveBeenCalledTimes(1);
144144
expect(widget.getWidgetSearchParameters).toHaveBeenCalledTimes(1);
145145

146+
await runAllMicroTasks();
147+
146148
expect(router.write).toHaveBeenCalledTimes(0);
147149

148150
search.mainIndex.getHelper()!.setQuery('q'); // routing write updates on change
149151

152+
await runAllMicroTasks();
153+
150154
expect(router.write).toHaveBeenCalledTimes(1);
151155
expect(router.write).toHaveBeenCalledWith({
152156
indexName: {
@@ -301,6 +305,8 @@ describe('RoutingManager', () => {
301305
// Trigger an update - push a change
302306
fakeSearchBox.refine('Apple');
303307

308+
await runAllMicroTasks();
309+
304310
expect(router.write).toHaveBeenCalledTimes(1);
305311
expect(router.write).toHaveBeenLastCalledWith({
306312
indexName: {
@@ -403,6 +409,8 @@ describe('RoutingManager', () => {
403409
// Trigger an update - push a change
404410
fakeSearchBox.refine('Apple');
405411

412+
await runAllMicroTasks();
413+
406414
expect(router.write).toHaveBeenCalledTimes(1);
407415
expect(router.write).toHaveBeenLastCalledWith({
408416
indexName: {
@@ -413,6 +421,8 @@ describe('RoutingManager', () => {
413421
// Trigger an update - push a change
414422
fakeSearchBox.refine('Apple iPhone');
415423

424+
await runAllMicroTasks();
425+
416426
expect(router.write).toHaveBeenCalledTimes(2);
417427
expect(router.write).toHaveBeenLastCalledWith({
418428
indexName: {
@@ -492,6 +502,8 @@ describe('RoutingManager', () => {
492502
// Trigger an update - push a change
493503
fakeSearchBox.refine('Apple');
494504

505+
await runAllMicroTasks();
506+
495507
expect(router.write).toHaveBeenCalledTimes(1);
496508
expect(router.write).toHaveBeenLastCalledWith({
497509
indexName: {
@@ -502,6 +514,8 @@ describe('RoutingManager', () => {
502514
// Trigger change without UI state change
503515
search.removeWidgets([fakeHitsPerPage1]);
504516

517+
await runAllMicroTasks();
518+
505519
expect(router.write).toHaveBeenCalledTimes(1);
506520

507521
await runAllMicroTasks();
@@ -510,6 +524,8 @@ describe('RoutingManager', () => {
510524
// Trigger change without UI state change but with a route change
511525
search.removeWidgets([fakeHitsPerPage2]);
512526

527+
await runAllMicroTasks();
528+
513529
expect(router.write).toHaveBeenCalledTimes(2);
514530
expect(router.write).toHaveBeenLastCalledWith({
515531
indexName: {

‎src/widgets/index/__tests__/index-test.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -1904,7 +1904,7 @@ See documentation: https://www.algolia.com/doc/api-reference/widgets/index-widge
19041904
it('updates the local `uiState` when they differ on first render', () => {
19051905
const instance = index({ indexName: 'indexName' });
19061906
const instantSearchInstance = createInstantSearch({
1907-
onInternalStateChange: jest.fn(),
1907+
onInternalStateChange: jest.fn() as any,
19081908
});
19091909

19101910
instance.addWidgets([createSearchBox()]);
@@ -1967,7 +1967,7 @@ See documentation: https://www.algolia.com/doc/api-reference/widgets/index-widge
19671967
const topLevelInstance = index({ indexName: 'topLevelIndexName' });
19681968
const subLevelInstance = index({ indexName: 'subLevelIndexName' });
19691969
const instantSearchInstance = createInstantSearch({
1970-
onInternalStateChange: jest.fn(),
1970+
onInternalStateChange: jest.fn() as any,
19711971
});
19721972

19731973
topLevelInstance.addWidgets([

‎test/mock/createInstantSearch.ts

+5-1
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,11 @@ export const createInstantSearch = (
3939
_createURL: jest.fn(() => '#'),
4040
onStateChange: null,
4141
setUiState: jest.fn(),
42-
onInternalStateChange: jest.fn(),
42+
// Since we defer `onInternalStateChange` with our `defer` util which
43+
// creates a scoped deferred function, we're not able to spy that method.
44+
// We'll therefore need to override it when calling `createInstantSearch`.
45+
// See https://github.com/algolia/instantsearch.js/blob/f3213b2f118d75acac31a1f6cf4640241c438e9d/src/lib/utils/defer.ts#L13-L28
46+
onInternalStateChange: jest.fn() as any,
4347
createURL: jest.fn(() => '#'),
4448
addWidget: jest.fn(),
4549
addWidgets: jest.fn(),

0 commit comments

Comments
 (0)
Please sign in to comment.