refine icon indicating copy to clipboard operation
refine copied to clipboard

[BUG] useDataGrid and syncWithLocation set to true triggers the same request twice

Open levipro opened this issue 2 years ago • 6 comments

Describe the bug

When using the useDataGrid hook for mui, and setting an initial sort along with the syncWithLocation set to true the same request gets sent twice:

useDataGrid({
    initialCurrent: 1,
    initialPageSize: 10,
    sorters: {
      initial: [
        {
          field: "Name",
          order: "asc",
        },
      ],
    },
    syncWithLocation: true,
    ...

What happens is the initial request gets made, then the URL is synced, and the second (same) request is triggered because the meta that is sent to the dataProvider has the following information in it:

meta: {
      "sorters[0][field]": "Name",
      "sorters[0][order]": "asc",
      ...

If i set those same keys in the useDataGrid hook then there is only a single request made.

Steps To Reproduce

  1. Run refine with mui and use the useDataGrid hook
  2. Set an initial sort
  3. Set syncWithLocation to true
  4. Load page and monitor your network. A second duplicate request will be made to the API

Expected behavior

Setting initial sort value should immediately sync to the meta key in order to prevent the second request. Or an alternative method to eliminate this second duplicate request.

Screenshot

No response

Desktop

No response

Mobile

No response

Additional Context

No response

levipro avatar Jan 15 '24 20:01 levipro

It looks like the issue might be related to how the useDataGrid hook handles the synchronization of sorting information with the URL when syncWithLocation is set to true. The described behavior indicates that the initial request is triggered correctly, but then a duplicate request is made due to the way the sorting information is included in the meta data.

To address the issue, you can try modifying the handleSortModelChange function in the useDataGrid hook. Specifically, you can update the function to immediately sync the sorting information with the URL when the sort model changes, preventing the need for a second request.

Link to that specific code.

Here's a modified version of the handleSortModelChange function:

const handleSortModelChange = (sortModel: GridSortModel) => {
    const crudSorting = transformSortModelToCrudSorting(sortModel);
    setSorters(crudSorting);

    // Immediately sync the sorting information with the URL
    if (syncWithLocationProp) {
        const link = createLinkForSyncWithLocation({ sorters: crudSorting });
        window.history.replaceState({}, "", link);
    }
};

This modification should update the URL immediately when the sorting model changes, avoiding the need for an additional request. Make sure to test this change in your application to ensure it resolves the described issue.

Will it create any other issue if we modify the code in that way @BatuhanW @aliemir ?

Conqxeror avatar Jan 16 '24 09:01 Conqxeror

Hello @Conqxeror thanks for the investigation, it looks great!

I think we need to confirm that duplicate request isn't caused by strict mode, see our FAQ here: https://refine.dev/docs/guides-concepts/faq/#why-are-api-calls-triggering-twice cc @levipro

BatuhanW avatar Jan 16 '24 13:01 BatuhanW

Hey @levipro sorry for the issue! This is a valid issue that we've been discussing with our team to provide a proper solution. We're passing the router parameters (parameters and query string) to the data providers with meta property and it's also used in the query keys. We've discussed about having a prop or a config to prevent router values to be included to the meta which user can then decide whether they should be included or not 🤔 We've also discussed to always remove the known parameters from the meta such as parameters from syncWithLocation like in this case.

I'll provide an update here as soon as we decide on the changes and working on the implementation. Please do not hesitate share if you have any ideas and comments on this.

aliemir avatar Jan 18 '24 06:01 aliemir

Hello @levipro, are you using Next.js?

Yesterday we released new version of @refinedev/nextjs-router and we believe this issue is fixed. Can you upgrade @refinedev/nextjs-router to @5.5.7 and try again please?

alicanerdurmaz avatar Feb 07 '24 06:02 alicanerdurmaz

Hey @levipro sorry for the issue! This is a valid issue that we've been discussing with our team to provide a proper solution. We're passing the router parameters (parameters and query string) to the data providers with meta property and it's also used in the query keys. We've discussed about having a prop or a config to prevent router values to be included to the meta which user can then decide whether they should be included or not 🤔 We've also discussed to always remove the known parameters from the meta such as parameters from syncWithLocation like in this case.

I'll provide an update here as soon as we decide on the changes and working on the implementation. Please do not hesitate share if you have any ideas and comments on this.

Hello, @aliemir. Just wanted to chime in that I'm facing this issue too, but my use case is a bit different. I'm using useDataGrid with another hook that also has syncWithLocation (useModalForm from @refinedev/react-hook-form). It's a list page with create/edit/show modals for the resource.

What happens is that the modal adds a modal-<resource>-create query to the params when it's open, and it gets included in the meta like you said, which causes the table to refresh. In this case, the second solution you mentioned (automatically removing known parameters) might not work.

suphon-t avatar Feb 07 '24 11:02 suphon-t

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.

stale[bot] avatar Apr 07 '24 11:04 stale[bot]