react-admin icon indicating copy to clipboard operation
react-admin copied to clipboard

Missing items in ArrayInput with SimpleIterator

Open horaklukas opened this issue 4 years ago • 23 comments

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

  • https://github.com/marmelab/react-admin/issues/5621
  • https://github.com/marmelab/react-admin/issues/5289

Environment

  • React-admin version: 3.19.2
  • Last version that did not exhibit the issue (if applicable): Seems like it was working in 3.18.1
  • React version: 17.0.2
  • Browser: any

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?

horaklukas avatar Nov 24 '21 16:11 horaklukas

@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

WiXSL avatar Nov 24 '21 17:11 WiXSL

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.

horaklukas avatar Nov 24 '21 17:11 horaklukas

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

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

At the end of dataProvider.tsx file

WiXSL avatar Nov 29 '21 18:11 WiXSL

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

horaklukas avatar Nov 30 '21 09:11 horaklukas

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

WiXSL avatar Nov 30 '21 11:11 WiXSL

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

WiXSL avatar Dec 04 '21 22:12 WiXSL

Ok, I'll give it a try, thanks

horaklukas avatar Dec 22 '21 17:12 horaklukas

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 avatar Dec 29 '21 12:12 WiXSL

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

WiXSL avatar Jan 28 '22 12:01 WiXSL

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?

DawidMikulski avatar Jan 31 '22 20:01 DawidMikulski

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.

hugofialho avatar Mar 10 '22 11:03 hugofialho

Experiencing the same issue here.

<SimpleFormIterator> within <ArrayInput> 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 <li> element (should be "fade-enter-done", so it seems).

Any clues on solving this?

asagafonov avatar Mar 11 '22 13:03 asagafonov

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 the fade-enter animation not completing. Any ideas on how to fix this issue?

rochelle-norman avatar Mar 14 '22 12:03 rochelle-norman

+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?

tong-bluehill avatar Mar 20 '22 22:03 tong-bluehill

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?

nels-m avatar Mar 22 '22 22:03 nels-m

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

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

tong-bluehill avatar Mar 23 '22 18:03 tong-bluehill

@tong-bluehill Thank you!

nels-m avatar Mar 23 '22 18:03 nels-m

@tong-bluehill Thank you very much!!!

nurizzzka avatar Apr 14 '22 06:04 nurizzzka

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

rafahoro avatar May 06 '22 17:05 rafahoro

I've just updated to [email protected] and it appears the issue is gone there along with the animation of a list (which I don't mind) 👍

horaklukas avatar Jun 17 '22 07:06 horaklukas

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>

waynebloss avatar Jun 22 '22 18:06 waynebloss

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>

waynebloss avatar Jun 22 '22 22:06 waynebloss

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 :)

waynebloss avatar Jun 22 '22 22:06 waynebloss