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

Unmounting one of multiple components that share a duplicate query deletes RecordSource records #2595

Open
BJTerry opened this issue Dec 27, 2018 · 4 comments

Comments

@BJTerry
Copy link

BJTerry commented Dec 27, 2018

If you include two QueryRenderers on a page that have the same query, and then you remove one, it deletes the store data associated with the component because fetchQueryAndComputeStateFromProps in ReactRelayQueryRenderer.jsx doesn't properly handle duplicate requests from different components.

When fetchQueryAndComputeStateFromProps is called the first time, it calls fetch in ReactRelayQueryFetcher, which then calls execute. After the environment executes, eventually environment.retain is called in ReactRelayQueryFetcher.execute, which retains the returned data in the RelayMarkSweepStore (assuming you are using the normal store setup). The second time fetchQueryAndComputeStateFromProps is called it does not call execute because the query matches an in-flight request. Therefore, the data is not retained a second time in the RelayMarkSweepStore. When you unmount either component, it disposes the selector in RelayMarkSweepStore, which performs a gc, deleting the data for all remaining components.

I attempted to create a repro of this, but the glitch site linked in the Issues description is extremely out of date and doesn't support Relay Modern. The above was determined using Relay 1.7.0.

@BJTerry
Copy link
Author

BJTerry commented Dec 27, 2018

I've created a reproduction of the issue here:

https://github.com/BJTerry/relay-examples

  1. clone repo
  2. cd to todo
  3. yarn install yarn build yarn update-schema yarn start
  4. Go to localhost:3000
  5. I've created two copies of the component at page load. Above them there is a hard to see button that says Delete entry. Click that button.
  6. Click any checkbox on the remaining Todos.

You should see an error in the console like this:

TodoList.js:77 Uncaught TypeError: Cannot read property 'edges' of undefined
    at TodoList.renderTodos (TodoList.js:77)
    at TodoList.render (TodoList.js:101)
    at finishClassComponent (react-dom.development.js:13393)
    at updateClassComponent (react-dom.development.js:13356)
    at beginWork (react-dom.development.js:13945)
    at performUnitOfWork (react-dom.development.js:16249)
    at workLoop (react-dom.development.js:16287)
    at HTMLUnknownElement.callCallback (react-dom.development.js:145)
    at Object.invokeGuardedCallbackDev (react-dom.development.js:195)
    at invokeGuardedCallback (react-dom.development.js:248)
react-dom.development.js:14389 The above error occurred in the <TodoList> component:
    in TodoList (created by ContainerConstructor)
    in ContainerConstructor (created by ForwardRef(Relay(TodoList)))
    in section (created by TodoApp)
    in div (created by TodoApp)
    in TodoApp (created by ContainerConstructor)
    in ContainerConstructor (created by ForwardRef(Relay(TodoApp)))
    in ReactRelayQueryRenderer (created by App)
    in div (created by App)
    in App

Consider adding an error boundary to your tree to customize error handling behavior.
Visit https://fb.me/react-error-boundaries to learn more about error boundaries.

And the entire page crashes. I've created a video of the reproduction here: https://cl.ly/cdab28ee298f

@josephsavona
Copy link
Contributor

cc @jstejada

@josephsavona
Copy link
Contributor

Thanks for reporting this! As a workaround note that GC can be disabled with environment.getStore().__disableGC().

@sibelius sibelius added the bug label Jan 10, 2019
@stale
Copy link

stale bot commented Dec 25, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Dec 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants