Skip to content

Commit

Permalink
fix(state): change behaviour of preserveSharedStateOnUnmount (#6488)
Browse files Browse the repository at this point in the history
With preserveSharedStateOnUnmount behavior, UI State is the source of truth, when a widget gets removed, the UI State is evaluated again and the parameters result out of that.

[FX-3192]

Not done in this PR yet:
- changed the signature of dispose (no more helper and state)
- implement a similar behavior for recommend

BREAKING CHANGE: The option `future.preserveSharedStateOnUnmount` is removed and now behaves as if it was set to `true`
  • Loading branch information
Haroenv committed Dec 26, 2024
1 parent 882dbca commit b37ba30
Show file tree
Hide file tree
Showing 9 changed files with 22 additions and 173 deletions.
3 changes: 0 additions & 3 deletions examples/react/next/pages/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,6 @@ export default function HomePage({ serverState, url }: HomePageProps) {
}),
}}
insights={true}
future={{
preserveSharedStateOnUnmount: true,
}}
>
<div className="Container">
<div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1900,7 +1900,6 @@ const searchClient = algoliasearch('appId', 'apiKey');
const search = instantsearch({
indexName: 'indexName',
searchClient,
future: { preserveSharedStateOnUnmount: true },
insights: true,
});
Expand Down Expand Up @@ -4125,8 +4124,6 @@ import './App.css';
const searchClient = algoliasearch('appId', 'apiKey');
const future = { preserveSharedStateOnUnmount: true };
export function App() {
return (
<div>
Expand All @@ -4146,7 +4143,6 @@ export function App() {
<InstantSearch
searchClient={searchClient}
indexName=\\"indexName\\"
future={future}
insights
>
<Configure hitsPerPage={8} />
Expand Down Expand Up @@ -5218,7 +5214,6 @@ export default {
data() {
return {
searchClient: algoliasearch('appId', 'apiKey'),
future: { preserveSharedStateOnUnmount: true },
};
},
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ const searchClient = algoliasearch('{{appId}}', '{{apiKey}}');
const search = instantsearch({
indexName: '{{indexName}}',
searchClient,
future: { preserveSharedStateOnUnmount: true },
{{#if flags.insights}}insights: true,{{/if}}
});

Expand Down Expand Up @@ -174,4 +173,4 @@ function isModifierEvent(event) {
event.shiftKey
);
}
{{/if}}
{{/if}}
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,6 @@ const searchClient = algoliasearch(
'{{apiKey}}'
);

const future = { preserveSharedStateOnUnmount: true };

export function App() {
return (
<div>
Expand All @@ -49,7 +47,7 @@ export function App() {
</header>

<div className="container">
<InstantSearch searchClient={searchClient} indexName="{{indexName}}" future={future} {{#if flags.insights}}insights{{/if}}>
<InstantSearch searchClient={searchClient} indexName="{{indexName}}" {{#if flags.insights}}insights{{/if}}>
<Configure hitsPerPage={8} />
<div className="search-panel">
<div className="search-panel__filters">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,6 @@ export default {
data() {
return {
searchClient: algoliasearch('{{appId}}', '{{apiKey}}'),
future: { preserveSharedStateOnUnmount: true },
};
},
};
Expand Down
17 changes: 1 addition & 16 deletions packages/instantsearch-core/src/instantsearch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,7 @@ function defaultCreateURL() {

export const INSTANTSEARCH_FUTURE_DEFAULTS: Required<
InstantSearchOptions['future']
> = {
preserveSharedStateOnUnmount: false,
};
> = {};

/**
* The actual implementation of the InstantSearch. This is
Expand Down Expand Up @@ -151,19 +149,6 @@ See ${createDocumentationLink({
})}`
);

if (__DEV__ && options.future?.preserveSharedStateOnUnmount === undefined) {
// eslint-disable-next-line no-console
console.info(`Starting from the next major version, InstantSearch will change how widgets state is preserved when they are removed. InstantSearch will keep the state of unmounted widgets to be usable by other widgets with the same attribute.
We recommend setting \`future.preserveSharedStateOnUnmount\` to true to adopt this change today.
To stay with the current behaviour and remove this warning, set the option to false.
See documentation: ${createDocumentationLink({
name: 'instantsearch',
})}#widget-param-future
`);
}

this.client = searchClient;
this.future = future;
this.indexName = indexName;
Expand Down
13 changes: 1 addition & 12 deletions packages/instantsearch-core/src/types/instantsearch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,18 +90,7 @@ export type InstantSearchOptions<
* @default false
*/
insights?: InsightsProps | boolean;
future?: {
/**
* Changes the way `dispose` is used in InstantSearch lifecycle.
*
* If `false` (by default), each widget unmounting will remove its state as well, even if there are multiple widgets reading that UI State.
*
* If `true`, each widget unmounting will only remove its own state if it's the last of its type. This allows for dynamically adding and removing widgets without losing their state.
*
* @default false
*/
preserveSharedStateOnUnmount?: boolean; // @MAJOR remove option, only keep the "true" behaviour
};
future?: Record<string, never>;
};

export type InstantSearchStatus = 'idle' | 'loading' | 'stalled' | 'error';
122 changes: 10 additions & 112 deletions packages/instantsearch-core/src/widgets/__tests__/index-widget.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -603,11 +603,9 @@ See documentation: https://www.algolia.com/doc/api-reference/widgets/index-widge
hitsPerPage: 5,
});

instance.addWidgets([
configureTopLevel,
configureSubLevel,
virtualSearchBox({}),
]);
const searchBox = virtualSearchBox({});

instance.addWidgets([configureTopLevel, configureSubLevel, searchBox]);

instance.init(
createIndexInitOptions({
Expand All @@ -628,12 +626,12 @@ See documentation: https://www.algolia.com/doc/api-reference/widgets/index-widge
})
);

instance.removeWidgets([configureSubLevel]);
instance.removeWidgets([searchBox]);

expect(instance.getHelper()!.state).toEqual(
new SearchParameters({
index: 'indexName',
query: 'Apple iPhone',
hitsPerPage: 5,
distinct: true,
})
);
Expand All @@ -646,111 +644,9 @@ See documentation: https://www.algolia.com/doc/api-reference/widgets/index-widge
).toHaveBeenCalledTimes(2);
});

it('cleans shared refinements when `preserveSharedStateOnUnmount` is unset', () => {
it('preserves shared refinements', () => {
const instance = index({ indexName: 'indexName' });
const instantSearchInstance = createInstantSearch();

const refinementList1 = virtualRefinementList({
attribute: 'brand',
});

const refinementList2 = virtualRefinementList({
attribute: 'brand',
});

instance.addWidgets([refinementList1, refinementList2]);

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

// Simulate a state change
instance.getHelper()!.addDisjunctiveFacetRefinement('brand', 'Apple');

expect(instance.getHelper()!.state).toEqual(
new SearchParameters({
index: 'indexName',
maxValuesPerFacet: 10,
disjunctiveFacets: ['brand'],
disjunctiveFacetsRefinements: {
brand: ['Apple'],
},
})
);

instance.removeWidgets([refinementList2]);

expect(instance.getHelper()!.state).toEqual(
new SearchParameters({
index: 'indexName',
maxValuesPerFacet: 10,
disjunctiveFacets: ['brand'],
disjunctiveFacetsRefinements: {
brand: [],
},
})
);
});

it('cleans shared refinements when `preserveSharedStateOnUnmount` is false', () => {
const instance = index({ indexName: 'indexName' });
const instantSearchInstance = createInstantSearch({
future: { preserveSharedStateOnUnmount: false },
});

const refinementList1 = virtualRefinementList({
attribute: 'brand',
});

const refinementList2 = virtualRefinementList({
attribute: 'brand',
});

instance.addWidgets([refinementList1, refinementList2]);

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

// Simulate a state change
instance.getHelper()!.addDisjunctiveFacetRefinement('brand', 'Apple');

expect(instance.getHelper()!.state).toEqual(
new SearchParameters({
index: 'indexName',
maxValuesPerFacet: 10,
disjunctiveFacets: ['brand'],
disjunctiveFacetsRefinements: {
brand: ['Apple'],
},
})
);

instance.removeWidgets([refinementList2]);

expect(instance.getHelper()!.state).toEqual(
new SearchParameters({
index: 'indexName',
maxValuesPerFacet: 10,
disjunctiveFacets: ['brand'],
disjunctiveFacetsRefinements: {
brand: [],
},
})
);
});

it('preserves shared refinements when `preserveSharedStateOnUnmount` is true', () => {
const instance = index({ indexName: 'indexName' });
const instantSearchInstance = createInstantSearch({
future: { preserveSharedStateOnUnmount: true },
});
const instantSearchInstance = createInstantSearch({});

const refinementList1 = virtualRefinementList({
attribute: 'brand',
Expand Down Expand Up @@ -816,13 +712,15 @@ See documentation: https://www.algolia.com/doc/api-reference/widgets/index-widge
expect(widget.dispose).toHaveBeenCalledTimes(0);
});

const stateBefore = instance.getHelper()!.state;

instance.removeWidgets(widgets);

widgets.forEach((widget) => {
expect(widget.dispose).toHaveBeenCalledTimes(1);
expect(widget.dispose).toHaveBeenCalledWith({
helper: instance.getHelper(),
state: instance.getHelper()!.state,
state: stateBefore,
recommendState: instance.getHelper()!.recommendState,
parent: instance,
});
Expand Down
27 changes: 8 additions & 19 deletions packages/instantsearch-core/src/widgets/index-widget.ts
Original file line number Diff line number Diff line change
Expand Up @@ -438,8 +438,9 @@ export const index = (widgetParams: IndexWidgetParams): IndexWidget => {
});

if (localInstantSearchInstance && Boolean(widgets.length)) {
const { cleanedSearchState, cleanedRecommendState } = widgets.reduce(
const { cleanedRecommendState } = widgets.reduce(
(states, widget) => {
// @MAJOR remove the "cleanup" part of the dispose method
// the `dispose` method exists at this point we already assert it
const next = widget.dispose!({
helper: helper!,
Expand All @@ -462,24 +463,12 @@ export const index = (widgetParams: IndexWidgetParams): IndexWidget => {
}
);

const newState = localInstantSearchInstance.future
.preserveSharedStateOnUnmount
? getLocalWidgetsSearchParameters(localWidgets, {
uiState: localUiState,
initialSearchParameters: new algoliasearchHelper.SearchParameters(
{
index: this.getIndexName(),
}
),
})
: getLocalWidgetsSearchParameters(localWidgets, {
uiState: getLocalWidgetsUiState(localWidgets, {
searchParameters: cleanedSearchState,
helper: helper!,
}),
initialSearchParameters: cleanedSearchState,
});

const newState = getLocalWidgetsSearchParameters(localWidgets, {
uiState: localUiState,
initialSearchParameters: new algoliasearchHelper.SearchParameters({
index: this.getIndexName(),
}),
});
localUiState = getLocalWidgetsUiState(localWidgets, {
searchParameters: newState,
helper: helper!,
Expand Down

0 comments on commit b37ba30

Please sign in to comment.