Recently there have been some questions regarding mutations and how engineers should handle GraphQL mutations.
The broad questions:
Should mutations be fine-grained ("update issue weight") or coarse-grained ("update issue")
Should we set a standard for mutation granularity?
Our service-oriented architecture is such that most mutations that target a similar object call the same kind of service (in the example above, UpdateIssueService). So it's a matter of figuring out which we prefer.
I think it might be worth discussing the pros vs cons (on both frontend and backend) of both approaches.
Let's imagine I want to upload a design. I would need to update a designCollection/designs property of the issue so I imagine we would have something like
This still seems to be doable, but what if I want to update comments on one of these designs? Should I change them and pass an updated list? For coarse-grained mutations, we would need to have more extensive documentation than just GraphiQL because of these unclear points - and this adds a cognitive load to all GraphQL clients. Any GraphQL consumer would need to guess what actions are possible with this mutation rather than checking a granular mutation with a descriptive name.
Also, all fields of the input here must be optional. By removing any nullability constraints on the input fields, we defer all this validation to the runtime, instead of using the schema to guide the API user towards proper usage.
Fine-grained is very typical of GraphQL mutations. They're designed around the client's needs. I think, that said, having an updateIssue mutation that updates common things of an issue and also fine-grained mutations are probably also fine for the same reason. Looking at GitHub and Shopify I can see examples of both fine-grained and more resourceful-like mutations. I think it's probably client-oriented and reasonably pragmatically designed.
I agree that simple properties should be updated in the same mutation. Since we are using the same service to update those i don't think there is any performance advantage or maintainability improvement of keeping them on separate mutations.
We do have a recent example where a fine-grained mutation was introduced because it could add more complexity to the "default" one: !38081 (merged).
Maybe we could set one rule: If updating the property only needs one argument we keep it on the coarse-grained otherwise we introduce a fine-grained?
I'm conflicted here - fine grained mutations make a lot of sense from an API design point of view, and provide much better type safety (given the weak guarantees of the GraphQL system).
The massive downside is the codebloat on the backend (essentially we have to write and test a whole mutation class for each property of each class), which ends up in hundreds or even thousands of potential mutations. We could generate such classes, but at the moment these are all hand-written.
An equally important concern is performance - why pay for 3 round-trips, and SQL updates, and authorizations, etc. if you want to update three fields, when they could be batched?
Overall, I would want to keep or even encourage coarse-grained mutations, perhaps with better tooling to deal with parameters, even if we add fine-grained mutations for common cases.
I think there needs to be a balance. An API is a language, and sometimes you want it verbose, and sometimes you want it concise. To me, updating the status of a todo item is better as markTodoDone rather than updateTodo(done: true). But I also think having a separate setter for each attribute is overkill as well.
This is basically my thoughts too, complete with vagueness.
I was trying to reflect on what, for me, feels desirable in terms of fine-grained mutations, and what seemed less-fine. I agree that as @alexkalderimis says #233063 (comment 389888618), if we were to mandate that every property needed its own fine-grained mutation, that would feel silly. However, sometimes, these finer-gainer mutations feel great alongside the bulk update mutations.
Some existing finer-grained mutations that, to me, feel good:
markTodoDone
setMergeRequestWip
I'm not sure how to be specific about why these seem fine to me. Perhaps it's to do with mutations that feel like they're setting state vs properties ('though, behind the scenes, all states are properties).
In future, I feel like a designManagementDesignApprove mutation would feel good (although, I'm a fan of additionally allowing these stateful properties to be set through the bulk mutations too, so also supporting designManagementDesignUpdate(approved: true)). So perhaps for me, that's one distinction, mutations that set state-like properties, maybe?
I started an MR based on some of the above, but it's mostly to get the conversation started, not that I'm deciding anything Please have a look and comment -> !38558 (merged)