Make args optional in readField (i.e. infer them)

[joelmukuthu]

We use client-resolved fields to compute new fields that rely on other fields that are returned by a graphql server e.g. in the following case, resolving isCompleted uses the completionState field:

query {
  todo {
    completionState(filter: 'foo') { value }
    isCompleted @client
  }
}

const todoTypePolicy = {
  Todo: {
    fields: {
      isCompleted: {
        read(_current, { readField, variables }) {
          const completionState = readField({ 
            fieldName: 'completionState', 
            args: { filter: varibles?.['filter'] } 
          });
          // the optional chaining is intentional so that `undefined` 
          // is returned when `completionState` is `undefined`:
          return completionState?.value === 100; 
        }
      }
    }
  } 
}

We need to pass the args to readField otherwise it doesn't find the data we want. This makes sense, since the completion value will be different depending on the value of filters.

The trouble arises when another argument is added to the completionState field, but is not added to the args passed to readField. In that case, isCompleted stops working as expected and instead fails silently, leading to incorrect data getting returned. There's no way to indicate that isCompleted is dependent upon completionState besides:

1. Adding a comment in the query
2. Adding an explicit check in the type policy for isCompleted and throwing or logging an error

Adding the explicit check might seem sufficient, but unfortunately it only runs at run-time so it's easy for this to slip through unnoticed.

This issue proposes updating the implementation of readField such that the default behaviour is to apply the args that the field was called with if there's only field/args pair for that field. That is, if my query only includes one instance of completionState, have readField return that value regardless of what args it was called with. If there are more than one, then fallback to the current behaviour where undefined is returned (or throw an error, print a warning etc) -- in this case the consumer would need to pass args so they're explicit about what they want. I'm guessing that args are required because there might be multiple instances of the same field with different args -- though I'm not sure about this, nor if graphql allows doing that without adding aliases for the different instances.

So for the query above, the type policy would be:

const todoTypePolicy = {
  Todo: {
    fields: {
      isCompleted: {
        read(_current, { readField }) {
          const completionState = readField('completionState');
          return completionState?.value === 100; 
        }
      }
    }
  } 
}

And it would keep doing the right thing regardless what args are passed to completionState.

[jerelmiller]

Hey @joelmukuthu 👋

This seems like a reasonable ask!

I can't promise anything in the immediate future as we are going to be working on our v4 release as soon as 3.12 is out. Once v4 is out, I'd love to come back to this ask. Thanks for opening the issue!

[joelmukuthu]

Thanks for responding and validating this use-case @jerelmiller 🙌

I fully understand regarding prioritisation. I wonder if this would count as a breaking change though, in which case it would be nice to include in v4?

[jerelmiller]

@joelmukuthu My gut says this isn't a breaking change so I don't think we need to put it in a major on its own. That said, I haven't looked too deeply into what it would take to implement this so I could be wrong. The biggest question on whether its breaking would be to determine if adding this behavior breaks any existing cases with the existing API. If no, then I think this change is purely additive.