marmelab / react-admin

A frontend Framework for single-page applications on top of REST/GraphQL APIs, using TypeScript, React and Material Design
http://marmelab.com/react-admin
MIT License
24.89k stars 5.24k forks source link

Selecting an item in ReferenceArrayInput does get #7480

Closed RWOverdijk closed 2 years ago

RWOverdijk commented 2 years ago

What you were expecting:

No GET request

What happened instead:

A GET request

Steps to reproduce:

I have a relatively simple form:

import * as React from 'react';
import { Create, SimpleForm, TextInput, ReferenceArrayInput, SelectArrayInput } from 'react-admin';

const resourceSort = { field: 'resource', direction: 'asc' };

export const RoleCreate = (props: any) => (
  <Create {...props}>
    <SimpleForm>
      <TextInput source="name" />
      <ReferenceArrayInput source="permissions" reference="permissions" sort={resourceSort}>
        <SelectArrayInput optionText="action" />
      </ReferenceArrayInput>
    </SimpleForm>
  </Create>
);

After selecting an item in the dropdown a GET request is performed to:

Request URL: http://localhost:5201/api/permissions?id=69437072-89a2-496c-ade8-3970e7cd0841

I see no reason for this since we already have a list.

Related code:

In steps to reproduce.

Other information:

Environment

Additional

Bonus question: I also notice that sends both the ids and the objects. Is this on purpose? Can I disable one or the other? See screenshot for what I mean. This is after updating a permission id.

image
RWOverdijk commented 2 years ago

I had to jump in a meeting so I realise the issue is a bit thin as it is. If this isn't a bug I can't find in the docs why this is needed. If the description of the issue isn't clear enough I'll try adding info after my meeting.

RWOverdijk commented 2 years ago

This video illustrates what I am talking about:

https://user-images.githubusercontent.com/781745/161532535-b195def1-70db-4a1c-8d7e-a9a393831b44.mov

I think this might be related: https://github.com/marmelab/react-admin/issues/3108

slax57 commented 2 years ago

Hi! I cannot reproduce your issue with a codesandbox based on the "next" branch. https://codesandbox.io/s/upbeat-frost-no1kv7?file=/src/posts/TagReferenceInput.tsx I only see "getMany" calls in the console. Can you try with this codesandbox, or provide one that reproduces the issue? Thanks

RWOverdijk commented 2 years ago

@slax57 Hi! The reason you can't reproduce it in your sandbox is because it is not using an api. It's using local data and so there are no network requests.

It does however do getMany calls (the ones you also see in the console) and that's exactly what I'm talking about 😄 Those shouldn't be needed.

ZachSelindh commented 2 years ago

Definitely experienced this as well on our 4.0 refactor.

I would add that ReferenceInputs with an AutocompleteInput child seem to make an extra call (even beyond the redundant getOne) using the optionText of the AutoCompleteInput and the hard filters of the ReferenceInput. Here's an example photo, I'm sure I could recreate using a CodeSandbox if needed.

This call will probably fail anywhere that the optionText is using a custom string combining record fields. And the returned data from this redundant call doesn't seem to be making its way into the input/form either way.

In the below picture, the optionText is a combination of three different fields, and the whole optionText is being used for filters in the highlighted call.

extraRefCall

slax57 commented 2 years ago

Okay, I think I get it now. Thank you both for your answers. I got confused about the get vs. getMany because of the issue description mentioning "A GET request" instead of "No GET request".

But you're right, I believe the fix brought by https://github.com/marmelab/react-admin/pull/3252/files#diff-2ce97bf121b48fb61e5ace9d046319b5465971faa538f5f5c84b735cd9019823 did not make its way to the v4, hence leading to superfluous calls to the dataProvider.

I'll relabel this as a bug. Thanks again.

ZachSelindh commented 2 years ago

Not sure if this is related or not (if not, I'll open a new issue) but I've noticed that my ReferenceInputs on forms re-make their calls when I blur the form and return to it.

I've already set up a new queryClient as per issue #7433, which stopped new calls for list view values, but hasn't affected the repeated ReferenceInput calls on forms.

joobacca commented 2 years ago

@ZachSelindh I am experiencing the "refetch on refocusing after blur" issue as well, I'll assume you haven't found a solution to it yet?

ZachSelindh commented 2 years ago

@joobacca The original solution I was offered on this issue is posted in issue #7433, but that didn't initially work.

It looks like it may be solved by pull request #7558, which is a 4.0.2 milestone.

Either way the custom queryClient will likely be necessary as the re-fetches are default behavior in react-query.

ZachSelindh commented 2 years ago

FYI, During my 4.0 refactor I've found that using react-hook-form's 'setValue' method to alter the value of a ReferenceInput has the same effect of both making the initial call for the requested record, and then a subsequent redundant call using the text generated by the optionText on the input component.

joobacca commented 2 years ago

Yeah, same here - I noticed that behavior when setting the value of an ArrayInput which contains ReferenceInputs. Kinda problematic when the ArrayInput has 10+ elements and the API gets fetched equally as much, in addition to the other ReferenceInputs outside of the ArrayInput

ZachSelindh commented 2 years ago

As an update; As of 4.0.2, ReferenceInputs are no longer re-making their calls to get choices on focus when passing a custom queryClient with "refetchOnWindowFocus: false".

Any updates to the base issue of the extra data calls? This bug is potentially breaking several of our forms that rely on setValue (formerly form.change) to populate 15-20 line items on the form automatically; the setValue process takes up to a minute to execute, and I'm suspicious that the extra ReferenceInput calls are a big part of the problem.

ZachSelindh commented 2 years ago

@slax57 Any updates from RA on this issue? It's one of the things keeping our team from migrating to 4.0 since our larger forms with many ReferenceInputs get bogged down making unnecessary calls, and performance grinds to a halt.

RWOverdijk commented 2 years ago

@ZachSelindh I'd assume that if there were updates they'd be added here.

Another option is to try and fix it yourself. It sounds pretty important to you, so it's worth spending time on.

slax57 commented 2 years ago

@slax57 Any updates from RA on this issue? It's one of the things keeping our team from migrating to 4.0 since our larger forms with many ReferenceInputs get bogged down making unnecessary calls, and performance grinds to a halt.

Hi! We will try to include an improvement for this in the next minor release (4.1.0), but I can't guarantee it, since the fix will probably not be trivial and require some time and thinking through to deal with the underlying react-query behaviour.

ZachSelindh commented 2 years ago

@slax57 I could be wrong but it appears that PR #7909 has resolved this issue by updating doesQueryMatchSelection to prevent calls if the filter matches the selection. If I'm correct, this issue can be closed.

EDIT: The extra GET request on selection still occurs, what has been fixed is the redundant calls that use the optionText as a filter.

davidhenley commented 2 years ago

Tested with the new release and we are all good!

Thanks so much for your help in this issue.

jashwant commented 1 year ago

@ZachSelindh, Did you find solution?

I just need a simple Autocomplete filled with the static data I already have (User does not fetch remote data by typing).

But ReferenceInput always sends 1 additional request on each different selection from Autocomplete dropdown.

Also if user types, it sends additional requests per n character typed (n depends on options passed).

Here's how I dealt with it hackishly (better way would be to provide value to ChoicesContext.

export const MyReferenceInput = ({ source, children, reference, sort, perPage }) => {
  const { data, isLoading, error } = useGetList(reference, {
    pagination: { page: 1, perPage },
    sort,
  });

  if (isLoading || data?.length === 0) {
    return <CircularProgress />;
  }
  if (error) {
    return <></>;
  }

  return cloneElement(children, {
    source: source,
    choices: data,
  });
};

Usage:

 <MyReferenceInput
        source="user_id"
        reference="users"
        perPage={10}
        sort={{ field: "id", order: "ASC" }}
      >
        <AutocompleteInput optionText="username"  />
</MyReferenceInput>