Bug: Different behavior about rerendering on development and production

[richardaum]

Describe the bug

I am writing a React+react-query application and I am running into a problem when I switched from development mode to production mode. Production mode is a mix of running npm run build and serving those HTML/JavaScript/CSS files inside a server (maybe using npx serve dist). While development mode is just running npm run dev (or npm start if using CRA).

const useMyQuery = () =>
  useQuery(
    ["myAwesomeData"],
    async () => {
      const { data } = await axios.get(`http://http.us/500`);
      return data;
    },
    { placeholderData: { myAwesomeProperty: [] } }
  );
const App = () => {
  const { isError } = useMyQuery();
  return isError ? <div>Error</div> : <Child />;
};
const Child = () => {
  const { data } = useMyQuery();
  return (
    <pre>
      <code>{JSON.stringify(data.myAwesomeProperty)}</code>
    </pre>
  );
};

For the sake of simplicity, I omitted QueryClientProvider and its client. Also placeholderData is being used as a loading state, which allows Child to be properly rendered while real data is loading.

The most important part here is data.myAwesomeProperty AFTER the loading state - which throws an error ONLY when running in production. The error produced is:

TypeError: Cannot read property 'myAwesomeProperty' of undefined

When running on development, the <div>Error</div> appears as expected.

My main concern here is NOT solve the problem (as I could simply add a if statement on Child component to prevent accessing data.myAwesomeProperty).

Instead, I would like to know why there is a difference between development's and production's behavior? Is it documented somewhere?

To Reproduce
Steps to reproduce the behavior:

1. https://codesandbox.io/s/react-query-rerender-6pvhy
2. Run to see the Error word
3. Deploy to any option available and see the expcetion on console terminal.
4. You can also see it deployed here: https://csb-6pvhy.netlify.app/

OR

1. Clone the repo: https://github.com/richardaum/error-sandbox
2. Run npm run dev to run development mode. See the "Error" word appearing.
3. Run npm run build and serve the html using npx serve dist. See the exception on console terminal.

Expected behavior
The "Error" word should also appear on production mode and shouldn't be any differences.

Desktop

• react: 17.0.2
• react-query: 3.21.1
• OS: Windows 10
• Browser: Chrome
• Version: 93.0.4577.63

[richardaum]

@TkDodo is helping me a lot goining deeper in this problem. Our main guess is around the Observer used by react-query.

observer creation is in the order: parent -> child, but observer subscription is in the order child -> parent

It looks like the changes are informed in a different order on development and production.

[TkDodo]

so I found out that if we add observers from top to bottom, the error is gone. so changing this line:

https://github.com/tannerlinsley/react-query/blob/e6da990703482cff8ea99f5a254975cb57463ed5/src/core/query.ts#L312

to like:

[observer].concat(this.observers)

made it work for me so that we inform observers in the "right" order.

This is obviously not the only place where we add observers, and inverting the order only works for observers that come from useBaseQuery, so it's probably not correct for all situations. Have to think about this some more.

[richardaum]

I've tried that you suggest and it worked, but I'm still thinking about what would be the "right" order.

It looks like useEffect runs bottom-up. Even that being a not well-documented officially, the community seems to already know that [1] [2]:

Notice, though, that React renders from the bottom up! In this case: Bottom, then Middle, then Top. It’s recursive – the parent is not “done” until all of its children have rendered, and the useEffect will only run after the render of a component is complete.

Stale props and zomibe children in Redux.
(at some point...) Since useEffect (or componentDidMount) fires from bottom to Top [...].

Should we enforce a top-down approach? Even useEffect being bottom-up? By concating (or unshifting) the observer, we are assuming a top-down direction.

But even so, the main question still remains - we are already pushing the observer to the observers list. Then why the behavior is odd only on development (or non-production) environment?

[TkDodo]

Then why the behavior is odd just on development (or non-production) environment?

yes, this is the real question and I don't really have an answer to this now. What I think we could theoretically do is give each observer a timestamp when it was created, which happens synchronously, and then once the subscription is triggered add it to the correct position in the array of observers - basically sorting it by creation timestamp.

But generally, I would advise to think of components that use a query as being self-contained, and not rely on the fact that you only render the component if it has been checked that there is no error higher up the component tree. TypeScript also won't let you do that, because it can't know that. If we stick with that, the order of subscriptions invocations doesn't really matter I believe.

[TkDodo]

@richardaum is this maybe related to #2675?

[richardaum]

Yes, it is.

Already tested this fix (#2703) locally and it works here! As soon as it is merged and released, I can close this issue 🎉