Skip to content

Commit

Permalink
Add replaceQuery + pushQuery to useURLQuery hook
Browse files Browse the repository at this point in the history 
Signed-off-by: Sara Bee <855595+doeg@users.noreply.github.com>
  • Loading branch information
@doeg
doeg committed Feb 17, 2021
1 parent 8b6e690 commit 5bb242c
Show file tree
Hide file tree
Showing 6 changed files with 369 additions and 64 deletions.
4 changes: 2 additions & 2 deletions web/vtadmin/src/components/dataTable/DataTable.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,12 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import qs from 'query-string';
import * as React from 'react';
import { useLocation } from 'react-router-dom';

import { useURLPagination } from '../../hooks/useURLPagination';
import { useURLQuery } from '../../hooks/useURLQuery';
import { stringify } from '../../util/queryString';
import { PaginationNav } from './PaginationNav';

interface Props<T> {
Expand Down Expand Up @@ -48,7 +48,7 @@ export const DataTable = <T extends object>({ columns, data, pageSize = DEFAULT_

const formatPageLink = (p: number) => ({
pathname,
search: qs.stringify({ ...urlQuery, page: p === 1 ? undefined : p }),
search: stringify({ ...urlQuery.query, page: p === 1 ? undefined : p }),
});

return (
Expand Down
6 changes: 3 additions & 3 deletions web/vtadmin/src/hooks/useURLPagination.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,21 +50,21 @@ describe('useURLPagination', () => {
url: '/test?page=100&foo=bar',
opts: { totalPages: 10 },
expected: { page: 1 },
redirectParams: { pathname: '/test', search: '?foo=bar&page=1' },
redirectParams: { search: '?foo=bar&page=1' },
},
{
name: 'redirects to the first page if current page is a negative number',
url: '/test?page=-123&foo=bar',
opts: { totalPages: 10 },
expected: { page: 1 },
redirectParams: { pathname: '/test', search: '?foo=bar&page=1' },
redirectParams: { search: '?foo=bar&page=1' },
},
{
name: 'redirects to the first page if current page is not a number',
url: '/test?page=abc&foo=bar',
opts: { totalPages: 10 },
expected: { page: 1 },
redirectParams: { pathname: '/test', search: '?foo=bar&page=1' },
redirectParams: { search: '?foo=bar&page=1' },
},
{
name: 'does not redirect if totalPages is 0',
Expand Down
18 changes: 8 additions & 10 deletions web/vtadmin/src/hooks/useURLPagination.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
*/
import { useEffect } from 'react';

import qs from 'query-string';
import { useHistory, useLocation } from 'react-router-dom';
import { useURLQuery } from './useURLQuery';

Expand All @@ -39,25 +38,24 @@ const FIRST_PAGE = 1;
export const useURLPagination = ({ totalPages }: PaginationOpts): PaginationParams => {
const history = useHistory();
const location = useLocation();
const query = useURLQuery();
const { query, replaceQuery } = useURLQuery();

// A slight nuance here -- if `page` is not in the URL at all, then we can assume
// it's the first page. This makes for slightly nicer URLs for the first/default page:
// "/foo" instead of "/foo?page=1". No redirect required.
//
// However, if the value in the URL *is* defined but is negative, non-numeric,
// too big, or otherwise Weird, then we *do* want to redirect to the first page.
const page = !('page' in query) || query.page === null ? FIRST_PAGE : query.page;

useEffect(() => {
const isPageTooBig = totalPages > 0 && page > totalPages;
const isPageTooSmall = page < FIRST_PAGE;
// If the value in the URL *is* defined but is negative, non-numeric,
// too big, or otherwise Weird, then we *do* want to redirect to the first page.
const isPageTooBig = typeof page === 'number' && totalPages > 0 && page > totalPages;
const isPageTooSmall = typeof page === 'number' && page < FIRST_PAGE;

if (isPageTooBig || isPageTooSmall || typeof page !== 'number') {
const nextQuery = qs.stringify({ ...query, page: FIRST_PAGE });
history.replace({ pathname: location.pathname, search: `?${nextQuery}` });
// Replace history so the invalid value is not persisted in browser history
replaceQuery({ page: FIRST_PAGE });
}
}, [page, totalPages, history, location.pathname, query]);
}, [page, totalPages, history, location.pathname, query, replaceQuery]);

return {
page,
Expand Down
237 changes: 206 additions & 31 deletions web/vtadmin/src/hooks/useURLQuery.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,51 +13,226 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import { act } from '@testing-library/react';
import { renderHook } from '@testing-library/react-hooks';
import { createMemoryHistory } from 'history';
import { Router } from 'react-router-dom';
import { QueryParams } from '../util/queryString';

import { useURLQuery } from './useURLQuery';

describe('useURLQuery', () => {
const tests: {
name: string;
url: string;
expected: ReturnType<typeof useURLQuery>;
}[] = [
{
name: 'parses numbers',
url: '/test?page=1',
expected: { page: 1 },
},
{
name: 'parses booleans',
url: '/test?foo=true&bar=false',
expected: { foo: true, bar: false },
},
{
name: 'parses arrays by duplicate keys',
url: '/test?list=1&list=2&list=3',
expected: { list: [1, 2, 3] },
},
{
name: 'parses complex URLs',
url: '/test?page=1&isTrue=true&isFalse=false&list=one&list=two&list=three&foo=bar',
expected: { page: 1, isTrue: true, isFalse: false, list: ['one', 'two', 'three'], foo: 'bar' },
},
];

test.each(tests.map(Object.values))('%s', (name: string, url: string, expected: ReturnType<typeof useURLQuery>) => {
const history = createMemoryHistory({
initialEntries: [url],
describe('parsing', () => {
const tests: {
name: string;
url: string;
expected: ReturnType<typeof useURLQuery>['query'];
}[] = [
{
name: 'parses numbers',
url: '/test?page=1',
expected: { page: 1 },
},
{
name: 'parses booleans',
url: '/test?foo=true&bar=false',
expected: { foo: true, bar: false },
},
{
name: 'parses arrays by duplicate keys',
url: '/test?list=1&list=2&list=3',
expected: { list: [1, 2, 3] },
},
{
name: 'parses complex URLs',
url: '/test?page=1&isTrue=true&isFalse=false&list=one&list=two&list=three&foo=bar',
expected: { page: 1, isTrue: true, isFalse: false, list: ['one', 'two', 'three'], foo: 'bar' },
},
];

test.each(tests.map(Object.values))(
'%s',
(name: string, url: string, expected: ReturnType<typeof useURLQuery>) => {
const history = createMemoryHistory({
initialEntries: [url],
});

const { result } = renderHook(() => useURLQuery(), {
wrapper: ({ children }) => {
return <Router history={history}>{children}</Router>;
},
});

expect(result.current.query).toEqual(expected);
}
);
});

describe('pushQuery', () => {
const tests: {
name: string;
initialEntries: string[];
nextQuery: QueryParams;
expected: QueryParams;
}[] = [
{
name: 'stringifies and pushes query parameters onto history',
initialEntries: ['/test'],
nextQuery: { goodnight: 'moon' },
expected: { goodnight: 'moon' },
},
{
name: 'merges the next query with the current query',
initialEntries: ['/test?hello=world'],
nextQuery: { goodnight: 'moon' },
expected: { hello: 'world', goodnight: 'moon' },
},
{
name: 'it does not merge array-like queries',
initialEntries: ['/test?arr=one&arr=two'],
nextQuery: { arr: [3, 4, 5] },
expected: { arr: [3, 4, 5] },
},
];

test.concurrent.each(tests.map(Object.values))(
'%s',
(name: string, initialEntries: string[], nextQuery: QueryParams, expected: QueryParams) => {
const history = createMemoryHistory({ initialEntries });
const initialPathname = history.location.pathname;

jest.spyOn(history, 'push');

const { result } = renderHook(() => useURLQuery(), {
wrapper: ({ children }) => {
return <Router history={history}>{children}</Router>;
},
});

act(() => {
result.current.pushQuery(nextQuery);
});

expect(history.push).toHaveBeenCalledTimes(1);
expect(result.current.query).toEqual(expected);
expect(history.location.pathname).toEqual(initialPathname);
}
);
});

describe('replaceQuery', () => {
const tests: {
name: string;
initialEntries: string[];
nextQuery: QueryParams;
expected: QueryParams;
}[] = [
{
name: 'stringifies and replaces query parameters onto history',
initialEntries: ['/test'],
nextQuery: { goodnight: 'moon' },
expected: { goodnight: 'moon' },
},
{
name: 'merges the next query with the current query',
initialEntries: ['/test?hello=world'],
nextQuery: { goodnight: 'moon' },
expected: { hello: 'world', goodnight: 'moon' },
},
{
name: 'it does not merge array-like queries',
initialEntries: ['/test?arr=one&arr=two'],
nextQuery: { arr: [3, 4, 5] },
expected: { arr: [3, 4, 5] },
},
];

test.concurrent.each(tests.map(Object.values))(
'%s',
(name: string, initialEntries: string[], nextQuery: QueryParams, expected: QueryParams) => {
const history = createMemoryHistory({ initialEntries });
const initialPathname = history.location.pathname;

jest.spyOn(history, 'replace');

const { result } = renderHook(() => useURLQuery(), {
wrapper: ({ children }) => {
return <Router history={history}>{children}</Router>;
},
});

act(() => {
result.current.replaceQuery(nextQuery);
});

expect(history.replace).toHaveBeenCalledTimes(1);
expect(result.current.query).toEqual(expected);
expect(history.location.pathname).toEqual(initialPathname);
}
);
});

it('uses parsing/formatting options when specified', () => {
const history = createMemoryHistory({ initialEntries: ['/test?foo=true&count=123'] });
const { result } = renderHook(
() =>
useURLQuery({
parseBooleans: false,
parseNumbers: false,
}),
{
wrapper: ({ children }) => {
return <Router history={history}>{children}</Router>;
},
}
);

expect(result.current.query).toEqual({ foo: 'true', count: '123' });

act(() => {
result.current.pushQuery({ foo: false, count: 456 });
});

expect(result.current.query).toEqual({ foo: 'false', count: '456' });

act(() => {
result.current.pushQuery({ foo: true, count: 789 });
});

expect(result.current.query).toEqual({ foo: 'true', count: '789' });
});

it('memoizes the query object by search string', () => {
const history = createMemoryHistory({ initialEntries: ['/test?hello=world'] });

jest.spyOn(history, 'push');

const { result } = renderHook(() => useURLQuery(), {
wrapper: ({ children }) => {
return <Router history={history}>{children}</Router>;
},
});

expect(result.current).toEqual(expected);
const firstResult = result.current.query;
expect(firstResult).toEqual({ hello: 'world' });

act(() => {
result.current.pushQuery({ hello: 'world' });
});

// Make sure the returned object is memoized when the search string
// is updated but the value doesn't change.
expect(history.push).toHaveBeenCalledTimes(1);
expect(result.current.query).toEqual({ hello: 'world' });
expect(result.current.query).toBe(firstResult);

// Make sure the returned query actually changes when the search string changes,
// and that we're not memoizing too aggressively.
act(() => {
result.current.pushQuery({ hello: 'moon' });
});

expect(history.push).toHaveBeenCalledTimes(2);
expect(result.current.query).toEqual({ hello: 'moon' });
});
});

0 comments on commit 5bb242c

Please sign in to comment.