From 7a8f4b1bdac5d218b09abf71771cb5de34399135 Mon Sep 17 00:00:00 2001 From: Jake Archibald Date: Wed, 9 Aug 2023 12:56:04 +0000 Subject: [PATCH 1/6] Rule for select vs update args in update mutations --- TUTORIAL.md | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/TUTORIAL.md b/TUTORIAL.md index b0c8071..83dc2a8 100644 --- a/TUTORIAL.md +++ b/TUTORIAL.md @@ -951,6 +951,18 @@ input CollectionInput { *Rule #21: Structure mutation inputs to reduce duplication, even if this requires relaxing requiredness constraints on certain fields.* +As in the example above, `collectionUpdate` takes two arguments. The first +selects the collection to update, and the second provides the update data. An +alternative to this would be one argument, `CollectionInput!`, which would +include the ID. In fact, [some older Shopify APIs follow this +pattern](https://shopify.dev/docs/api/admin-graphql/2023-07/mutations/productUpdate). +However, this is no longer recommended, as it makes it difficult to determine +which parts of the call relate to 'select', and which relate to 'update'. + +*Rule #22: For update mutations, the parameter relating to selecting the entry +must be separate to the parameter providing the change data. The parameter +relating to selecting the entry must come first.* + ### Output The final design question we need to deal with is the return value of our @@ -981,7 +993,7 @@ would return the newly-created collection for the `collection` field. An unsuccessful mutation would return one or more `UserError` objects, and `null` for the collection. -*Rule #22: Mutations should provide user/business-level errors via a +*Rule #23: Mutations should provide user/business-level errors via a `userErrors` field on the mutation payload. The top-level query errors entry is reserved for client and server-level errors.* @@ -1001,7 +1013,7 @@ It's worth noting that `collection` is still nullable even here, since if the provided ID doesn't represent a valid collection, there is no collection to return. -*Rule #23: Most payload fields for a mutation should be nullable, unless there +*Rule #24: Most payload fields for a mutation should be nullable, unless there is really a value to return in every possible error case.* ## TLDR: The rules @@ -1028,8 +1040,9 @@ return. - Rule #19: Use weaker types for inputs (e.g. String instead of Email) when the format is unambiguous and client-side validation is complex. This lets the server run all non-trivial validations at once and return the errors in a single place in a single format, simplifying the client. - Rule #20: Use stronger types for inputs (e.g. DateTime instead of String) when the format may be ambiguous and client-side validation is simple. This provides clarity and encourages clients to use stricter input controls (e.g. a date-picker widget instead of a free-text field). - Rule #21: Structure mutation inputs to reduce duplication, even if this requires relaxing requiredness constraints on certain fields. -- Rule #22: Mutations should provide user/business-level errors via a userErrors field on the mutation payload. The top-level query errors entry is reserved for client and server-level errors. -- Rule #23: Most payload fields for a mutation should be nullable, unless there is really a value to return in every possible error case. +- Rule #22: For update mutations, the parameter relating to selecting the entry must be separate to the parameter providing the change data. The parameter relating to selecting the entry must come first. +- Rule #23: Mutations should provide user/business-level errors via a userErrors field on the mutation payload. The top-level query errors entry is reserved for client and server-level errors. +- Rule #24: Most payload fields for a mutation should be nullable, unless there is really a value to return in every possible error case. ## Conclusion From 91e0ca9a3a8f61230aeb76d85f8453f14b122da7 Mon Sep 17 00:00:00 2001 From: Jake Archibald Date: Wed, 9 Aug 2023 13:39:35 +0000 Subject: [PATCH 2/6] Remove reference to parameter ordering --- TUTORIAL.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/TUTORIAL.md b/TUTORIAL.md index 83dc2a8..92169d0 100644 --- a/TUTORIAL.md +++ b/TUTORIAL.md @@ -960,8 +960,7 @@ However, this is no longer recommended, as it makes it difficult to determine which parts of the call relate to 'select', and which relate to 'update'. *Rule #22: For update mutations, the parameter relating to selecting the entry -must be separate to the parameter providing the change data. The parameter -relating to selecting the entry must come first.* +must be separate to the parameter providing the change data.* ### Output @@ -1040,7 +1039,7 @@ return. - Rule #19: Use weaker types for inputs (e.g. String instead of Email) when the format is unambiguous and client-side validation is complex. This lets the server run all non-trivial validations at once and return the errors in a single place in a single format, simplifying the client. - Rule #20: Use stronger types for inputs (e.g. DateTime instead of String) when the format may be ambiguous and client-side validation is simple. This provides clarity and encourages clients to use stricter input controls (e.g. a date-picker widget instead of a free-text field). - Rule #21: Structure mutation inputs to reduce duplication, even if this requires relaxing requiredness constraints on certain fields. -- Rule #22: For update mutations, the parameter relating to selecting the entry must be separate to the parameter providing the change data. The parameter relating to selecting the entry must come first. +- Rule #22: For update mutations, the parameter relating to selecting the entry must be separate to the parameter providing the change data. - Rule #23: Mutations should provide user/business-level errors via a userErrors field on the mutation payload. The top-level query errors entry is reserved for client and server-level errors. - Rule #24: Most payload fields for a mutation should be nullable, unless there is really a value to return in every possible error case. From 3ef0167f99b8ef6b39c2a3a065ef139090ae2ae2 Mon Sep 17 00:00:00 2001 From: Jake Archibald Date: Wed, 9 Aug 2023 14:42:09 +0100 Subject: [PATCH 3/6] Update TUTORIAL.md Co-authored-by: Scott Walkinshaw --- TUTORIAL.md | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/TUTORIAL.md b/TUTORIAL.md index 92169d0..88c355b 100644 --- a/TUTORIAL.md +++ b/TUTORIAL.md @@ -951,13 +951,12 @@ input CollectionInput { *Rule #21: Structure mutation inputs to reduce duplication, even if this requires relaxing requiredness constraints on certain fields.* -As in the example above, `collectionUpdate` takes two arguments. The first -selects the collection to update, and the second provides the update data. An -alternative to this would be one argument, `CollectionInput!`, which would -include the ID. In fact, [some older Shopify APIs follow this -pattern](https://shopify.dev/docs/api/admin-graphql/2023-07/mutations/productUpdate). -However, this is no longer recommended, as it makes it difficult to determine -which parts of the call relate to 'select', and which relate to 'update'. +As in the example above, `collectionUpdate` takes two arguments. `collectionId` +selects the collection to update, and `collection` provides the update data. An +alternative to this would be a single merged `collection: CollectionInput!` argument, where +`CollectionInput` would have a nullable `id` input field. However, this makes it +difficult to determine which parts of the call relate to 'select', and which relate +to 'update' and is not recommended. *Rule #22: For update mutations, the parameter relating to selecting the entry must be separate to the parameter providing the change data.* From 1e0f337819c5a5f066095fe23b8bf8d214393878 Mon Sep 17 00:00:00 2001 From: Jake Archibald Date: Wed, 9 Aug 2023 13:43:40 +0000 Subject: [PATCH 4/6] Grammar and wrapping tweak --- TUTORIAL.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/TUTORIAL.md b/TUTORIAL.md index 88c355b..af46f29 100644 --- a/TUTORIAL.md +++ b/TUTORIAL.md @@ -953,10 +953,10 @@ input CollectionInput { As in the example above, `collectionUpdate` takes two arguments. `collectionId` selects the collection to update, and `collection` provides the update data. An -alternative to this would be a single merged `collection: CollectionInput!` argument, where -`CollectionInput` would have a nullable `id` input field. However, this makes it -difficult to determine which parts of the call relate to 'select', and which relate -to 'update' and is not recommended. +alternative to this would be a single merged `collection: CollectionInput!` +argument, where `CollectionInput` would have a nullable `id` input field. +However, this makes it difficult to determine which parts of the call relate to +'select', and which relate to 'update', and is therefore not recommended. *Rule #22: For update mutations, the parameter relating to selecting the entry must be separate to the parameter providing the change data.* From 0144385bec132a8cbe270756bdb218a80e76a153 Mon Sep 17 00:00:00 2001 From: Jake Archibald Date: Wed, 9 Aug 2023 14:09:33 +0000 Subject: [PATCH 5/6] 'parameter' -> 'argument' --- TUTORIAL.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/TUTORIAL.md b/TUTORIAL.md index af46f29..4dee0ba 100644 --- a/TUTORIAL.md +++ b/TUTORIAL.md @@ -958,8 +958,8 @@ argument, where `CollectionInput` would have a nullable `id` input field. However, this makes it difficult to determine which parts of the call relate to 'select', and which relate to 'update', and is therefore not recommended. -*Rule #22: For update mutations, the parameter relating to selecting the entry -must be separate to the parameter providing the change data.* +*Rule #22: For update mutations, the argument relating to selecting the entry +must be separate to the argument providing the change data.* ### Output @@ -1038,7 +1038,7 @@ return. - Rule #19: Use weaker types for inputs (e.g. String instead of Email) when the format is unambiguous and client-side validation is complex. This lets the server run all non-trivial validations at once and return the errors in a single place in a single format, simplifying the client. - Rule #20: Use stronger types for inputs (e.g. DateTime instead of String) when the format may be ambiguous and client-side validation is simple. This provides clarity and encourages clients to use stricter input controls (e.g. a date-picker widget instead of a free-text field). - Rule #21: Structure mutation inputs to reduce duplication, even if this requires relaxing requiredness constraints on certain fields. -- Rule #22: For update mutations, the parameter relating to selecting the entry must be separate to the parameter providing the change data. +- Rule #22: For update mutations, the argument relating to selecting the entry must be separate to the argument providing the change data. - Rule #23: Mutations should provide user/business-level errors via a userErrors field on the mutation payload. The top-level query errors entry is reserved for client and server-level errors. - Rule #24: Most payload fields for a mutation should be nullable, unless there is really a value to return in every possible error case. From 874e5a255f90727eede9d4af5a06873b71ec7cc9 Mon Sep 17 00:00:00 2001 From: Jake Archibald Date: Thu, 10 Aug 2023 07:54:17 +0000 Subject: [PATCH 6/6] Add note about non-nullable update args --- TUTORIAL.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/TUTORIAL.md b/TUTORIAL.md index 4dee0ba..792d9ae 100644 --- a/TUTORIAL.md +++ b/TUTORIAL.md @@ -959,7 +959,9 @@ However, this makes it difficult to determine which parts of the call relate to 'select', and which relate to 'update', and is therefore not recommended. *Rule #22: For update mutations, the argument relating to selecting the entry -must be separate to the argument providing the change data.* +must be separate to the argument providing the change data. The argument to +select the entry should be non-nullable unless there's value in making the +condition an optional filter.* ### Output @@ -1038,7 +1040,7 @@ return. - Rule #19: Use weaker types for inputs (e.g. String instead of Email) when the format is unambiguous and client-side validation is complex. This lets the server run all non-trivial validations at once and return the errors in a single place in a single format, simplifying the client. - Rule #20: Use stronger types for inputs (e.g. DateTime instead of String) when the format may be ambiguous and client-side validation is simple. This provides clarity and encourages clients to use stricter input controls (e.g. a date-picker widget instead of a free-text field). - Rule #21: Structure mutation inputs to reduce duplication, even if this requires relaxing requiredness constraints on certain fields. -- Rule #22: For update mutations, the argument relating to selecting the entry must be separate to the argument providing the change data. +- Rule #22: For update mutations, the argument relating to selecting the entry must be separate to the argument providing the change data. The argument to select the entry should be non-nullable unless there's value in making the condition an optional filter. - Rule #23: Mutations should provide user/business-level errors via a userErrors field on the mutation payload. The top-level query errors entry is reserved for client and server-level errors. - Rule #24: Most payload fields for a mutation should be nullable, unless there is really a value to return in every possible error case.