search

facebook / relay

Relay is a JavaScript framework for building data-driven React applications.
https://relay.dev
MIT License
18.41k stars 1.83k forks source link

@deleteRecord should delete from all connections/lists #3579

Open ianplatt555 opened 3 years ago

ianplatt555 commented 3 years ago

Version: v12.0.0

@deleteRecord does not properly remove references to the record from other places in the store such as inside connections and lists. It instead turns them all into dangling null references. This seems like a bug because why would you ever want to leave those nulls hanging around referencing records that no longer exist?

ianplatt555 commented 3 years ago

cc: @zth @n1ru4l who have mentioned related work before in https://github.com/facebook/relay/pull/3148

Also @Blitz2145 who asked a similar question on GraphQL discord.

damikun commented 3 years ago

I think this is kind hard to do since you need to iterate over entire store and remove that refs.. Another thing is what will happen with UI in case it lost reference.. In case it is null there is no action...;

n1ru4l commented 3 years ago

@damikun With the right data structures iterating over the entire store could be avoided. In case there is null you trigger a runtime exception if an edge is not expected to be null. Not automatically removing it enforces components that use mutations to be aware of the other components that are rendered on the screen and introduce a tight-coupling which the relay framework approach actually discourages doing.

stale[bot] commented 2 years ago

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.

theseyi commented 1 year ago

Seems there's another merged PR that somewhat addresses this issue? https://github.com/facebook/relay/pull/3177 From connections at least

JCMais commented 3 months ago

Looks like this bug is still present, and it is super counter-intuitive and hard to debug.