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.23k forks source link

Missing items in ArrayInput with SimpleIterator #6900

Closed horaklukas closed 3 months ago

horaklukas commented 2 years ago

What you were expecting: When I open posts list and then open post detail with iterated data (backlinks) using ArrayInput and SimpleFormIterator, all backlinks are visible.

What happened instead: No backlink is visible. After page refresh all backlinks are back there.

Steps to reproduce:

  1. Go to the Posts list - api return list of posts without backlinks property, because it's not needed in the list
  2. Open Post with backlinks (eg. id 3) and go to tab "Miscellaneous" - api return detail of post containing backlinks
  3. You should see list of backlinks but none are there until you refresh the page

Related code: https://codesandbox.io/s/frosty-tess-qzbxb?file=/src/posts/PostEdit.tsx

I did some debugging and found out that problem is probably inside SimpleFormIterator at lines 59-75. When the detail is loading and fields is empty, the nextId is initialized with 0 value which means ids become empty array. Once the detail is loaded, nextId nor ids is reevaluated and remain as it is which means ids is empty array, but fields contains backlinks

// We need a unique id for each field for a proper enter/exit animation
    // so we keep an internal map between the field position and an auto-increment id
    const nextId = useRef(
        fields && fields.length
            ? fields.length
            : defaultValue
            ? defaultValue.length
            : 0
    );

    // We check whether we have a defaultValue (which must be an array) before checking
    // the fields prop which will always be empty for a new record.
    // Without it, our ids wouldn't match the default value and we would get key warnings
    // on the CssTransition element inside our render method
    const ids = useRef(
        nextId.current > 0 ? Array.from(Array(nextId.current).keys()) : []
    );

Other information:

I suppose it's related to these issues

Environment

I know the another cause of the issue is that the entity (post in this case) has different shape for list and for detail, but it's a common use case for use, because some entities can be huge and we need only few simple fields for distinguishing them in a list.

Is it an antipattern for RA to have different entity shapes between endpoints?

WiXSL commented 2 years ago

@horaklukas, thanks for the report.

Did you change anything in the code sandbox? Because I can reproduce it in your sandbox but not in a new instance of https://codesandbox.io/s/github/marmelab/react-admin/tree/master/examples/simple

horaklukas commented 2 years ago

Oh, sorry, forgot to mention. I added removeBacklinksFromPostsList which is used in dataProvider to remove backlinks from posts in getList endpoint to simulate API behaviour.

WiXSL commented 2 years ago

@horaklukas, try it again in your codesandbox, but changing :

export default cacheDataProviderProxy(delayedDataProvider); for export default delayedDataProvider;

At the end of dataProvider.tsx file

horaklukas commented 2 years ago

You're right, it works after that change although it doesn't work in my code. Anyway, even with this change, it's still incorrect and it leads to this error again

client-hook-6.js:1 Warning: Each child in a list should have a unique "key" prop.

