graphql / graphql-js

A reference implementation of GraphQL for JavaScript
http://graphql.org/graphql-js/
MIT License
20.06k stars 2.02k forks source link

Standardize the naming of the first argument to resolver functions (obj vs parent) #2842

Open magicmark opened 3 years ago

magicmark commented 3 years ago

According to, graphql.org/learn, "A resolver function receives four arguments". It lists the first one as:

obj The previous object, which for a field on the root Query type is often not used.

However, Apollo's docs calls this same argument 'parent':

parent The return value of the resolver for this field's parent (i.e., the previous resolver in the resolver chain).

obj vs parent is a source of confusion throughout the ecosystem. We see this elsewhere too:

"We already have a standard - obj! Ask the non-conformists to switch!"

I could see this argument, and I'm happy to close this issue out if that's how the mainters feel.

However! ...allow me to put forward a case for parent and bikeshed for a moment:

  1. I believe the name "parent" converys more meaningful information than "obj". It signals some sort of relationship to something above us. This more strongly hints that obj/parent === the return value of the previous (or "parent") resolver.
  2. "object" could be implementation detaily, or at best, is ambiguous to if this represents the language-specific construct (e.g. javascript objects) or a conceptual "object" (not all languages have dicts/objects!)

I'm not sure what the timelines are or who first decided to use "parent", but I'm guessing this happened for a reason, and this isn't the first time this issue has been discussed. If anyone can find any written deliberation about this, I'd be interested to read up!

Speaking first hand, teaching resolver arguments concepts is hard. This in particular has come up as a source of confusion (hence me making this issue) internally at my workplace. Any gradual win here will be leveraged across many many folks learning this anew.

Members of the jury, in conclusion, I believe it would be an admirable goal to standardise obj vs parent. I personally vote for parent, but would be happy with obj. Either way, I would like to start a mission to try and standardize this across tooling/languages.

What does the spec say?

The official GraphQL spec avoids giving language specific implementation details, so it doesn't (can't?) standardize a name for this. However it does hint at "object" in an example given.

Perhaps a non-normative note somewhere in the spec would be a good place provide a hint and normalize the naming.

benjie commented 3 years ago

I personally favour "parent", but I think one problem with "parent" is that it is potentially ambiguous. For example, imagine you have a query like:

{
  forum { # Query.forum
    post { # Forum.post
      title # Post.title
    }
  }
}

Here the field title can be thought of as a property of the Post object (rather than a "child" of it). So the Post object is sort of "self", and the Forum object is sort of the parent.

mike-marcacci commented 3 years ago

I have found the terminology of "parent" to be very confusing to newcomers for exactly the reason @benjie described: it can easily be misinterpreted as "parent of the current entity" rather than "parent of the current field, which is the current entity."

We use the word source internally, for better or worse.

IvanGoncharov commented 3 years ago

@magicmark Agree with @mike-marcacci on source it's actually the official name in our codebase: https://github.com/graphql/graphql-js/blob/cc146bc7d4c7d46950d4b43dcc051bc9eb058bdb/src/type/definition.js#L938

In general, I'm totally 👍 for standardization and agree that obj is too generic also parent resolvers can return scalar values (e.g. IDs) and in this case, obj looks especially strange.

That said I'm against parent in particular because:

  1. We should think about resolvers in terms of graphql type they are bind to and not in terms of resolvers chains. Because in the complex schema, you simply can't track all "parent" resolvers.
  2. In many cases GraphQL resolvers defined as classes (so the first argument replaced with this) we also support this use case (e.g. swapi-demo) so it confusing that parent is replaced with this in this case.

If anyone has a good alternative to source let's discuss it.