Calling setSort and setFilters after one another filckers and doesn't setSort

[estephan]

What you were expecting:

I've built a button and added it as part of the actions props on a List component. This button is intended to clear the filters applied to the List as well as return the sort to its default value. The button calls the setSort and setFilters functions that are passed in as props to actions. I expected the filters to be cleared and the sort to return to its default value.

What happened instead:

Instead, what I'm seeing is that the filters are cleared (as expected) however the sort "flicks" to the default sort options and then flicks back to the previously selected sort.
If I call setSort alone or setFilters alone, they behave as expected and it seems that this issue only occurs when both are called after one another.

Steps to reproduce:

1. Go to the Posts list in the following sandbox.
2. Click on the "Title" column to sort posts by title.
3. In the search box, type "Mai" to add a filter.
4. Then click the "Clear sort and filters" button in the top right.
5. You'll see the sort goes back to "Published At" as expected but then flicks back to "Title".

https://codesandbox.io/s/elastic-raman-kvypc

Environment

• React-admin version: 3.1.0
• Last version that did not exhibit the issue (if applicable): 2.x
• React version: 16.12.0
• Browser: Chrome (v. 78.0.3904.108)

[fzaninotto]

You call setSort() with no argument, it's not something expected by react-admin (it should not be accepted at all, in fact). Pass the default sort you want as parameter, or else react uses the last selected sort (we persist user choices in the Redux store).

The flickr comes from the fact that we use the URL to store the sort and filters. setSort and setFilter each change the URL once, so that triggers two fetches of the dataProvider. Nothing anormal.

In your case, your button should set the query parameter directly to do all in once. That would avoid the flickr.

Anyway, this is not a bug IMO, so I'm closing the issue.

[estephan]

No arguments passed to setSort was just an example - if you try it with setSort("published_at") the same flicker result happens.

Try with https://codesandbox.io/s/kind-wave-qjxb9

[fzaninotto]

You're right - I believe that's an issue with a bad dependency in a useEffect. Repoening and classifying as a bug. Thanks for the CodeSandbox links.

[ZachSelindh]

Also having this issue in one of my projects; would love to know if anyone knows of a workaround or if a fix has been implemented. I'm on version 3.11.3.

[fzaninotto]

This was fixed by #6226, released in 3.15.

[ZachSelindh]

@fzaninotto Thanks for your attention. Unfortunately, after migrating to 3.17.0, I'm still seeing the issue. I've replicated it in this sandbox: https://codesandbox.io/s/setsortsetfilter-je2em?file=/src/postList.jsx

Basically, if I use either of the action buttons on the Posts list view to try to simultaneously set the sort and filters, the filters are set but the sort reverts back to what it was before the button was pressed. If you comment out either the setSort or SetFilters method in the button handlers, whatever method remains will work as expected. The two methods just won't work in tandem.

Once again, thanks for your time!

[fzaninotto]

thanks for the sandbox, I managed to reproduce the issue. I'm reopening this ticket.

[fzaninotto]

The problem comes from the debounced version, because if I change your code to:

  const handleLeanne = () => {
    const newFilters = { userId: 1 };
    setFilters(newFilters, displayedFilters, false);
    setSort("title", "ASC");
  };
  const handleErvin = () => {
    const newFilters = { userId: 2 };
    setFilters(newFilters, displayedFilters, false);
    setSort("title", "DESC");
  };

It works.

We're using an internal debouncer to allow sync calls to setSort, setFilters, etc, and the lodash debouncer for setFilters alone. I think the two debouncers conflict with each other.

Until we find a fix for this issue, I recommend that you use the direct (non debounced) call to setFilters by passing false as the third parameter.

[ZachSelindh]

@fzaninotto Thank you for this suggestion! My form element that sets filters and sorts unmounts on load, so minimal risk to performance from un-debouncing setFilters. I've confirmed that in my application, your suggestion causes my functions to work with setSort and setFilters in tandem as expected.

Again, thank you so much for your time and attention.

[ZachSelindh]

As an update, I'd still love to see this bug fixed, as while defeating setFilters debounce allows setSort and setFilters to be called simultaneously, no debounce can result in errors such as incorrect dataprovider calls if a user changes list views while filters/sort are in the process of being set. I could probably whip together a codesandbox to display this if necessary.

Thanks!

[oparisblue]

+1 for this issue, I just ran into it as well.

In our case, we have a set of tabs which swap out Datagrids within a list, and we wanted to make one of thoseDatagrids default to sorting on a different column than the others. The tab change effectively calls setFilters to filter by the given status represented by one of the tabs.

The debouncing fix doesn't seem to work for us --- it just results in the sort getting applied, but not the filter.

[ZachSelindh]

In 4.0, we've found a workaround for this issue that I hope others will find helpful.

After upgrading, I examined the Saved Query feature that has become available in version 4, and the way that loading a saved query uses react-router's useNavigate hook to change the URL instead of changing list context with setFilters, setSort, etc. The relevant code is found in the FilterButton component.

I'm happy to report that calling navigate() with the list parameters will (so far) reliably change the filters and sort without debounce issues. Not sure if this closes the issue or not since presumably setSort and setFilters should still be able to be called simultaneously, but using useNavigate() essentially solves the implementation for my part.

[djhi]

We should probably expose a setListParams here as the navigate workaround isn't always the solution (Fields that exposes a ListContext)

[Revarh]

Hi,
Facing the same issue.
Although navigate() indeed resets sorting, it doesn't work when resetting filters.

Worse, the problem persists:

const handleResetFilters = () => {
    setFilters({})
    navigate()
  }

Here navigate() will do the same as setSort(), which is to go back to previous sorting.

[fzaninotto]

+1 for this issue, I just ran into it as well.

In our case, we have a set of tabs which swap out Datagrids within a list, and we wanted to make one of thoseDatagrids default to sorting on a different column than the others. The tab change effectively calls setFilters to filter by the given status represented by one of the tabs.

The debouncing fix doesn't seem to work for us --- it just results in the sort getting applied, but not the filter.

I can't replicate this problem in the e-commerce demo, where we have such a datagrid. I changed the TabbedDatagrid handleChange to:

    const handleChange = useCallback(
        (event: React.ChangeEvent<{}>, value: any) => {
            setFilters &&
                setFilters(
                    { ...filterValues, status: value },
                    displayedFilters,
                    false // no debounce, we want the filter to fire immediately
                );
            setSort({ field: 'date', order: 'DESC' });
        },
        [displayedFilters, filterValues, setFilters, setSort]
    );

And switching tabs properly changes both the filter and the sort.

Please open a new issue with a reproduction.

[fzaninotto]

After carefully testing, I can confirm that disabling the debounce by passing false as the third parameter to setFilters() fixes this problem in all cases. So I don't think we should introduce a new setParams callback, it's way too much changes for a problem that can be solved otherwise. Instead, we should just document a warning in setFilters saying that when used in conjunction with other callbacks, you have to disable the debounce.

I'm marking this as a documentation issue.