Skip to content

Commit

Permalink
fix(routing): fix history router based on history length (#5004)
Browse files Browse the repository at this point in the history
* fix(routing): fix history router based on history length

* chore(tests): add routing test

* chore(tests): split tests to avoid flakiness

* fix(routing): stay responsive on external updates (before dispose)

* chore(tests): assert replaceState behavior

* chore: run codesandbox

* refactor(routing): update dispose flag logic

* refactor(routing): remove unused line

* refactor(routing): apply code review suggestions

* chore(tests): apply feedback from code review

* chore(tests): apply feedback
  • Loading branch information
FabienMotte committed Feb 7, 2022
1 parent 59fbe4d commit 40541af
Show file tree
Hide file tree
Showing 7 changed files with 519 additions and 2 deletions.
98 changes: 98 additions & 0 deletions src/lib/__tests__/routing/dispose-start-test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
import { createSearchClient } from '../../../../test/mock/createSearchClient';
import { wait } from '../../../../test/utils/wait';
import historyRouter from '../../routers/history';
import instantsearch from '../../..';
import { connectSearchBox } from '../../../connectors';
import type InstantSearch from '../../InstantSearch';

/* eslint no-lone-blocks: "off" */

const writeDelay = 10;
const writeWait = 1.5 * writeDelay;

const addWidgetsAndStart = (search: InstantSearch) => {
search.addWidgets([connectSearchBox(() => {})({})]);
search.start();
};

describe('routing back and forth to an InstantSearch instance', () => {
test('updates the URL after the instance is disposed then restarted', async () => {
// -- Flow
// 1. Initial: '/'
// 2. Refine: '/?indexName[query]=Apple'
// 3. Dispose: '/'
// 4. Refine: '/'
// 5. Start: '/'
// 6. Refine: '/?indexName[query]=Apple'

const pushState = jest.spyOn(window.history, 'pushState');

const search = instantsearch({
indexName: 'indexName',
searchClient: createSearchClient(),
routing: {
router: historyRouter({
writeDelay,
}),
},
});

// 1. Initial: '/'
{
addWidgetsAndStart(search);

await wait(writeWait);
expect(window.location.search).toEqual('');
expect(pushState).toHaveBeenCalledTimes(0);
}

// 2. Refine: '/?indexName[query]=Apple'
{
search.renderState.indexName!.searchBox!.refine('Apple');

await wait(writeWait);
expect(window.location.search).toEqual(
`?${encodeURI('indexName[query]=Apple')}`
);
expect(pushState).toHaveBeenCalledTimes(1);
}

// 3. Dispose: '/'
{
search.dispose();

await wait(writeWait);
expect(window.location.search).toEqual('');
expect(pushState).toHaveBeenCalledTimes(2);
}

// 4. Refine: '/'
{
search.renderState.indexName!.searchBox!.refine('Apple');

await wait(writeWait);
expect(window.location.search).toEqual('');
expect(pushState).toHaveBeenCalledTimes(2);
}

// 5. Start: '/'
{
addWidgetsAndStart(search);

await wait(writeWait);
expect(window.location.search).toEqual('');
expect(pushState).toHaveBeenCalledTimes(2);
}

// 6. Refine: '/?indexName[query]=Apple'
{
search.renderState.indexName!.searchBox!.refine('Samsung');

await wait(writeWait);
expect(window.location.search).toEqual(
`?${encodeURI('indexName[query]=Samsung')}`
);
expect(pushState).toHaveBeenCalledTimes(3);
}
});
});
74 changes: 74 additions & 0 deletions src/lib/__tests__/routing/external-influence-test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
import { createSearchClient } from '../../../../test/mock/createSearchClient';
import { wait } from '../../../../test/utils/wait';
import historyRouter from '../../routers/history';
import instantsearch from '../../..';
import { connectSearchBox } from '../../../connectors';

/* eslint no-lone-blocks: "off" */

const writeDelay = 10;
const writeWait = 1.5 * writeDelay;

describe('routing with external influence', () => {
test('keeps on working when the URL is updated by another program', async () => {
// -- Flow
// 1. Initial: '/'
// 2. Refine: '/?indexName[query]=Apple'
// 3. External influence: '/about'
// 4. Refine: '/about?indexName[query]=Samsung'

const pushState = jest.spyOn(window.history, 'pushState');

const search = instantsearch({
indexName: 'indexName',
searchClient: createSearchClient(),
routing: {
router: historyRouter({
writeDelay,
}),
},
});

// 1. Initial: '/'
{
search.addWidgets([connectSearchBox(() => {})({})]);
search.start();

await wait(writeWait);
expect(window.location.search).toEqual('');
expect(pushState).toHaveBeenCalledTimes(0);
}

// 2. Refine: '/?indexName[query]=Apple'
{
search.renderState.indexName!.searchBox!.refine('Apple');

await wait(writeWait);
expect(window.location.search).toEqual(
`?${encodeURI('indexName[query]=Apple')}`
);
expect(pushState).toHaveBeenCalledTimes(1);
}

// 3. External influence: '/about'
{
window.history.pushState({}, '', '/about');

await wait(writeWait);
expect(window.location.pathname).toEqual('/about');
expect(window.location.search).toEqual('');
expect(pushState).toHaveBeenCalledTimes(2);
}

// 4. Refine: '/about?indexName[query]=Samsung'
{
search.renderState.indexName!.searchBox!.refine('Samsung');

await wait(writeWait);
expect(window.location.pathname).toEqual('/about');
expect(window.location.search).toEqual(
`?${encodeURI('indexName[query]=Samsung')}`
);
}
});
});
63 changes: 63 additions & 0 deletions src/lib/__tests__/routing/modal-test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
import { createSearchClient } from '../../../../test/mock/createSearchClient';
import { wait } from '../../../../test/utils/wait';
import historyRouter from '../../routers/history';
import instantsearch from '../../..';
import { connectSearchBox } from '../../../connectors';

/* eslint no-lone-blocks: "off" */

const writeDelay = 10;
const writeWait = 1.5 * writeDelay;

describe('routing with no navigation', () => {
test('cleans the URL when InstantSearch is disposed within the same page', async () => {
// -- Flow
// 1. Initial: '/'
// 2. Refine: '/?indexName[query]=Apple'
// 3. Dispose: '/'

const pushState = jest.spyOn(window.history, 'pushState');

const search = instantsearch({
indexName: 'indexName',
searchClient: createSearchClient(),
routing: {
router: historyRouter({
writeDelay,
}),
},
});

// 1. Initial: '/'
{
search.addWidgets([connectSearchBox(() => {})({})]);
search.start();

await wait(writeWait);
expect(window.location.search).toEqual('');
expect(pushState).toHaveBeenCalledTimes(0);
}

// 2. Refine: '/?indexName[query]=Apple'
{
search.renderState.indexName!.searchBox!.refine('Apple');

await wait(writeWait);
expect(window.location.pathname).toEqual('/');
expect(window.location.search).toEqual(
`?${encodeURI('indexName[query]=Apple')}`
);
expect(pushState).toHaveBeenCalledTimes(1);
}

// 3. Dispose: '/'
{
search.dispose();

await wait(writeWait);
expect(window.location.pathname).toEqual('/');
expect(window.location.search).toEqual('');
expect(pushState).toHaveBeenCalledTimes(2);
}
});
});
96 changes: 96 additions & 0 deletions src/lib/__tests__/routing/spa-debounced-test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
import { createSearchClient } from '../../../../test/mock/createSearchClient';
import { wait } from '../../../../test/utils/wait';
import historyRouter from '../../routers/history';
import instantsearch from '../../..';
import { connectSearchBox } from '../../../connectors';

/* eslint no-lone-blocks: "off" */

const writeDelay = 10;
const writeWait = 1.5 * writeDelay;

describe('routing with debounced third-party client-side router', () => {
test('does not clean the URL after navigating', async () => {
// -- Flow
// 1. Initial: '/'
// 2. Refine: '/?indexName[query]=Apple'
// 3. Dispose: '/'
// 4. Route change: '/about'
// 5. Back: '/'
// 6. Back: '/?indexName[query]=Apple'

const pushState = jest.spyOn(window.history, 'pushState');

const search = instantsearch({
indexName: 'indexName',
searchClient: createSearchClient(),
routing: {
router: historyRouter({
writeDelay,
}),
},
});

// 1. Initial: '/'
{
search.addWidgets([connectSearchBox(() => {})({})]);
search.start();

await wait(writeWait);
expect(window.location.search).toEqual('');
expect(pushState).toHaveBeenCalledTimes(0);
}

// 2. Refine: '/?indexName[query]=Apple'
{
search.renderState.indexName!.searchBox!.refine('Apple');

await wait(writeWait);
expect(window.location.search).toEqual(
`?${encodeURI('indexName[query]=Apple')}`
);
expect(pushState).toHaveBeenCalledTimes(1);
}

// 3. Dispose: '/'
{
search.dispose();

await wait(writeWait);
expect(window.location.pathname).toEqual('/');
expect(window.location.search).toEqual('');
expect(pushState).toHaveBeenCalledTimes(2);
}

// 4. Route change: '/about'
{
window.history.pushState({}, '', '/about');

await wait(writeWait);
expect(window.location.pathname).toEqual('/about');
expect(window.location.search).toEqual('');
expect(pushState).toHaveBeenCalledTimes(3);
}

// 5. Back: '/'
{
window.history.back();

await wait(writeWait);
expect(window.location.pathname).toEqual('/');
expect(window.location.search).toEqual('');
}

// 6. Back: '/?indexName[query]=Apple'
{
window.history.back();

await wait(writeWait);
expect(window.location.pathname).toEqual('/');
expect(window.location.search).toEqual(
`?${encodeURI('indexName[query]=Apple')}`
);
expect(pushState).toHaveBeenCalledTimes(3);
}
});
});
Loading

0 comments on commit 40541af

Please sign in to comment.