Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Selecting an item in ReferenceArrayInput does get #7480

Closed
RWOverdijk opened this issue Apr 4, 2022 · 18 comments
Closed

Selecting an item in ReferenceArrayInput does get #7480

RWOverdijk opened this issue Apr 4, 2022 · 18 comments
Labels

Comments

@RWOverdijk
Copy link
Contributor

RWOverdijk commented Apr 4, 2022

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

  • React-admin version: 4.0.0-rc.0
  • React version: 17.0.2
  • Browser: Chrome

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
Copy link
Contributor Author

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
Copy link
Contributor Author

RWOverdijk commented Apr 4, 2022

This video illustrates what I am talking about:

GETreq.mov

I think this might be related: #3108

@slax57
Copy link
Contributor

slax57 commented Apr 5, 2022

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
Copy link
Contributor Author

RWOverdijk commented Apr 6, 2022

@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
Copy link
Contributor

ZachSelindh commented Apr 6, 2022

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
Copy link
Contributor

slax57 commented Apr 7, 2022

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.

@slax57 slax57 added bug and removed needs more info labels Apr 7, 2022
@ZachSelindh
Copy link
Contributor

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
Copy link

@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
Copy link
Contributor

@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
Copy link
Contributor

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
Copy link

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
Copy link
Contributor

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
Copy link
Contributor

@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
Copy link
Contributor Author

RWOverdijk commented May 10, 2022

@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
Copy link
Contributor

slax57 commented May 16, 2022

@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
Copy link
Contributor

ZachSelindh commented Jul 6, 2022

@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
Copy link
Contributor

Tested with the new release and we are all good!

Thanks so much for your help in this issue.

@jashwant
Copy link

@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>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants