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

How to avoid Dup Records when executing Create Mutation Commits that Insert New Edges into Connections #1734

Closed
kguller opened this issue May 8, 2017 · 5 comments

Comments

@kguller
Copy link

kguller commented May 8, 2017

Having trouble implementing a simple Document Component that adds a document and updates a connection relation with Application.documents edges. The Create works fine, but the Updater keeps adding the LinkedRecord reference to the same Edge every time I run InsertEdgeAfter on the Application.documents Connection. This causes duplicate records since it keeps updating the old Edge records by reference. I'm hoping I don't have to destroy the Component to create a separate reference for the Create Edge LinkedRecord. How do we add multiple edges to Connections if we always receive the same referenced Edge?

Here is the mutation to create a document, it returns an associated Application.id as well as the expected edge.

const mutation = graphql
  mutation CreateDocumentMutation($input: CreateDocumentInput!) {
    createDocument(input: $input) {
      document {
        id
        data
      }
      edge {
        __typename
        cursor
        node {
          id
          application{
            id
          }
          data
        }
      }
      application {
        id        
      }
    }
  }

Here is my updater method that gets the payload, finds the Edge, then uses the application proxy to get the Connection to insert the Edge into. The problem is the Edge is a linked record and every time I run a commit on this create it keeps updating the Edge reference and incrementing pointers to the latest Edge value.

updater: (store) => {
        const payload = store.getRootField('createDocument')
        const newEdge = payload.getLinkedRecord('edge')
        const proxy = store.get(application.id)
        const conn = ConnectionHandler.getConnection(proxy, 'application_documents')        
        ConnectionHandler.insertEdgeAfter(conn, newEdge)
      }

My connection is simply just a list of the documents for this application. After I insert each document, the list starts creating duplicates simply because the references all point to the last Edge created. The GraphQL data storage is fine, but my local update is failing due to this issue.

Application.documents = [
11111, <- created in an earlier session
2222, <- created in an earlier session
12345, <- created in the current session
12345, <- created in the current session
12345, <- created in the current session, keeps repeating as I run the commit trigger
]

@amozoss
Copy link

amozoss commented May 30, 2017

I had a similar problem and setup. I was able to resolve the issue by pulling the 'node' out of my edge, creating a new edge with ConnectionHandler.createEdge, then calling insertEdgeAfter with the newly created edge.

function commit(channelID, kind) {
  const variables = {
    input: {
      channelID,
      kind,
    }
  }

  commitMutation(
    environment,
    {
      mutation,
      variables,
      updater: (store) => {
        const payload = store.getRootField('createInvitation');
        const node = payload.getLinkedRecord('invitationEdge').getLinkedRecord('node');
        const channelProxy = store.get(channelID);
        const conn = ConnectionHandler.getConnection(
          channelProxy,
          'ChannelView_invitationConnection',
        );
        const newEdge = ConnectionHandler.createEdge(
          store,
          conn,
          node,
          "InvitationEdge"
        )
        ConnectionHandler.insertEdgeAfter(conn, newEdge);
    }
  })
}

Not entirely sure why it didn't work before, but seems to work when I create the new edge. I think the issue could be related to my variables not changing between mutation commits?

@josephsavona
Copy link
Contributor

Thanks for reporting this. It's a known issue where mutations with the same variables end up having the same ids for some records (those that don't have their own id, including edges). @amozoss's suggestion of using createEdge() is a solid workaround.

@wieseljonas
Copy link

@josephsavona I this need to create my own updater and cannot relay on RANGE_ADD for the moment?

@gpbaculio
Copy link

Hey, I solved this by removing RANGE_ADD on configs, it's either you choose from updater or RANGE_ADD

@sibelius
Copy link
Contributor

use updater or config

tks

