Skip to content

Commit

Permalink
chore(enhanceConfiguration): prevent helper to be superfluously creat…
Browse files Browse the repository at this point in the history
…ed (#3959)

* fix(enhanceConfiguration): create SearchParameters once less

* test: add test for hierarchicalFacets

this test failed with the old mergeDeep-based enhanceConfiguration

* create helpers outside of enhanceConfiguration

* chore: more consistent type

* getConfiguration now gets called with SearchParameters only

* update existing TS widgets to make tsc pass

* chore: change conditional style to be more readable

* chore: clarify TODO

* avoid extra constructor call

* chore: go back to old behaviour, test later

* chore: remove import

* chore: reuse parameters instance
  • Loading branch information
Haroenv committed Oct 23, 2019
1 parent 344d1b7 commit 89be615
Show file tree
Hide file tree
Showing 12 changed files with 92 additions and 86 deletions.
9 changes: 3 additions & 6 deletions src/connectors/configure/connectConfigure.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,12 +61,9 @@ export default function connectConfigure(renderFn = noop, unmountFn = noop) {
return searchParameters => {
// merge new `searchParameters` with the ones set from other widgets
const actualState = this.removeSearchParameters(helper.state);
const nextSearchParameters = enhanceConfiguration(
{ ...actualState },
{
getConfiguration: () => searchParameters,
}
);
const nextSearchParameters = enhanceConfiguration(actualState, {
getConfiguration: () => searchParameters,
});

// trigger a search with the new merged searchParameters
helper.setState(nextSearchParameters).search();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -660,7 +660,9 @@ See documentation: https://www.algolia.com/doc/api-reference/widgets/infinite-hi
const makeWidget = connectInfiniteHits(renderFn);
const widget = makeWidget({});

const nextConfiguration = widget.getConfiguration!();
const nextConfiguration = widget.getConfiguration!(
new SearchParameters()
);

expect(nextConfiguration.page).toBe(0);
});
Expand All @@ -670,7 +672,9 @@ See documentation: https://www.algolia.com/doc/api-reference/widgets/infinite-hi
const makeWidget = connectInfiniteHits(renderFn);
const widget = makeWidget({});

const nextConfiguration = widget.getConfiguration!();
const nextConfiguration = widget.getConfiguration!(
new SearchParameters()
);

expect(nextConfiguration.highlightPreTag).toBe(
TAG_PLACEHOLDER.highlightPreTag
Expand All @@ -688,7 +692,9 @@ See documentation: https://www.algolia.com/doc/api-reference/widgets/infinite-hi
escapeHTML: false,
});

const nextConfiguration = widget.getConfiguration!();
const nextConfiguration = widget.getConfiguration!(
new SearchParameters()
);

expect(nextConfiguration.highlightPreTag).toBeUndefined();
expect(nextConfiguration.highlightPostTag).toBeUndefined();
Expand Down
8 changes: 4 additions & 4 deletions src/connectors/infinite-hits/connectInfiniteHits.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,19 +88,19 @@ const connectInfiniteHits: InfiniteHitsConnector = (
return {
$$type: 'ais.infiniteHits',

getConfiguration() {
getConfiguration(config) {
const parameters = {
page: 0,
};

if (!escapeHTML) {
return parameters;
return config.setQueryParameters(parameters);
}

return {
return config.setQueryParameters({
...parameters,
...TAG_PLACEHOLDER,
};
});
},

init({ instantSearchInstance, helper }) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ See documentation: https://www.algolia.com/doc/api-reference/widgets/voice-searc
const makeWidget = connectVoiceSearch(renderFn);
const widget = makeWidget({});

const nextConfiguration = widget.getConfiguration();
const nextConfiguration = widget.getConfiguration(new SearchParameters());

expect(nextConfiguration.query).toBe('');
});
Expand Down
6 changes: 2 additions & 4 deletions src/connectors/voice-search/connectVoiceSearch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,8 @@ const connectVoiceSearch: VoiceSearchConnector = (
return {
$$type: 'ais.voiceSearch',

getConfiguration() {
return {
query: '',
};
getConfiguration(config) {
return config.setQuery('');
},

init({ helper, instantSearchInstance }) {
Expand Down
25 changes: 7 additions & 18 deletions src/lib/RoutingManager.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,4 @@
import algoliasearchHelper, {
SearchParameters,
PlainSearchParameters,
} from 'algoliasearch-helper';
import { SearchParameters } from 'algoliasearch-helper';
import { isEqual } from './utils';
import {
InstantSearch,
Expand Down Expand Up @@ -137,20 +134,12 @@ class RoutingManager implements Widget {
}

public getConfiguration(
currentConfiguration: PlainSearchParameters
): PlainSearchParameters {
// We have to create a `SearchParameters` because `getAllSearchParameters`
// expects an instance of `SearchParameters` and not a plain object.
const currentSearchParameters = algoliasearchHelper.SearchParameters.make(
currentConfiguration
);

return {
...this.getAllSearchParameters({
uiState: this.currentUiState,
currentSearchParameters,
}),
};
currentConfiguration: SearchParameters
): SearchParameters {
return this.getAllSearchParameters({
uiState: this.currentUiState,
currentSearchParameters: currentConfiguration,
});
}

public init({ state }: { state: SearchParameters }): void {
Expand Down
39 changes: 30 additions & 9 deletions src/lib/utils/__tests__/enhanceConfiguration-test.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { SearchParameters } from 'algoliasearch-helper';
import enhanceConfiguration from '../enhanceConfiguration';

const createWidget = (configuration = {}) => ({
Expand All @@ -6,23 +7,23 @@ const createWidget = (configuration = {}) => ({

describe('enhanceConfiguration', () => {
it('should return the same object if widget does not provide a configuration', () => {
const configuration = { analytics: true, page: 2 };
const configuration = new SearchParameters({ analytics: true, page: 2 });
const widget = {};

const output = enhanceConfiguration(configuration, widget);
expect(output).toBe(configuration);
});

it('should return a new object if widget does provide a configuration', () => {
const configuration = { analytics: true, page: 2 };
const configuration = new SearchParameters({ analytics: true, page: 2 });
const widget = createWidget(configuration);

const output = enhanceConfiguration(configuration, widget);
expect(output).not.toBe(configuration);
});

it('should add widget configuration to an empty state', () => {
const configuration = { analytics: true, page: 2 };
const configuration = new SearchParameters({ analytics: true, page: 2 });
const widget = createWidget(configuration);

const output = enhanceConfiguration(configuration, widget);
Expand All @@ -32,7 +33,7 @@ describe('enhanceConfiguration', () => {
it('should call `getConfiguration` from widget correctly', () => {
const widget = { getConfiguration: jest.fn() };

const configuration = {};
const configuration = new SearchParameters({});

enhanceConfiguration(configuration, widget);

Expand All @@ -41,7 +42,7 @@ describe('enhanceConfiguration', () => {
});

it('should replace boolean values', () => {
const actualConfiguration = { analytics: false };
const actualConfiguration = new SearchParameters({ analytics: false });
const widget = createWidget({ analytics: true });

const output = enhanceConfiguration(actualConfiguration, widget);
Expand All @@ -50,23 +51,25 @@ describe('enhanceConfiguration', () => {

it('should union facets', () => {
{
const actualConfiguration = { facets: ['foo'] };
const actualConfiguration = new SearchParameters({ facets: ['foo'] });
const widget = createWidget({ facets: ['foo', 'bar'] });

const output = enhanceConfiguration(actualConfiguration, widget);
expect(output.facets).toEqual(['foo', 'bar']);
}

{
const actualConfiguration = { facets: ['foo'] };
const actualConfiguration = new SearchParameters({ facets: ['foo'] });
const widget = createWidget({ facets: ['bar'] });

const output = enhanceConfiguration(actualConfiguration, widget);
expect(output.facets).toEqual(['foo', 'bar']);
}

{
const actualConfiguration = { facets: ['foo', 'bar'] };
const actualConfiguration = new SearchParameters({
facets: ['foo', 'bar'],
});
const widget = createWidget({ facets: [] });

const output = enhanceConfiguration(actualConfiguration, widget);
Expand All @@ -75,7 +78,9 @@ describe('enhanceConfiguration', () => {
});

it('should replace unknown deep values', () => {
const actualConfiguration = { refinements: { lvl1: ['foo'], lvl2: false } };
const actualConfiguration = new SearchParameters({
refinements: { lvl1: ['foo'], lvl2: false },
});
const widget = createWidget({ refinements: { lvl1: ['bar'], lvl2: true } });

const output = enhanceConfiguration(actualConfiguration, widget);
Expand All @@ -85,4 +90,20 @@ describe('enhanceConfiguration', () => {
})
);
});

it('does not duplicate hierarchicalFacets (object in array)', () => {
const actualConfiguration = new SearchParameters({
hierarchicalFacets: [{ attribute: 'duplicate' }],
});
const widget = createWidget({
hierarchicalFacets: [{ attribute: 'duplicate' }],
});
const output = enhanceConfiguration(actualConfiguration, widget);

expect(output).toEqual(
expect.objectContaining({
hierarchicalFacets: [{ attribute: 'duplicate' }],
})
);
});
});
13 changes: 6 additions & 7 deletions src/lib/utils/enhanceConfiguration.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
import algoliasearchHelper, {
PlainSearchParameters,
} from 'algoliasearch-helper';
import algoliasearchHelper, { SearchParameters } from 'algoliasearch-helper';
import { Widget } from '../../types';
import mergeSearchParameters from './mergeSearchParameters';

export function enhanceConfiguration(
configuration: PlainSearchParameters,
function enhanceConfiguration(
configuration: SearchParameters,
widget: Widget
): PlainSearchParameters {
): SearchParameters {
if (!widget.getConfiguration) {
return configuration;
}
Expand All @@ -16,7 +14,8 @@ export function enhanceConfiguration(
const partialConfiguration = widget.getConfiguration(configuration);

return mergeSearchParameters(
new algoliasearchHelper.SearchParameters(configuration),
configuration,
// @TODO: remove this after IFW-874 is completed (all widgets return SP)
new algoliasearchHelper.SearchParameters(partialConfiguration)
);
}
Expand Down
10 changes: 7 additions & 3 deletions src/types/instantsearch.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
import { Client as AlgoliaSearchClient } from 'algoliasearch';
import { AlgoliaSearchHelper, SearchParameters } from 'algoliasearch-helper';
import {
AlgoliaSearchHelper,
SearchParameters,
PlainSearchParameters,
} from 'algoliasearch-helper';
import { Index } from '../widgets/index/index';
import { InsightsClient as AlgoliaInsightsClient } from './insights';
import { Widget, UiState } from './widget';
Expand Down Expand Up @@ -107,8 +111,8 @@ export type InstantSearch = {
insightsClient: AlgoliaInsightsClient | null;
templatesConfig: object;
_isSearchStalled: boolean;
_searchParameters: Partial<SearchParameters>;
_createAbsoluteURL(state: Partial<SearchParameters>): string;
_searchParameters: PlainSearchParameters;
_createAbsoluteURL(state: PlainSearchParameters): string;
scheduleSearch(): void;
scheduleRender(): void;
scheduleStalledRender(): void;
Expand Down
5 changes: 1 addition & 4 deletions src/types/widget.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import {
AlgoliaSearchHelper as Helper,
SearchParameters,
SearchResults,
PlainSearchParameters,
} from 'algoliasearch-helper';
import { InstantSearch } from './instantsearch';

Expand Down Expand Up @@ -93,9 +92,7 @@ export interface Widget {
init?(options: InitOptions): void;
render?(options: RenderOptions): void;
dispose?(options: DisposeOptions): SearchParameters | void;
getConfiguration?(
previousConfiguration?: PlainSearchParameters
): PlainSearchParameters;
getConfiguration?(previousConfiguration: SearchParameters): SearchParameters;
getWidgetState?(
uiState: UiState,
widgetStateOptions: {
Expand Down
Loading

0 comments on commit 89be615

Please sign in to comment.