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 Patterns: Single vs Double Argument #23

Open
SachaG opened this issue Nov 3, 2019 · 7 comments
Open

Mutation Patterns: Single vs Double Argument #23

SachaG opened this issue Nov 3, 2019 · 7 comments

Comments

@SachaG
Copy link

SachaG commented Nov 3, 2019

I know most guides recommend a "single input argument" pattern for mutations, but I wanted to get feedback on this alternate pattern. Here's what I mean.

Single Argument

The single argument makes sense for update mutations where input actually contains multiple nested arguments:

mutation updateMovie {  
  updateMovie(input: { filter: { title: "Die Hard" } }, data: { year: 1989 } }) {
    result {
       ...SomeFragment
    }
  }
}

But for create mutations for example that extra level of nesting doesn't seem useful?

mutation createMovie {  
  createMovie(input: { data: { title: "Die Hard", year: 1989 } }) {
    result {
       ...SomeFragment
    }
  }
}

Double Argument

Maybe for that reason, Hasura for example uses two arguments, where and _set, and doesn't nest them inside an input:

mutation update_article {
  update_article(
    where: {id: {_eq: 3}},
    _set: {
      title: "lorem ipsum",
      content: "dolor sit amet",
      rating: 2
    }
  ) {
    affected_rows
    returning {
      ...
    }
  }
}

Elaborating on this, I would like to suggest that a double argument pattern can pretty much work for any of the basic CRUD queries and mutations. The two arguments being:

  • selector: { filter: { ... }, sort: { ... } }: an argument used to select data.
  • data (a.k.a. movie, patch, etc.): contains any data that will be mutated.

Unlike the Hasura example where where is a top-level argument, selector would itself contain filter (a.k.a. where), sort (a.k.a. orderBy), and so on. The idea is to use the same selector input type anywhere you need to select data, whether it's for a query or mutation.

Which gives us:

  • movie(selector)
  • movies(selector)
  • createMovie(data)
  • updateMovie(selector, data)
  • deleteMovie(selector)

(Note: to that I would even add a third options argument that control how the server should behave (e.g. caching behavior). But that's another matter.)

Basically, I get why a single argument is better than n arguments (as explained here) but I think two arguments is actually better than both of these approaches, since selecting vs mutating data seems to be a good natural distinction that lends itself to this 2-argument approach.

EDIT: oh and the selector and data names are just placeholders, maybe there are better ones.

@SachaG
Copy link
Author

SachaG commented Nov 5, 2019

After thinking about it maybe target would be a good name for the first argument since it lets you "target" what data you want to query or mutate. Also unlike selector (used in MongoDB) or select (used in MySQL) I'm not aware of any previous associations with database concepts.

@RIP21
Copy link
Member

RIP21 commented Nov 6, 2019

Interesting idea, but IMO only for very very flexible and SQL like experience. Two arguments pattern IMO doesn't give much of difference. I'm not lazy to nest things :) So I would rather say it's matter of taste.

For instance, if you use a selector argument that has loads of filters, you probably expect it to behave as SQL, which can change many entries using the UPDATE clause. The same applies to DELETE.

I would suggest in this case having a structure like

movie {
  bulkCreate(input: { data: [Movie!]! })
  # behaves like SQL UPDATE
  bulkUpdate(selector, data: MovieUpdatePayload!)
  # behaves like SQL DELETE
  bulkDelete(selector)
  # behaves more like RESTish `POST`
  create(input: { data:  MovieCreatePayload!})
    # behaves more like RESTish `PUT`
  update(id: String!, input: { data: MovieUpdatePayload! }) # or just update(input: { id: String!, data:MovieCreatePayload!})
  # etc...
}

About the third argument e.g. options I think you can use a directive @cache or something similar for such concerns as caching etc. to not mix it up with querying part of things.
E.g. directives for metadata and some other stuff, arguments to change query/mutation results in some way :)

Anyway, I really like single input for its ability to extend almost endlessly with no breaking or minimal breaking changes (which can lead to bad design, because too much freedom is bad, but still)

