Skip to content
This repository has been archived by the owner on Dec 30, 2022. It is now read-only.

Commit

Permalink
fix(ssr): avoid duplicate serializing (#2726)
Browse files Browse the repository at this point in the history
* fix(ssr): avoid duplicate serializing

Response of `findResultsState` (and `resultsState` prop) before:

```js
const resultsState = {
  content: SearchResults(state: searchParameters, results: rawResults),
  state: searchParameters,
  _originalResponse: {
    results: rawResults
  },
};
```

Note that `SearchResults` has the properties `_rawResults` & `_state`. This means that when we serialize, we are retrieving the original response and state twice.

Then on further searching, I also find that we don't need to serialize the `SearchResults` object at all, since only the raw response and state is used.

I modified this to be more consistent and no longer have duplication:

```js
const resultsState = {
  state: searchParameters,
  rawResults,
};
```

The `SearchParameters` object gets serialized without issue, as before.

This change in shape has of course also an impact on the shape of `resultsState`, the prop, but that is fine, since it was previously typed as `any`.

To be able to type it correctly, I had to add `@types/algoliasearch` & update `algoliasearch-helper` for typescript though.

This was initially noticed by @cloakedninjas, thanks!

* empty commit for CI
  • Loading branch information
Haroenv committed Aug 13, 2019
1 parent dea493c commit c768b1a
Show file tree
Hide file tree
Showing 8 changed files with 219 additions and 167 deletions.
3 changes: 2 additions & 1 deletion packages/react-instantsearch-core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@
},
"dependencies": {
"@babel/runtime": "^7.1.2",
"algoliasearch-helper": "0.0.0-27095c0",
"@types/algoliasearch": "^3.30.16",
"algoliasearch-helper": "0.0.0-6ac260d",
"fast-deep-equal": "^2.0.1",
"prop-types": "^15.5.10"
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,15 @@ const runAllMicroTasks = () => new Promise(setImmediate);
const createSearchClient = () => ({
search: jest.fn(requests =>
Promise.resolve({
results: requests.map(request => ({
index: request.indexName,
query: request.params.query,
page: request.params.page,
hitsPerPage: request.params.hitsPerPage,
hits: [],
})),
results: requests.map(
({ indexName, params: { page, query, hitsPerPage } }) => ({
index: indexName,
query,
page,
hitsPerPage,
hits: [],
})
),
})
),
searchForFacetValues() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,14 +69,12 @@ describe('createInstantSearchManager', () => {
});

const resultsState = {
_originalResponse: {
results: [
{
index: 'index',
query: 'query',
},
],
},
rawResults: [
{
index: 'index',
query: 'query',
},
],
state: {
index: 'index',
query: 'query',
Expand Down Expand Up @@ -112,29 +110,25 @@ describe('createInstantSearchManager', () => {
const resultsState = [
{
_internalIndexId: 'index1',
_originalResponse: {
results: [
{
index: 'index1',
query: 'query1',
},
],
},
rawResults: [
{
index: 'index1',
query: 'query1',
},
],
state: {
index: 'index1',
query: 'query1',
},
},
{
_internalIndexId: 'index2',
_originalResponse: {
results: [
{
index: 'index2',
query: 'query2',
},
],
},
rawResults: [
{
index: 'index2',
query: 'query2',
},
],
state: {
index: 'index2',
query: 'query2',
Expand Down Expand Up @@ -187,14 +181,12 @@ describe('createInstantSearchManager', () => {
};

const resultsState = {
_originalResponse: {
results: [
{
index: 'indexName',
query: 'query',
},
],
},
rawResults: [
{
index: 'indexName',
query: 'query',
},
],
state: {
index: 'indexName',
query: 'query',
Expand All @@ -218,14 +210,12 @@ describe('createInstantSearchManager', () => {
});

const resultsState = {
_originalResponse: {
results: [
{
index: 'indexName',
query: 'query',
},
],
},
rawResults: [
{
index: 'indexName',
query: 'query',
},
],
state: {
index: 'indexName',
query: 'query',
Expand All @@ -250,14 +240,12 @@ describe('createInstantSearchManager', () => {
indexName: 'index',
searchClient: createSearchClient(),
resultsState: {
_originalResponse: {
results: [
{
index: 'indexName',
query: 'query',
},
],
},
rawResults: [
{
index: 'indexName',
query: 'query',
},
],
state: {
index: 'indexName',
query: 'query',
Expand All @@ -276,29 +264,25 @@ describe('createInstantSearchManager', () => {
resultsState: [
{
_internalIndexId: 'index1',
_originalResponse: {
results: [
{
index: 'index1',
query: 'query1',
},
],
},
rawResults: [
{
index: 'index1',
query: 'query1',
},
],
state: {
index: 'index1',
query: 'query1',
},
},
{
_internalIndexId: 'index2',
_originalResponse: {
results: [
{
index: 'index2',
query: 'query2',
},
],
},
rawResults: [
{
index: 'index2',
query: 'query2',
},
],
state: {
index: 'index2',
query: 'query2',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@ export default function createInstantSearchManager({
requests: results.reduce(
(acc, result) =>
acc.concat(
result._originalResponse.results.map(request => ({
result.rawResults.map(request => ({
indexName: request.index,
params: request.params,
}))
Expand All @@ -335,7 +335,7 @@ export default function createInstantSearchManager({
...client.cache,
[key]: {
results: results.reduce(
(acc, result) => acc.concat(result._originalResponse.results),
(acc, result) => acc.concat(result.rawResults),
[]
),
},
Expand All @@ -349,15 +349,17 @@ export default function createInstantSearchManager({
// computation of the key inside the client (see link below).
// https://github.com/algolia/algoliasearch-client-javascript/blob/c27e89ff92b2a854ae6f40dc524bffe0f0cbc169/src/AlgoliaSearchCore.js#L232-L240
const key = `/1/indexes/*/queries_body_${JSON.stringify({
requests: results._originalResponse.results.map(request => ({
requests: results.rawResults.map(request => ({
indexName: request.index,
params: request.params,
})),
})}`;

client.cache = {
...client.cache,
[key]: results._originalResponse,
[key]: {
results: results.rawResults,
},
};
}

Expand All @@ -372,7 +374,7 @@ export default function createInstantSearchManager({
...acc,
[result._internalIndexId]: new algoliasearchHelper.SearchResults(
new algoliasearchHelper.SearchParameters(result.state),
result._originalResponse.results
result.rawResults
),
}),
{}
Expand All @@ -381,7 +383,7 @@ export default function createInstantSearchManager({

return new algoliasearchHelper.SearchResults(
new algoliasearchHelper.SearchParameters(results.state),
results._originalResponse.results
results.rawResults
);
}

Expand Down
19 changes: 11 additions & 8 deletions packages/react-instantsearch-core/src/widgets/InstantSearch.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,13 @@ import PropTypes from 'prop-types';
import createInstantSearchManager from '../core/createInstantSearchManager';
import { InstantSearchProvider, InstantSearchContext } from '../core/context';
import { Store } from '../core/createStore';
import { PlainSearchParameters, SearchParameters } from 'algoliasearch-helper';
import { MultiResponse } from 'algoliasearch';

function isControlled(props: Props) {
return Boolean(props.searchState);
}

// @TODO: move this to the helper?
type SearchParameters = any; // algoliaHelper.SearchParameters
type SearchResults = any; // algoliaHelper.SearchResults
type ResultsState = {
state: PlainSearchParameters;
rawResults: MultiResponse;
};

// @TODO: move to createInstantSearchManager when it's TS
type InstantSearchManager = {
Expand Down Expand Up @@ -53,13 +52,17 @@ type Props = {
searchState: SearchState
) => void;
stalledSearchDelay?: number;
resultsState: SearchResults | { [indexId: string]: SearchResults };
resultsState: ResultsState | { [indexId: string]: ResultsState };
};

type State = {
contextValue: InstantSearchContext;
};

function isControlled(props: Props) {
return Boolean(props.searchState);
}

/**
* @description
* `<InstantSearch>` is the root component of all React InstantSearch implementations.
Expand Down
Loading

0 comments on commit c768b1a

Please sign in to comment.