[BUG] not clear filters and sorters when clicking menu item

[dfang]

Describe the bug

In a table, apply some filters then click the menu item in left side bar, the url should reset filters and sorters (only apply default ones)

Steps To Reproduce

1. go to https://example.admin.refine.dev/orders
2. apply status filter
3. click 'Orders' in left side bar

Expected behavior

reset the filters

Packages

    "@ant-design/charts": "^1.4.2",
    "@ant-design/graphs": "^1.4.0",
    "@ant-design/icons": "5.0.1",
    "@dnd-kit/core": "^6.1.0",
    "@dnd-kit/modifiers": "^7.0.0",
    "@dnd-kit/sortable": "^8.0.0",
    "@emotion/react": "^11.11.1",
    "@emotion/styled": "^11.11.0",
    "@googlemaps/react-wrapper": "^1.1.35",
    "@refinedev/antd": "^5.42.0",
    "@refinedev/cli": "^2.16.36",
    "@refinedev/core": "^4.53.0",
    "@refinedev/devtools": "^1.2.6",
    "@refinedev/graphql": "^6.5.4",
    "@refinedev/inferencer": "^4.6.6",
    "@refinedev/kbar": "1.3.12",
    "@refinedev/react-router-v6": "^4.5.11",
    "@refinedev/simple-rest": "^5.0.8",
    "@refinedev/supabase": "^5.9.2",

Additional Context

No response

[alicanerdurmaz]

Hey @dfang, thanks for reporting the issue. I will look into this and let you know when I find a solution.

[arndom]

@alicanerdurmaz Can I work on this

[BatuhanW]

Hey @arndom sure. Assigning to you.

[arndom]

Hello @BatuhanW

While attempting to resolve this issue, I discovered this bug is present even in new refine projects. I checked multiple examples as well and it's the same thing.

Eventually, I found that the issue is coming from the useTable hook in @refinedev/core, as this is what is used to update the URL based on the filters and sorters coming from the tables (Antd, MUI).

How do I go about running the refinedev/core package?

[aliemir]

I want to clarify the issue here and propose a small workaround. Refine's syncWithLocation works in one direction and only infers the parameters at initial render to avoid syncing issues with the state and the query params.

• When useTable is mounted and a routerProvider is present and ready, filters, sorters, search etc. are inferred from query params.
• Those values are stored in a state in the useTable hook, any changes in those states will update the query params.
• If there are any changes in the query params that is not caused by useTable and if those changes doesn't re-mount the hook, those changes are not synced with the internal states.
• Initially, this was done to avoid unnecessary renders. Having a two-way binding will probably require deep equality checks and might be more complex than it seems to properly implement.

We've discussed some changes and refactors about this feature with the team before but it wasn't prioritized. We don't want this issue to be resolved partially and only work for some use cases such as only clearing it when query params are emptied. We'd be happy help if anyone wants to work on a proper implementation with two-way binding 🙏

For now, as a workaround a small effect can be added to the page to check for the changes in the query params and reflect those. Check the example below:

const { params } = useParsed();
const { setFilters, setSorters } = useTable();

React.useEffect(() => {
  // actual implementation might be different and slightly more complex depending on the use case.
  if (!params.filters) setFilters([], "replace");
  if (!params.sorters) setSorters([]);
}, [params]);

[arndom]

Ah ok, having the query changes affect wherever synching is done is a better solution.

Where do I start though? From what I can see the syncWithLocation value after being defined in RefineContext is used in multiple implementations, the useTable hook for example, so when in sync the params changes should be reflected.

This is off the top of my head but does that mean changes would have to be made wherever params synching is needed? Or are there more efficient solutions your team has discussed?

[arndom]

hey @aliemir I finally had some time to work on this. My approach is to use the params from useParsed() as it is always up to date with the URL.

We can then compare the "new params" and the current params from the internal, if there is a match do nothing, otherwise rebuild internal state with the new query params. Possibly something like this in the useTable hook:

const current_sorters = differenceWith(
      sorters,
      preferredPermanentSorters,
      isEqual,
    );

const final_sorters = differenceWith(
      parsedParams.params.sorter,
      current_sorters ,
      isEqual,
    );

For the other implementations that depend on the syncWithLocation variable a similar approach could be used. What do you think?

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[arndom]

@aliemir I'm stuck on the issue of fixing the exceeding of the max depth of the useEffect.

• The process starts with storing the last sync of state params(filters and sorters) in a ref.

• So in the useEffect, I compare the current URL params and state params.

  • I also compare the last synched state params and the current state params, this ensures that internal changes are done before attempting to compare with the URL params.

• When there is a mismatch, the URL and state params are merged, if there is no param in the URL, the state is re-initialized.

• The logic for this URL -> State binding works fine alone.

• The issue is that this useEffect doesn't properly wait for the pre-existing useEffect that updates the URL params triggered by a state change to be done. Here: https://github.com/refinedev/refine/blob/57c352cdf93043b5c650a4c709d6c04de3357c33/packages/core/src/hooks/useTable/index.ts#L455

• I've tried a timeout(which is not optimal) before updating params in the new useEffect but it worked half the time in my tests.

• Due to this, the URL and State will never match, triggering the useEffect to be continuously called.

• I've pushed the latest, with my useEffect commented out, I would appreciate some assistance. Here

[BatuhanW]

Hey @arndom feel free to create a PR, would be easier to review and help on there.

[arndom]

Alright @BatuhanW, I've created one #6645

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[fer-ri]

hi guys

Any update?

For now I use something like @aliemir hint, but it would be very nice if this feature is built-in the package itself.

Thanks

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[fer-ri]

Pls dont stale it

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.