I would say that for something super flexible, an approach that you suggest is ok. But for more of a handcrafted and limited to some constrains, single input not complex and super stupid solution that just works.

@SachaG
Copy link
Author

SachaG commented Nov 7, 2019

Thanks for the feedback! Maybe I'll stick to single argument then.

About the third argument e.g. options I think you can use a directive @cache or something similar for such concerns as caching etc. to not mix it up with querying part of things.

Would you have an example of this? Or maybe a link to some resources? This could actually be a good section for the project, since I expect this is a pretty common need.

@RIP21
Copy link
Member

RIP21 commented Nov 7, 2019

@nodkz hey man. Do you have any examples of custom directives with Apollo Server or similar, because I never wrote any myself.

@nodkz
Copy link
Member

nodkz commented Nov 13, 2019

As an example in rule mutation-input-arg I provide the following code:

# Good:
mutation ($input: UpdatePostInput!) {
  updatePost(input: $input) { ... }
}

# Not so good – it's harder to write a query (duplication of variables)
mutation ($id: ID!, $newText: String, ...) {
  updatePost(id: $id, newText: $newText, ...) { ... }
}

In most cases when frontenders write mutations, they are using GraphQL-variables. And searching one type name for input variable UpdatePostInput much easier than providing several type names for variables. Also, we have duplication of variables (need to define in operation and almost the same definition made in field args when using $...) when writing queries.

BUT if we say about just two or three top-level args like suggested @SachaG I don't see any big problems too. We are providing an opportunity to simplify eg update operation where we need to provide just selector and data. For API newcomers it will be much easier to understand what they should provide.

I think that we should create a new mutation-double-arg rule. And make mutation-input-arg and mutation-double-arg rules mutually exclusive (make cross-links to each other with some notes). And provide users the ability to choose their way. For making more obvious the choice for users I need to create backend server which will:

  • gather Likes Dislikes for any rule (for authorized users via GitHub)
  • allow providing logo & company name who are using and recommending this rule
  • allow discussing rules via comments (or maybe better to discuss rules via GitHub's issues).

@SachaG you may prepare mutation-double-arg rule and I somehow add them to the site menu. But we should strongly recommend to not create many args mutations due to so ugly query definition on the client-side:

mutation ($id: ID!, $name: String, $lastname: String, dob: Date, gender: GenderEnum, city: String, ...) {
  updatePost(id: $id, name: $name, lastname: $lastname, dob: $dob, gender: $gender, city: $city, ...) { ... }
}

PS. Sorry guys in the delay of my answer, cause I just returned from HolyJS Conf and had no time to answer in time. I'm very sorry. But in a couple of days, I'll try to answer all your issues.

PSS. @RIP21 sorry, I don't use Apollo custom directives.

@SachaG
Copy link
Author

SachaG commented Nov 14, 2019

Well, I ended up going with the single argument pattern for Vulcan in the end, even if two arguments might be better in some aspects I wanted to stick to the most standard solution. Here's what I decided:

  • movie(input: { filter, sort, id })
  • movies(input: { filter, sort, search, skip, limit })
  • createMovie(input: { data })
  • updateMovie(input: { filter, id, data })
  • deleteMovie(input { filter, id })

createMovie is the main example where being forced to always have an input argument is a little clunky, but it's probably not that big a deal.

(btw, in case you're wondering search is a special filter we have that lets you search for a keyword across all searchable fields without having to specify all of them individually in the query)

@nodkz
Copy link
Member

nodkz commented Nov 14, 2019

@SachaG Not too fast ;) Check the rule https://graphql-rules.com/rules/list-filter for lists:

- movie(input: { filter, sort, id })
+ movie(filter, sort, id)
- movies(input: { filter, sort, search, skip, limit })
+ movies(filter, sort, search, skip, limit)

Even we check existing other Spec eg Relay connection spec, then they are using args on top-level.

For now Queries have top-level args, Mutations have input args. 🤯 That's why I said that "Double Argument" is not a bad solution for Mutations. And maybe better for API unification.

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

No branches or pull requests

3 participants