Check the render method of `SimpleFormIterator`. See https://reactjs.org/link/warning-keys for more information.
    at CSSTransition (https://qzbxb.sse.codesandbox.io/index.bundle.js:172084:35)
    at SimpleFormIterator (https://qzbxb.sse.codesandbox.io/index.bundle.js:127182:20)
    at div
    at FormControl (https://qzbxb.sse.codesandbox.io/index.bundle.js:11754:24)

because all Transitions obtain undefined as a key

 <CSSTransition
    // ids.current is empty array because of memoization 
    key={ids.current[index]}

https://github.com/marmelab/react-admin/blob/master/packages/ra-ui-materialui/src/input/ArrayInput/SimpleFormIterator.tsx#L136

WiXSL commented 2 years ago

I'll try to look into this and see what I can find

WiXSL commented 2 years ago

After #6932 gets merged, we should try if it fixes this issue

horaklukas commented 2 years ago

Ok, I'll give it a try, thanks

WiXSL commented 2 years ago

Oh, sorry, forgot to mention. I added removeBacklinksFromPostsList which is used in dataProvider to remove backlinks from posts in getList endpoint to simulate API behaviour.

If you remove the backlinks fromthe dataProvider like that you are going to have weird results with the cached version of the dataProvider, I don't think this is a valid use case.

WiXSL commented 2 years ago

@horaklukas, I think I misunderstood your issue, I believe this is indeed a bug. The index error was fixed in the last version (3.19.7) but the despairing items' problem is still there, and it shouldn't. This is reproducible in your original code-sandbox.

In fact, you have reached some of the same conclusions that in #5289. Which looks right (to me). I'll have to dig in further on this.

DawidMikulski commented 2 years ago

I have the same problem, when you inspect an element you will see that the reason is "fade-enter" class.

<ul class="RaSimpleFormIterator-root-149">
    <li class="RaSimpleFormIterator-line-150 fade-enter">[...]</li> // hidden element - bug
    <li class="RaSimpleFormIterator-line-150 fade-enter-done">[...]</li> // visible element
    <li class="RaSimpleFormIterator-line-150">[...]</li> // panel for adding new entry 
</ul>

In my case, it doesn't even help to restore react-admin to v3.0.0. Something wrong with MUI?

hugofialho commented 2 years ago

i have same issue, my model "questions" have an array "responses" that is only available in the method getOne of the dataProvider. In the getList my API respond with only a few fields. When i editi a record the form only render the last response of the array, if i refresh the page render all elements of the array correctly.

asagafonov commented 2 years ago

Experiencing the same issue here.

\ within \ doesn't render any but the last element of the array. The other ones stay empty until the page is refreshed.

I also noticed that the "fade-enter" attribute isn't triggered in any but the last \

  • element (should be "fade-enter-done", so it seems).

    Any clues on solving this?

  • rochelle-norman commented 2 years ago

    I am having the same issue <SimpleFormIterator> within <ArrayInput> doesn't render all of the array elements until the page is refreshed. Seems like the same issue above with thefade-enter animation not completing. Any ideas on how to fix this issue?

    tong-bluehill commented 2 years ago

    +1. Same issue. Only display last item but not other items. In browser developer console, I manually set 'fade-enter' to 'fade-enter-done'. Then the item is displayed. Before fixing the bug, is there a workaround?

    nels-m commented 2 years ago

    I am also experiencing this issue. The first element of a SimpleFormIterator does not load, but appears after page refresh.

    Is it possible to disable the fade animation for SimpleFormIterator as a workaround?

    tong-bluehill commented 2 years ago

    I have a very simple workaround. Disable the animation. The form function still work.

    <SimpleFormIterator TransitionProps={{ enter: false, exit: false }}>
    </SimpleFormIterator>
    nels-m commented 2 years ago

    @tong-bluehill Thank you!

    nurizzzka commented 2 years ago

    @tong-bluehill Thank you very much!!!

    rafahoro commented 2 years ago

    Based on @tong-bluehill workaround:

    I have a very simple workaround. Disable the animation. The form function still work.

    <SimpleFormIterator TransitionProps={{ enter: false, exit: false }}>
    </SimpleFormIterator>

    This is how I've solved in "react-admin": "^3.19.11",

    import { CSSTransitionProps } from 'react-transition-group/CSSTransition';
    ...
    
    const TransitionProps: CSSTransitionProps = {
      classNames: "",
      addEndListener: () => undefined,
    }
    ...
    ...
       <SimpleFormIterator TransitionProps={TransitionProps}>
    
    horaklukas commented 2 years ago

    I've just updated to react-admin@4.1.1 and it appears the issue is gone there along with the animation of a list (which I don't mind) 👍

    ghost commented 2 years ago

    For v3 I fixed this error by providing ArrayInput with a defaultValue of an array with a single empty object...

    I only needed this because my getList query doesn't return the spec field at all (it's a huge JSON object), but when react-admin asks for a details record (for editing), my server returns the spec field. So, the optimistic rendering of v3 was having errors while it used the cached data. Problem now solved.

          <ArrayInput
            source="spec.things"
            label="Things"
            defaultValue={[{}]}
          >
            <SimpleFormIterator>
              <TextInput
                source="name"
                fullWidth
              />
              <OtherCustomInput source="yada" />
            </SimpleFormIterator>
          </ArrayInput>
    ghost commented 2 years ago

    The workaround that I detailed above only works until you add a 2nd array item. Then, whenever you edit that record it has the same error because it starts with a defaultValue which has only 1 element, but then while you're loading a record that has 2 or more elements, it loses the key.

    The workaround for v3 that finally did it for me was to only render the ArrayInput after the record was finished loading. Here's the quick and simple way that I did that. (It could probably be done better, but here it is...)

      /** True if we are CURRENTLY loading. */
      const isLoading = useLoading();
      /** True if we WERE loading. */
      const wasLoadingRef = React.useRef(false);
      /** True if we WERE loading and now we're NOT. */
      const isLoaded = React.useMemo(() => {
        if (isLoading) {
          wasLoadingRef.current = true;
          return false;
        } else {
          return wasLoadingRef.current;
        }
      }, [isLoading]);
    
      return (
        <Edit {...props}>
          <SimpleForm>
            <TextInput source="yada" />
            {isLoaded && (
              <ArrayInput
                source="spec.queries"
                label="Queries"
              >
                <SimpleFormIterator>
                  <TextInput source="whatever" />
                  {/* ... */}
                </SimpleFormIterator>
              </ArrayInput>
            )}
          </SimpleForm>
        </Edit>
    ghost commented 2 years ago

    This just in: The last workaround doesn't work all the time either. It does work some of the time. So, the workaround will now be to ignore this error until we upgrade this project :)

    fzaninotto commented 3 months ago

    With the release of react-admin v5, react-admin v3 has reached its end of life. We won't fix bugs or make any new release on the 3.x branch. We recommend that you switch to a more recent version of react-admin.

    So I'm closing this issue as we won't fix it.