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

Create own context for mutation response resolvers #1308

Closed
mxstbr opened this issue Apr 7, 2018 · 4 comments
Closed

Create own context for mutation response resolvers #1308

mxstbr opened this issue Apr 7, 2018 · 4 comments

Comments

@mxstbr
Copy link

mxstbr commented Apr 7, 2018

Imagine a mutation top edit a users username, which has a signature something like this:

type Mutation {
  changeUsername(username: String!): User
}

mutation changeUsername($username: String!) {
  changeUsername(username: $username {
    username
  }
}

In said mutation resolver, I now have to load the user for whatever reason. (in my real use case for checking permissions) I'm using DataLoader to do so to avoid repeated database calls:

const RootMutation = {
  changeUsername: (root, args, context) => {
    const user = context.loaders.user.load(args.input.id);
    // ...actual username changing code...
  }
}

In my query resolvers I also use the same loader to resolve a user object:

const RootQuery = {
  user: (root, args, context) => {
    return context.loaders.user.load(root.id || args.id)
  }
}

The issue I'm running into is that the query resolver for the single user returns the cached data from the dataloader without the new changes because the context is shared between the mutation resolver and the returned query resolvers:

this.props.changeUsername('new-username').then(result => {
  console.log(result) // { username: 'old-username' }
})

Now, in this specific case I can work around this by manually clearing the DataLoader cache with context.loaders.user.clear(id). I'm wondering though, maybe it'd make sense to create the context anew for the resolvers of a mutations return value vs. for the mutation resolver itself?

@leebyron
Copy link
Contributor

Happy to entertain suggestions on how to do this, however I think the expectation is that since mutations definitionally are side-effectful on global state that doing things like clearing caches is totally within the expectations of the kind of work you would do in a mutation resolver

@Pruxis
Copy link

Pruxis commented Dec 3, 2018

This is an issue I am running into aswell, I thought I was misreading the documentation when exploring this but it actuality seems to not be possible to update the context during a query / mutation.

@IvanGoncharov
Copy link
Member

I thought I was misreading the documentation when exploring this but it actuality seems to not be possible to update the context during a query / mutation.

You can change things inside context but not a context itself.
Moreover, by putting DataLoader into the context you already mutating it every time you update its internal cache.
Nothing prevents you from fully replacing loaders property if you wish so.

In general, I think such issues should be solved by 3rd-party middlewares proposed in #1516

@IvanGoncharov
Copy link
Member

After reading through #354, I think we need some way to modify the context for subsequent resolvers without affecting resolvers executed in parallel or next mutation.
But I don't think it should be mutation specific so we should focus on #354 instead

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

5 participants
@leebyron @Pruxis @mxstbr @IvanGoncharov and others