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.8k stars 5.23k forks source link

useShowController returns data for a previous record on first show component re-render #5492

Closed panfiva closed 3 years ago

panfiva commented 3 years ago

What you were expecting: When I use useShowController(props), I want it to always return data for the record I am requesting as specified by props.id; however, when I change record ID in the URL, it returns previous record during first render.

What happened instead: useShowController returns previous record

Steps to reproduce:

  1. Navigate to https://el6sp.sse.codesandbox.io/#/comments/1/show
  2. Change ID in the url from "1" to "2": https://el6sp.sse.codesandbox.io/#/comments/2/show

first line in the image below is a subset for component's props; the second is a subset of data returned by useShowController. Each table row shows data from each component re-render. Notice that when i changed ID from 1 to 2 in the URL, the first re-render showed data from element with ID#1

image

const CommentShow = (props) => {
  const controller = useShowController(props);

  const { loading, loaded, record } = controller;
  const { id, location } = props;

  const ref = React.useRef([]);

  ref.current.push(
    <div style={{ border: "1px green solid" }}>
      <pre>
        {JSON.stringify({ id, location: { pathname: location.pathname } })}
      </pre>
      <pre>
        {JSON.stringify({ loading, loaded, record: { id: record?.id } })}
      </pre>
    </div>
  );

  return ref.current;
};

Related code: https://codesandbox.io/s/great-carson-el6sp?file=/src/comments/CommentShow.js

Other information:

Environment

fzaninotto commented 3 years ago

Reproduced. Thanks for the report!

fzaninotto commented 3 years ago

In fact, I'm not sure it's a bug. Due to how React works, the change of id triggers an effect after the component renders. This effect updates the record, which triggers another render. So there are 2 consecutive renders, and that's how React works.

My question is: why is that a problem in your case?

panfiva commented 3 years ago

I am embedding Tableau reports into the application and PowerBi reports and pick one or the other based on properties returned by a record. Due to poor design of Tableau Javascript API and tableau embedded, it does not work with Flexbox, so I had to resister and window onResize event listener and manipulate DOM directly for Tableau reports. I discovered this issues when I noticed DOM manipulation artifacts when switching from Tableau to PowerBI reports.

I was able to work around the issue within my show component as follows:

const controller = useShowController(props)

// useShowController may return old data after ID has changed due to bugs; manually override
  if (controller.record && controller.record.id !== props.id) {
    console.log('ReportShow fixed')
    controller.record = undefined
    controller.error = null
    controller.loading = true
    controller.loaded = false
  }

This way, first re-render is manually updated to pretend that no data has been returned.

fzaninotto commented 3 years ago

Good, so there is a way to circumvent this in user land.

As this is a corner case that would cost a lot to fix (including making SSR much harder by using useLayoutEffect instead of useEffect everywhere we do data manipulation), we won't address it. So I'll close your issue as a "won't fix".

Thanks for your detailed explanations!