Relay: optimisticUpdater does not work

[steida]

So, I have this code.

// @flow
import { graphql, commitMutation } from 'react-relay';
import type { Commit } from '../components/Mutation';
import type {
  DeleteWebMutationVariables,
  DeleteWebMutationResponse,
} from './__generated__/DeleteWebMutation.graphql';
import { ConnectionHandler } from 'relay-runtime';
import ensureConnection from './ensureConnection';

const mutation = graphql`
  mutation DeleteWebMutation($input: DeleteWebInput!) {
    deleteWeb(input: $input) {
      id
    }
  }
`;

const sharedUpdater = (store, id) => {
  const clientRoot = store.get('client:root');
  const connection = ConnectionHandler.getConnection(clientRoot, 'Webs_webs');
  ensureConnection(connection);
  ConnectionHandler.deleteNode(connection, id);
};

const commit: Commit<DeleteWebMutationVariables, DeleteWebMutationResponse> = (
  environment,
  variables,
  onCompleted,
  onError,
) =>
  commitMutation(environment, {
    mutation,
    variables,
    onCompleted,
    onError,
    updater: store => {
      const payload = store.getRootField('deleteWeb');
      const id = payload.getValue('id');
      sharedUpdater(store, id);
    },
    // This breaks everything.
    optimisticUpdater: store => {
      sharedUpdater(store, variables.input.id);
    },
  });

export default { commit };

As optimisticUpdater does not work. I don't know why. Probably because of clientMutationId, maybe.

[sibelius]

check this example https://github.com/relayjs/relay-examples/blob/master/todo/js/mutations/RemoveCompletedTodosMutation.js#L31

[steida]

@sibelius I just checked relay-examples. optimisticUpdater in RemoveCompletedTodosMutation does not work. https://github.com/relayjs/relay-examples/issues/69

[steida]

It's weird that relay-examples RemoveTodoMutation.js optimisticUpdater works, by mine not. Only the updater... 🤔

[steida]

Hmm, it's weird. console.log(webs.edges); in list render:

So it removes the item and immediately returns it back to the collection.

@jstejada Any idea why this can happen? Do Relay need a viewer? That's the only difference I have in https://github.com/este/este

[steida]

I think https://github.com/facebook/relay/issues/2343 and https://github.com/relayjs/relay-examples/issues/69 are related.

[ntkme]

The root cause is not what you are thinking. optimisticUpdater does work, but the parameter passed in was incorrect. Whoever wrote that code did not realize an obvious limitation of relay by design.

The problem is status: "completed" in the connection args. Apparently there is no magic for relay to resolve a connection with filters dynamically at frontend, so relay just gives up updating that connection once it's resolved.

That means, once the root query is resolved, this.props.viewer.completedTodos will never change. No matter how you change the state on the page, the optimistic updater is always trying to remove entries that are marked as completed when you initially open the page. Then the real updater kicks in later using the mutation on the server side to do the correct work, and overrides whatever optimistic updater has done incorrectly.

So there are two ways to fix this problem:

1. Wrap a refetch container to keep completedTodos up to date. Obviously, this approach creates lots of network overhead... I'm going to skip how to implement this.
2. Remove the filter status: "completed" from the fragment, then dynamically compute the completedTodos for RemoveCompletedTodosMutation. Please see the following:

 fragment TodoListFooter_viewer on User {
   id,
   completedCount,
-  completedTodos: todos(
-    status: "completed",
+  todos(
     first: 2147483647  # max GraphQLInt
-  ) {
+  ) @connection(key: "TodoList_todos") {
     edges {
       node {
         id

+      const edges = this.props.viewer.todos.edges.filter(edge => edge.node.complete === true)
       RemoveCompletedTodosMutation.commit(
         this.props.relay.environment,
-        this.props.viewer.completedTodos,
+        {
+          edges
+        },
         this.props.viewer
       )

[ntkme]

Oops, made a typo in previous comment. You need to give the connection in TodoList and TodoListFooter the save key, and it must be the key used in other mutations, otherwise it won't be updated.

[ntkme]

I just take another look. For the TodoMVC, the problem is sharedUpdater in all other mutations only updates the connection with key TodoList_todos.

So theoretically you can also give the connection in TodoListFooter a different key and manually update that connection accordingly. However, that approach is way much more complex than the method I showed in the previous comment, because you have to manually update two connections and pay attention to the "node.complete" when updating one of them.

[steida]

cc @jstejada

[alloy]

@ntkme Awesome. Mind sending a PR to the example repo?

[alloy]

@steida With @ntkme’s suggestion and PR to the example app can this be considered done and closed?

[steida]

@alloy Unfortunately, no. Check https://github.com/este/este/blob/2a466085dfe2a85228580f02e73dfc8e08ed4c31/components/Webs.js#L33 I don't have any filter there. It just doesn't work in a way I described.

You can check out Este and try it for your self. Just uncomment this line https://github.com/este/este/blob/02ebb6399119d01108c1c031b0afa0d18c479b89/mutations/DeleteWebMutation.js#L45.

I think it must be a bug in Relay. cc @jstejada @kassens

[SinHouse]

I have a similar issue than @steida

OptimisticUpdater remove the fifth node correctly but when relay does the rollback to continue with the normal updater the edge is restored but not his content or perhaps the second call to deleteNode remove the content but not the edge, I don' know.

Then, react call the render function with undefined data and it crashes:

I disabled the call to optimisticUpdater for now and it works:

[mrogelja]

Any update on this issue ? I have the same error than @SinHouse and I'm forced to disable the optimistic updater on node removal.

[stale[bot]]

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.