facebook-github-bot pushed a commit that referenced this issue Dec 10, 2019
…or each executed mutation (#2349)

Summary:
Fixes #1734
Fixes #2333

Related to relayjs/relay-examples#30
When a mutation is executed, it's payload is added to the store using a key of the serialized arguments. In the Todo Modern example this looks like this:

```js
{
  'client:root:addTodo(input:{"text":"a"})': { ... } // added by mutation response
  'client:VXNlcjptZQ==:__TodoList_todos_connection': {
    edges: [
      __refs: [
        'client:VXNlcjptZQ==:__TodoList_todos_connection:edges:0',
        'client:VXNlcjptZQ==:__TodoList_todos_connection:edges:1',
        'client:root:addTodo(input:{"text":"a"}):todoEdge', // added by mutation imperative updater function
      ]
    ]
  }
}
```

If you execute a second `addTodo` mutation with the same arguments, the `client:root:addTodo(input:{"text":"a"})` field in the store will get overwritten with the second mutation and the store will look like this:

```js
{
  'client:root:addTodo(input:{"text":"a"})': { ... }
  'client:VXNlcjptZQ==:__TodoList_todos_connection': {
    edges: [
      __refs: [
        'client:VXNlcjptZQ==:__TodoList_todos_connection:edges:0',
        'client:VXNlcjptZQ==:__TodoList_todos_connection:edges:1',
        'client:root:addTodo(input:{"text":"a"}):todoEdge',
        'client:root:addTodo(input:{"text":"a"}):todoEdge',
      ]
    ]
  }
}
```

When React renders the data, you will have the latter `todo` in the connection array twice and will likely get an error along the lines of `Encountered two children with the same key, <id>. Child keys must be unique; when two children share a key, only the first child will be used`.

This was [fixed in relay-examples](relayjs/relay-examples@ed95594#diff-4b42725e6eb503f56da8e3f637e9a4e3) by adding a `clientMutationId` to the mutation arguments. I don't think this is ideal since
1. it requires the developer to manually add this to each mutation
2. it places an additional constraint on the GraphQL schema to support this field.

The approach I took to fix this, was to use a new root `dataID` (instead of the static `client:root`) for results on each mutation. This will namespace results from each mutation so they can never overwrite one another.

With these changes, the relay store now looks like this after completing the same series of mutations:

```js
{
  'client:mutationRoot0:addTodo(input:{"text":"a"})': { ... },
  'client:mutationRoot1:addTodo(input:{"text":"a"})': { ... },
  'client:VXNlcjptZQ==:__TodoList_todos_connection': {
    edges: [
      __refs: [
        'client:VXNlcjptZQ==:__TodoList_todos_connection:edges:0',
        'client:VXNlcjptZQ==:__TodoList_todos_connection:edges:1',
        'client:mutationRoot0:addTodo(input:{"text":"a"}):todoEdge',
        'client:mutationRoot1:addTodo(input:{"text":"a"}):todoEdge'
      ]
    ]
  }
}
```

I tested this and it works to solve this issue, but I'm unsure if there could be any unintended consequences by this change.
Closes #2349

Reviewed By: josephsavona

Differential Revision: D7631345

Pulled By: tyao1

fbshipit-source-id: fc53638536860855f271f436f810caa0fc60bcb0
jstejada pushed a commit that referenced this issue Dec 19, 2019
…or each executed mutation (#2349)

Summary:
Fixes #1734
Fixes #2333

Related to relayjs/relay-examples#30
When a mutation is executed, it's payload is added to the store using a key of the serialized arguments. In the Todo Modern example this looks like this:

```js
{
  'client:root:addTodo(input:{"text":"a"})': { ... } // added by mutation response
  'client:VXNlcjptZQ==:__TodoList_todos_connection': {
    edges: [
      __refs: [
        'client:VXNlcjptZQ==:__TodoList_todos_connection:edges:0',
        'client:VXNlcjptZQ==:__TodoList_todos_connection:edges:1',
        'client:root:addTodo(input:{"text":"a"}):todoEdge', // added by mutation imperative updater function
      ]
    ]
  }
}
```

If you execute a second `addTodo` mutation with the same arguments, the `client:root:addTodo(input:{"text":"a"})` field in the store will get overwritten with the second mutation and the store will look like this:

```js
{
  'client:root:addTodo(input:{"text":"a"})': { ... }
  'client:VXNlcjptZQ==:__TodoList_todos_connection': {
    edges: [
      __refs: [
        'client:VXNlcjptZQ==:__TodoList_todos_connection:edges:0',
        'client:VXNlcjptZQ==:__TodoList_todos_connection:edges:1',
        'client:root:addTodo(input:{"text":"a"}):todoEdge',
        'client:root:addTodo(input:{"text":"a"}):todoEdge',
      ]
    ]
  }
}
```

When React renders the data, you will have the latter `todo` in the connection array twice and will likely get an error along the lines of `Encountered two children with the same key, <id>. Child keys must be unique; when two children share a key, only the first child will be used`.

This was [fixed in relay-examples](relayjs/relay-examples@ed95594#diff-4b42725e6eb503f56da8e3f637e9a4e3) by adding a `clientMutationId` to the mutation arguments. I don't think this is ideal since
1. it requires the developer to manually add this to each mutation
2. it places an additional constraint on the GraphQL schema to support this field.

The approach I took to fix this, was to use a new root `dataID` (instead of the static `client:root`) for results on each mutation. This will namespace results from each mutation so they can never overwrite one another.

With these changes, the relay store now looks like this after completing the same series of mutations:

```js
{
  'client:mutationRoot0:addTodo(input:{"text":"a"})': { ... },
  'client:mutationRoot1:addTodo(input:{"text":"a"})': { ... },
  'client:VXNlcjptZQ==:__TodoList_todos_connection': {
    edges: [
      __refs: [
        'client:VXNlcjptZQ==:__TodoList_todos_connection:edges:0',
        'client:VXNlcjptZQ==:__TodoList_todos_connection:edges:1',
        'client:mutationRoot0:addTodo(input:{"text":"a"}):todoEdge',
        'client:mutationRoot1:addTodo(input:{"text":"a"}):todoEdge'
      ]
    ]
  }
}
```

I tested this and it works to solve this issue, but I'm unsure if there could be any unintended consequences by this change.
Closes #2349

Reviewed By: josephsavona

Differential Revision: D7631345

Pulled By: tyao1

fbshipit-source-id: fc53638536860855f271f436f810caa0fc60bcb0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants