Skip to content
This repository has been archived by the owner on Sep 3, 2021. It is now read-only.

@publish & @subscribe directives, nested @cypher mutation revision #580

Merged
merged 32 commits into from
Mar 7, 2021

Conversation

michaeldgraham
Copy link
Collaborator

This PR implements a @publish directive for Mutation fields and a @subscribe directive for Subscription fields.

A revision of the implementation for using @cypher directives on input object mutation arguments is also included.

Docs coming soon~

exports pubsub to the apollo-server and apollo-server-express example servers to be used by augmentation config and custom subscription resolvers
also trying to fix dependency merge conflict
src/augment/augment.js Outdated Show resolved Hide resolved
@johnymontana
Copy link
Contributor

Thanks so much @michaeldgraham for you work on this! I'm excited to see how users take advantage of this!

For users there's an important aspect of this implementation to be aware of. Since the result of the mutation is treated as the event payload for the subscription an important assumption then is that the selection set of the subscription is a subset of the corresponding mutation's selection set.

So for example given a non-nullable field movieId

type Movie {
  movieId: ID!
  title: String
  plot: String
}

If that movieId field is included in the subscription selection set

subscription subToCreateMovie {
  CreateMovie {
    title
    movieId
    plot
  }
}

but not in the mutation selection set

mutation {
  CreateMovie(movieId:"1234567890", title:"Foobar") {
    title
  }
}

then the client subscribed to the CreateMovie subscription will receive an error

 "errors": [
    {
      "message": "Resolver for \"Movie.movieId\" returned undefined",

The subscribing client may not be able to control the fields requested in the mutation selection set in all cases so this is something for users to be aware of. Having said that I think this PR still adds a lot of value and is something we can add as an experimental feature (tieing in to the authorization directives is another aspect to consider, but I think that's beyond the scope of this PR).

@michaeldgraham - do you want to send a docs PR with a minimal example?

@johnymontana johnymontana merged commit 36a754a into neo4j-graphql:master Mar 7, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants