Skip to content

Commit 7aca759

Browse files
authored
feat(server): reset scheduleSearch only after all components mount (#6650)
* wip: reset search scheduler based on callback * less api * better description * comment * chore * test correctly, we are server rendering because we're in ssr, there's no notion of eg useEffect and we also don't search twice
1 parent 2641a67 commit 7aca759

File tree

5 files changed

+57
-35
lines changed

5 files changed

+57
-35
lines changed

packages/instantsearch.js/src/lib/InstantSearch.ts

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,8 @@ class InstantSearch<
229229
public _searchStalledTimer: any;
230230
public _initialUiState: TUiState;
231231
public _initialResults: InitialResults | null;
232+
public _manuallyResetScheduleSearch: boolean = false;
233+
public _resetScheduleSearch?: () => void;
232234
public _createURL: CreateURL<TUiState>;
233235
public _searchFunction?: InstantSearchOptions['searchFunction'];
234236
public _mainHelperSearch?: AlgoliaSearchHelper['search'];
@@ -700,14 +702,25 @@ See documentation: ${createDocumentationLink({
700702
// because we already have the results to render. This skips the initial
701703
// network request on the browser on `start`.
702704
this.scheduleSearch = defer(noop);
703-
// We also skip the initial network request when widgets are dynamically
704-
// added in the first tick (that's the case in all the framework-based flavors).
705-
// When we add a widget to `index`, it calls `scheduleSearch`. We can rely
706-
// on our `defer` util to restore the original `scheduleSearch` value once
707-
// widgets are added to hook back to the regular lifecycle.
708-
defer(() => {
709-
this.scheduleSearch = originalScheduleSearch;
710-
})();
705+
if (this._manuallyResetScheduleSearch) {
706+
// If `_manuallyResetScheduleSearch` is passed, it means that we don't
707+
// want to rely on a single `defer` to reset the `scheduleSearch`.
708+
// Instead, the consumer will call `_resetScheduleSearch` to restore
709+
// the original `scheduleSearch` function.
710+
// This happens in the React flavour after rendering.
711+
this._resetScheduleSearch = () => {
712+
this.scheduleSearch = originalScheduleSearch;
713+
};
714+
} else {
715+
// We also skip the initial network request when widgets are dynamically
716+
// added in the first tick (that's the case in all the framework-based flavors).
717+
// When we add a widget to `index`, it calls `scheduleSearch`. We can rely
718+
// on our `defer` util to restore the original `scheduleSearch` value once
719+
// widgets are added to hook back to the regular lifecycle.
720+
defer(() => {
721+
this.scheduleSearch = originalScheduleSearch;
722+
})();
723+
}
711724
}
712725
// We only schedule a search when widgets have been added before `start()`
713726
// because there are listeners that can use these results.

packages/instantsearch.js/test/createInstantSearch.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ export const createInstantSearch = (
4646
_insights: undefined,
4747
_hasRecommendWidget: false,
4848
_hasSearchWidget: false,
49+
_manuallyResetScheduleSearch: false,
4950
onStateChange: null,
5051
setUiState: jest.fn(),
5152
getUiState: jest.fn(() => ({})),

packages/react-instantsearch-core/src/components/InstantSearch.tsx

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,13 @@
1-
import React from 'react';
1+
import React, { useEffect } from 'react';
22

33
import { IndexContext } from '../lib/IndexContext';
44
import { InstantSearchContext } from '../lib/InstantSearchContext';
55
import { useInstantSearchApi } from '../lib/useInstantSearchApi';
66

7-
import type { UseInstantSearchApiProps } from '../lib/useInstantSearchApi';
7+
import type {
8+
InternalInstantSearch,
9+
UseInstantSearchApiProps,
10+
} from '../lib/useInstantSearchApi';
811
import type {
912
InstantSearch as InstantSearchType,
1013
UiState,
@@ -33,7 +36,24 @@ export function InstantSearch<
3336
>
3437
<IndexContext.Provider value={search.mainIndex}>
3538
{children}
39+
<ResetScheduleSearch
40+
search={search as unknown as InternalInstantSearch<UiState, UiState>}
41+
/>
3642
</IndexContext.Provider>
3743
</InstantSearchContext.Provider>
3844
);
3945
}
46+
47+
function ResetScheduleSearch({
48+
search,
49+
}: {
50+
search: InternalInstantSearch<UiState, UiState>;
51+
}) {
52+
useEffect(() => {
53+
if (search._resetScheduleSearch) {
54+
search._resetScheduleSearch();
55+
}
56+
}, [search]);
57+
58+
return null;
59+
}

packages/react-instantsearch-core/src/lib/useInstantSearchApi.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,10 @@ export function useInstantSearchApi<TUiState extends UiState, TRouteState>(
108108
// an additional network request. (This is equivalent to monkey-patching
109109
// `scheduleSearch` to a noop.)
110110
search._initialResults = initialResults || {};
111+
// We don't rely on the `defer` to reset the schedule search, but will call
112+
// `search._resetScheduleSearch()` manually in the effect after children
113+
// mount in `InstantSearch`.
114+
search._manuallyResetScheduleSearch = true;
111115
}
112116

113117
addAlgoliaAgents(props.searchClient, [

packages/react-instantsearch-nextjs/src/__tests__/InitializePromise.test.tsx

Lines changed: 9 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,10 @@ import {
66
createSearchClient,
77
createSingleSearchResponse,
88
} from '@instantsearch/mocks';
9-
import { act, render } from '@testing-library/react';
109
import * as utils from 'instantsearch.js/es/lib/utils';
1110
import { ServerInsertedHTMLContext } from 'next/navigation';
1211
import React from 'react';
12+
import { renderToString } from 'react-dom/server';
1313
import { SearchBox, TrendingItems } from 'react-instantsearch';
1414
import {
1515
InstantSearch,
@@ -27,7 +27,7 @@ jest.mock('instantsearch.js/es/lib/utils', () => ({
2727
resetWidgetId: jest.fn(),
2828
}));
2929

30-
const renderComponent = ({
30+
const renderComponent = async ({
3131
children,
3232
ref = { current: null },
3333
nonce,
@@ -44,7 +44,7 @@ const renderComponent = ({
4444
}),
4545
});
4646

47-
render(
47+
renderToString(
4848
<InstantSearchRSCContext.Provider value={ref}>
4949
<InstantSearchSSRProvider>
5050
<InstantSearch searchClient={client} indexName="indexName">
@@ -60,6 +60,8 @@ const renderComponent = ({
6060
</InstantSearchRSCContext.Provider>
6161
);
6262

63+
await ref.current;
64+
6365
return client;
6466
};
6567

@@ -70,19 +72,13 @@ test('it calls resetWidgetId', () => {
7072
});
7173

7274
test('it applies provided nonce on the injected script tag', async () => {
73-
const ref: { current: PromiseWithState<void> | null } = { current: null };
7475
const insertedHTML = jest.fn();
75-
renderComponent({
76-
ref,
76+
await renderComponent({
7777
children: <SearchBox />,
7878
nonce: 'csp-nonce',
7979
insertedHTML,
8080
});
8181

82-
await act(async () => {
83-
await ref.current;
84-
});
85-
8682
expect(insertedHTML).toHaveBeenLastCalledWith(
8783
expect.objectContaining({
8884
props: expect.objectContaining({
@@ -95,7 +91,7 @@ test('it applies provided nonce on the injected script tag', async () => {
9591
test('it waits for both search and recommend results', async () => {
9692
const ref: { current: PromiseWithState<void> | null } = { current: null };
9793

98-
const client = renderComponent({
94+
const client = await renderComponent({
9995
ref,
10096
children: (
10197
<>
@@ -105,10 +101,6 @@ test('it waits for both search and recommend results', async () => {
105101
),
106102
});
107103

108-
await act(async () => {
109-
await ref.current;
110-
});
111-
112104
expect(ref.current!.status).toBe('fulfilled');
113105
expect(client.search).toHaveBeenCalledTimes(1);
114106
expect(client.getRecommendations).toHaveBeenCalledTimes(1);
@@ -117,7 +109,7 @@ test('it waits for both search and recommend results', async () => {
117109
test('it waits for search only if there are only search widgets', async () => {
118110
const ref: { current: PromiseWithState<void> | null } = { current: null };
119111

120-
const client = renderComponent({
112+
const client = await renderComponent({
121113
ref,
122114
children: (
123115
<>
@@ -126,10 +118,6 @@ test('it waits for search only if there are only search widgets', async () => {
126118
),
127119
});
128120

129-
await act(async () => {
130-
await ref.current;
131-
});
132-
133121
expect(ref.current!.status).toBe('fulfilled');
134122
expect(client.search).toHaveBeenCalledTimes(1);
135123
expect(client.search).toHaveBeenNthCalledWith(1, [
@@ -141,7 +129,7 @@ test('it waits for search only if there are only search widgets', async () => {
141129
test('it waits for recommend only if there are only recommend widgets', async () => {
142130
const ref: { current: PromiseWithState<void> | null } = { current: null };
143131

144-
const client = renderComponent({
132+
const client = await renderComponent({
145133
ref,
146134
children: (
147135
<>
@@ -150,10 +138,6 @@ test('it waits for recommend only if there are only recommend widgets', async ()
150138
),
151139
});
152140

153-
await act(async () => {
154-
await ref.current;
155-
});
156-
157141
expect(ref.current!.status).toBe('fulfilled');
158142
expect(client.search).not.toHaveBeenCalled();
159143
expect(client.getRecommendations).toHaveBeenCalledTimes(1);

0 commit comments

Comments
 (0)