msolomon/argo

Typer should mark omittable fields for variable `@include` and `@skip`

Closed this issue · 1 comments

TL;DR: Using @include(if: $var) or @skip(if: $var) should result in a field being marked as "omittable".

For example, a minimal query like this:

query($v:Boolean!){__typename@include(if:$v)}

Currently, the argo-js typer (and the Argo 1.1 spec) result in a WireType that looks like this:

{
  data: {
    __typename: STRING<String>
  }?
  errors?: DESC[]?
}

However, since the value of $v is indeterminate when the WireType is derived, it's expected that the __typename field would be marked as omittable like this:

{
  data: {
    __typename?: STRING<String>
  }?
  errors?: DESC[]?
}

This applies for both the @include and @skip directives on selections for Field, FragmentSread, and InlineFragment as shown below.

Service Document

schema {
  query: Query
}

interface Child {
  optional: Child
  required: Child!
}

type Object implements Child {
  optional: Child
  required: Child!
}

type Query {
  root: Child
}

Executable Document

query FieldOmittable($include: Boolean = false, $skip: Boolean = true) {
  root {
    includeAlways: __typename @include(if: true)
    includeNever: __typename @include(if: false)
    includeVariable: __typename @include(if: $include)
    skipAlways: __typename @skip(if: true)
    skipNever: __typename @skip(if: false)
    skipVariable: __typename @skip(if: $skip)
  }
}

Current WireType

{
  data: {
    root: {
      includeAlways: STRING<String>
      includeVariable: STRING<String>
      skipNever: STRING<String>
      skipVariable: STRING<String>
    }?
  }?
  errors?: DESC[]?
}

Expected WireType

{
  data: {
    root: {
      includeAlways: STRING<String>
      includeVariable?: STRING<String>
      skipNever: STRING<String>
      skipVariable?: STRING<String>
    }?
  }?
  errors?: DESC[]?
}
--- current
+++ expected
@@ -2,9 +2,9 @@
   data: {
     root: {
       includeAlways: STRING<String>
-      includeVariable: STRING<String>
+      includeVariable?: STRING<String>
       skipNever: STRING<String>
-      skipVariable: STRING<String>
+      skipVariable?: STRING<String>
     }?
   }?
   errors?: DESC[]?

Examples for FragmentSpread and InlineFragment can be seen in this file used in the test suite for erlang-argo.

Thanks for finding this issue and the detailed report! The spec and reference implementation are now updated to fix it.