Skip to content

Commit fa0b913

Browse files
author
React-Admin CI
committed
Apply review comments
1 parent f545c8d commit fa0b913

2 files changed

Lines changed: 118 additions & 87 deletions

File tree

‎packages/ra-core/src/controller/list/useListParams.spec.tsx‎

Lines changed: 61 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,12 @@ import { CoreAdminContext } from '../../core';
66

77
import { testDataProvider } from '../../dataProvider';
88
import { useStore } from '../../store/useStore';
9-
import { useListParams, getQuery, getNumberOrDefault } from './useListParams';
9+
import {
10+
useListParams,
11+
getQuery,
12+
getNumberOrDefault,
13+
ListParamsOptions,
14+
} from './useListParams';
1015
import { SORT_DESC, SORT_ASC } from './queryReducer';
1116
import { TestMemoryRouter } from '../../routing';
1217
import { memoryStore } from '../../store';
@@ -361,11 +366,16 @@ describe('useListParams', () => {
361366
});
362367
});
363368
describe('useListParams', () => {
364-
const Component = ({ disableSyncWithLocation = false }) => {
365-
const [{ page }, { setPage }] = useListParams({
366-
resource: 'posts',
367-
disableSyncWithLocation,
368-
});
369+
const Component = ({
370+
disableSyncWithLocation = false,
371+
...options
372+
}: Partial<ListParamsOptions>) => {
373+
const [{ page, perPage, sort, order, filter }, { setPage }] =
374+
useListParams({
375+
resource: 'posts',
376+
disableSyncWithLocation,
377+
...options,
378+
});
369379

370380
const handleClick = () => {
371381
setPage(10);
@@ -374,6 +384,10 @@ describe('useListParams', () => {
374384
return (
375385
<>
376386
<p>page: {page}</p>
387+
<p>perPage: {perPage}</p>
388+
<p>sort: {sort}</p>
389+
<p>order: {order}</p>
390+
<p>filter: {JSON.stringify(filter)}</p>
377391
<button onClick={handleClick}>update</button>
378392
</>
379393
);
@@ -608,14 +622,6 @@ describe('useListParams', () => {
608622

609623
it('should not synchronize location with store if the location already contains parameters', async () => {
610624
let location;
611-
let storeValue;
612-
const StoreReader = () => {
613-
const [value] = useStore('posts.listParams');
614-
React.useEffect(() => {
615-
storeValue = value;
616-
}, [value]);
617-
return null;
618-
};
619625
render(
620626
<TestMemoryRouter
621627
initialEntries={[
@@ -647,22 +653,11 @@ describe('useListParams', () => {
647653
},
648654
})}
649655
>
650-
<Component disableSyncWithLocation />
651-
<StoreReader />
656+
<Component />
652657
</CoreAdminContext>
653658
</TestMemoryRouter>
654659
);
655660

656-
await waitFor(() => {
657-
expect(storeValue).toEqual({
658-
sort: 'id',
659-
order: 'ASC',
660-
page: 10,
661-
perPage: 10,
662-
filter: {},
663-
});
664-
});
665-
666661
await waitFor(() => {
667662
expect(location).toEqual(
668663
expect.objectContaining({
@@ -693,14 +688,53 @@ describe('useListParams', () => {
693688
}}
694689
>
695690
<CoreAdminContext dataProvider={testDataProvider()}>
696-
<Component disableSyncWithLocation />
691+
<Component />
692+
</CoreAdminContext>
693+
</TestMemoryRouter>
694+
);
695+
696+
// Let React do its thing
697+
await new Promise(resolve => setTimeout(resolve, 0));
698+
699+
await waitFor(() => {
700+
expect(location).toEqual(
701+
expect.objectContaining({
702+
hash: '',
703+
key: expect.any(String),
704+
state: null,
705+
pathname: '/',
706+
search: '',
707+
})
708+
);
709+
});
710+
});
711+
712+
it('should not synchronize location with store if the store parameters are the custom defaults provided to the hook', async () => {
713+
let location;
714+
render(
715+
<TestMemoryRouter
716+
locationCallback={l => {
717+
location = l;
718+
}}
719+
>
720+
<CoreAdminContext dataProvider={testDataProvider()}>
721+
<Component
722+
perPage={5}
723+
sort={{ field: 'title', order: 'DESC' }}
724+
/>
697725
</CoreAdminContext>
698726
</TestMemoryRouter>
699727
);
700728

701729
// Let React do its thing
702730
await new Promise(resolve => setTimeout(resolve, 0));
703731

732+
// The list is using the default set on the component
733+
await screen.findByText('perPage: 5');
734+
await screen.findByText('sort: title');
735+
await screen.findByText('order: DESC');
736+
737+
// The location is the default for the list (no query parameters)
704738
await waitFor(() => {
705739
expect(location).toEqual(
706740
expect.objectContaining({

‎packages/ra-core/src/controller/list/useListParams.ts‎

Lines changed: 57 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -107,9 +107,10 @@ export const useListParams = ({
107107
disableSyncWithLocation,
108108
];
109109

110-
const queryFromLocation = disableSyncWithLocation
111-
? {}
112-
: parseQueryFromLocation(location);
110+
const queryFromLocation = useMemo(
111+
() => (disableSyncWithLocation ? {} : parseQueryFromLocation(location)),
112+
[location, disableSyncWithLocation]
113+
);
113114

114115
const query = useMemo(
115116
() =>
@@ -138,63 +139,59 @@ export const useListParams = ({
138139
// the categories products on the demo), we need to persist them in the
139140
// store as well so that we don't lose them after a redirection back
140141
// to the list
141-
useEffect(
142-
() => {
143-
// If the storeKey has changed, ignore the first effect call. This avoids conflicts between lists sharing
144-
// the same resource but different storeKeys.
145-
if (currentStoreKey.current !== storeKey) {
146-
// storeKey has changed
147-
currentStoreKey.current = storeKey;
148-
return;
149-
}
150-
if (disableSyncWithLocation) {
151-
return;
152-
}
153-
const defaultParams = {
154-
filter: filterDefaultValues || {},
155-
page: 1,
156-
perPage,
157-
sort: sort.field,
158-
order: sort.order,
159-
};
160-
161-
if (
162-
// The location params are not empty (we don't want to override them if provided)
163-
Object.keys(queryFromLocation).length > 0 ||
164-
// or the stored params are different from the location params
165-
isEqual(query, queryFromLocation) ||
166-
// or the stored params are not different from the default params (to keep the URL simple when possible)
167-
isEqual(query, defaultParams)
168-
) {
169-
return;
170-
}
171-
navigate(
172-
{
173-
search: `?${stringify({
174-
...query,
175-
filter: JSON.stringify(query.filter),
176-
displayedFilters: JSON.stringify(
177-
query.displayedFilters
178-
),
179-
})}`,
180-
},
181-
{
182-
replace: true,
183-
}
184-
);
185-
},
186-
// eslint-disable-next-line react-hooks/exhaustive-deps
187-
[
188-
navigate,
189-
disableSyncWithLocation,
190-
filterDefaultValues,
142+
useEffect(() => {
143+
// If the storeKey has changed, ignore the first effect call. This avoids conflicts between lists sharing
144+
// the same resource but different storeKeys.
145+
if (currentStoreKey.current !== storeKey) {
146+
// storeKey has changed
147+
currentStoreKey.current = storeKey;
148+
return;
149+
}
150+
if (disableSyncWithLocation) {
151+
return;
152+
}
153+
const defaultParams = {
154+
filter: filterDefaultValues || {},
155+
page: 1,
191156
perPage,
192-
sort,
193-
query,
194-
location.search,
195-
params,
196-
]
197-
);
157+
sort: sort.field,
158+
order: sort.order,
159+
};
160+
161+
if (
162+
// The location params are not empty (we don't want to override them if provided)
163+
Object.keys(queryFromLocation).length > 0 ||
164+
// or the stored params are the same as the location params
165+
isEqual(query, queryFromLocation) ||
166+
// or the stored params are the same as the default params (to keep the URL simple when possible)
167+
isEqual(query, defaultParams)
168+
) {
169+
return;
170+
}
171+
navigate(
172+
{
173+
search: `?${stringify({
174+
...query,
175+
filter: JSON.stringify(query.filter),
176+
displayedFilters: JSON.stringify(query.displayedFilters),
177+
})}`,
178+
},
179+
{
180+
replace: true,
181+
}
182+
);
183+
}, [
184+
navigate,
185+
disableSyncWithLocation,
186+
filterDefaultValues,
187+
perPage,
188+
sort,
189+
query,
190+
queryFromLocation,
191+
location.search,
192+
params,
193+
storeKey,
194+
]);
198195

199196
const changeParams = useCallback(
200197
action => {
@@ -340,7 +337,7 @@ const parseObject = (query, field) => {
340337
if (query[field] && typeof query[field] === 'string') {
341338
try {
342339
query[field] = JSON.parse(query[field]);
343-
} catch (err) {
340+
} catch {
344341
delete query[field];
345342
}
346343
}

0 commit comments

Comments
 (0)