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

Give mutation its own section and nested mutations #634

Merged
merged 1 commit into from
Mar 30, 2017
Merged

Give mutation its own section and nested mutations #634

merged 1 commit into from
Mar 30, 2017

Conversation

emilebosch
Copy link
Contributor

Ok sorry for the last one, that was messy and a bit too hasty.
Here a second attempt to get these docs in order.

In this PR i propose:

  • Give mutations its own spot in schema
  • Add nested mutations syntax for embedding

end
```

## Nested mutations
Copy link
Contributor

@xuorig xuorig Mar 23, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe have better wording for this since this seems to imply Nested Mutation in the sense of (not in spec):

myMutation(input: {...}) {
  myNestedMutation(input: {...}) {
     ...
  }
}

and not nested inputs?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I was confused by this too. I thought that "nested mutations" referred to this:

graphql/graphql-spec#252

(Which is the same as the example posted above.)

I would have described this as "mutations with complex inputs". However, I may be wrong to think that since at least the Graph.cool folks have called this "nested mutations": https://blog.graph.cool/improving-dx-with-nested-mutations-af7711113d6#.syi1rk3o8

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Graph.cool folks have called this "nested mutations"

Interesting 🤔 im not convinced of the wording

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Me either, but it wouldn't be the first time that my perspective as a graphql implementer was off track of the rest of the community 😖

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that they are not nested mutations but embedded inputs or complex input. Please let me know so i can adjust accordingly!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The use case would be that whenever creating items in bulk this really helps.
An added benefit of this is that it allows you to to easily wrap it in an transaction, when creating multiple records, making a mutation transaction safe. Which is for what i was using it in the first place.

Is that something that we should maybe mention? I.e. with ruby sample code? I know graphql is db agnostic, but still graphql/activerecord is a common use case.

@emilebosch
Copy link
Contributor Author

Fixed the naming to nested input types :)


## Nested input types

You can also nest input types. Let take this a todo list as an example.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy suggestion:

Let's create a mutation that adds multiple todo items at once

end
```

## Nested input types
Copy link
Contributor

@Willianvdv Willianvdv Mar 27, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you considered making a nested mutation input type that creates multiple post items? The example above already defines the type so it might be easier for the reader to continue with this example set.

@rmosolgo
Copy link
Owner

Sorry for the slow response on this one, thanks, it looks great!

@rmosolgo rmosolgo merged commit b019f26 into rmosolgo:master Mar 30, 2017
@rmosolgo
Copy link
Owner

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants