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

Mutation results #317

Closed
stubailo opened this issue Jun 27, 2016 · 10 comments
Closed

Mutation results #317

stubailo opened this issue Jun 27, 2016 · 10 comments
Assignees

Comments

@stubailo
Copy link
Contributor

There needs to be a good way to incorporate the result of a mutation into the store. You can already use the result of a mutation to do the following:

  1. Call any JavaScript code with the result in the then of the promise
  2. Dispatch any Redux action in the then of the promise
  3. Incorporate changes in fields, when the response of the mutation contains a normalizable response (like we do in GitHunt)

I think we need a few more, informed by talking to users and also by looking at the options for Relay mutation configs:

  1. Insert a new object created by the mutation into an array
  2. Delete an object removed by the store
  3. Remove the object from an array, but keep it in the store in case it shows up in other queries
  4. Some combination of one or all of the above

Lastly, there needs to be an escape hatch, which perhaps will require a lot of documentation to get right, but is important so that people don't feel like they are limited.

  1. Write a custom mutation result reducer, which lets you use stuff like writeFragmentToStore to do anything you like with the result.
@stubailo stubailo self-assigned this Jun 27, 2016
@stubailo
Copy link
Contributor Author

Inserting a new object into an array

Let's say we've got the following setup:

fragment todoFields on Todo {
  id
  text
  completed
}

query todoList {
  todoList(id: 5) {
    id
    todos {
      ...todoFields
    }
  }
}

mutation createTodo {
  createTodo(todo: {...}) {
    todo {
      ...todoFields
    }
  }
}

Essentially, we are looking at a todo list, and we want to create an item in that list. Let's say we want the item to show up at the top. To do this we need the following information:

  1. The location of the item in the mutation result
  2. The location of the list in the query

The most basic way to provide that would be:

  1. createTodo.todo
  2. 5.todos
  3. prepend

So a preliminary sketch of the API would be something like:

applyResult: [{
  type: 'ARRAY_INSERT',
  resultPath: [ 'createTodo', 'todo' ],
  storePath: [ '5', 'todos' ],
  where: 'prepend',
}]

This seems pretty good to start. Some concerns:

  1. This won't do too well if the mutation returns an array of items that all need to be inserted
  2. The store path is complicated by (1) any arguments to the fields, and (2) the pagination strategy we decide to go with

I'd say we can safely ignore (1) to start with, and ship that separately as something different later. For (2), we'll just have to see when we get there.

@stubailo
Copy link
Contributor Author

Delete an object

Let's add another mutation:

mutation deleteTodo {
  deleteTodo(todoId: 10) {
    id
  }
}

I think it is pretty reasonable that the object should have an ID if it is to be deleted. So we can just have the mutations return the ID of the deleted item. Since we might have custom dataId generation, we need all of the fields used by that method in the response.

In this case, we probably need:

id: 10
__typename: Todo

So we could, say, do the following:

applyResult: [{
  type: 'DELETE',
  dataIdToDelete: (result) => 'Todo' + result.deleteTodo.id,
}]

One concern is that we are hard-coding the typename here, and replicating the logic for getting the dataId. Perhaps we can get the dataId from the mutation argument?

deleteTodo: (dataId, id) => ({
  mutation: gql`...`,
  applyResult: [{
    type: 'DELETE',
    dataId,
  }]
})

This would imply that it's helpful to have a way to get that from the query, perhaps:

fragment todoFields on Todo {
  __apolloDataId
  id
  text
  completed
}

Where __apolloDataId would never be sent in a query to the server, and would be returned when reading from the store using the provided logic. Something to think about.

Questions about this approach:

  1. Is it valid to assume that any item to be deleted will have a dataId?
  2. Is it valid to assume that the dataId to be deleted can always be retrieved from the server, or from the argument to the mutation?

@stubailo
Copy link
Contributor Author

Remove an object from an array

This would happen if we had something that moved the todo to a different list:

moveTodo: (dataId, todoId, targetListId) => ({
  mutation: gql`...`,
  applyResult: [{
    type: 'ARRAY_REMOVE',
    dataId,
    storePath: [ '5', 'todos' ],
  }]
})

This is basically like a cross between ARRAY_INSERT and DELETE since it needs both an array to operate on, and the data ID of the item to remove.

I don't think there are any new questions over the above.

@stubailo
Copy link
Contributor Author

Writing a custom reducer

We should document well the implementation of the above methods, which are basically like mini reducers. Maybe you can add new ones via the apollo client constructor?

const client = new ApolloClient({
  ...,
  mutationResultReducers: {
    MULTI_INSERT: multiInsertReducer, 
  }
});

So this would be like:

// state is the 'data' section of the store state
// action is just the 'applyResult' array item
function multiInsertReducer(state, action, result) {
  const array = readFragmentFromStore(...);
  // add result items to array, based on action data
  return writeFragmentToStore(state, ...);
}

Seems like if we document the tools well it will be quite possible to write nice custom reducers.

@stubailo stubailo mentioned this issue Jun 27, 2016
11 tasks
@abhiaiyer91
Copy link
Contributor

abhiaiyer91 commented Jun 28, 2016

What if queries and mutations both shared these custom reducers?

Client queries -> query results hit our custom reducer maybe specified via QUERY_NAME_INIT.

Then we can also mutate the data via this custom reducer. This would actually help Optimistic UI, client state mgmt, etc.

@stubailo
Copy link
Contributor Author

Sounds pretty interesting! I'm sure the design for mutations can transfer over to that as well

@abhiaiyer91
Copy link
Contributor

The more I think about this, the more I'm contemplating a redux way of doing this.

In redux, our reducers describe our state change. So whether its a query or a mutation, a reducer is expecting some shape of data to then return a new state.

If we take this approach with Apollo the store can actually have a different shape.

Maybe it can work like this,

  1. Apollo Client ships some generic reducers. Much of the things described above. These reducers will take the response from both a query or mutation and apply those results to the reducer.
  2. Optimistic UI can be handled since our reducer is just expecting meta data, we can dispatch the expected data right away and essentially call and forget on the mutation. The only rollback we need is if we catch errors.

I guess the question then is, why do we store everything under a data object today? Do we need to do this? If these custom reducers represent the slices of state from our server, we can associate queries to them, hydrate them on the server, etc. We can also unit test these specific pieces of state in isolation?

@abhiaiyer91
Copy link
Contributor

abhiaiyer91 commented Jun 28, 2016

What do we gain by having a store path?

moveTodo: (dataId, todoId, targetListId) => ({
  mutation: gql`...`,
  applyResult: [{
    type: 'ARRAY_REMOVE',
    dataId,
    storePath: [ '5', 'todos' ],
  }]
})

Potential view from query

fetchTodos: () => ({
  query: gql`...`,
 // allTodos represent the reducer we associate the state change to
  stateKey: 'allTodos',
  applyResult: [{
    type: 'ARRAY_INIT',
  }]
})

@stubailo
Copy link
Contributor Author

@abhiaiyer91 we need store path to tell Apollo which array we are trying to remove from. The goal of this action is if you have an item that is displayed in one or more lists on the page, and you want to stop displaying it there. This is for cases where you aren't deleting the item entirely, so we can't just remove all references like we do with DELETE.

For the query example, is this better than doing client.query with a thunk action creator?

@stubailo
Copy link
Contributor Author

Merged #320

jbaxleyiii pushed a commit that referenced this issue Oct 17, 2017
adding a super simple client/server example
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants