RFC: Extra property on field definition to pass extra metadata

[JCMais]

I'm currently adding extra properties to some graphql object field definitions, like the following:

const MutationType = new GraphQLObjectType({
  name: 'Mutation',
  fields: () => ({
    AddSomething: {
      // ... normal field properties
      somethingElse: {},
    }
  }),
})

And then using them later on via the info argument inside some middlewares (using graphql-middleware):

const mutationField = info.schema.getMutationType().getFields()[info.fieldName];
console.log(mutationField.somethingElse);

For more details, see the following medium post: graphql mutation arguments validation using yup

---

The thing is, this is relying on internal behavior.
The following code spreads all properties given to the field:

graphql-js/src/type/definition.js

Lines 720 to 724 in 8171974

	const field = {
	...fieldConfig,
	isDeprecated: Boolean(fieldConfig.deprecationReason),
	name: fieldName,
	};

Is that something expected to not change? If yes, then no need for any other extra property or for this issue. 😄

But if this is something that can change in future versions, I would love the possibility of having an extra field for that extra metadata.

I'm available to work on adding this, if it's approved.

[IvanGoncharov]

Is that something expected to not change?

Yes. It's explicitly forbidden in Flow typings:

graphql-js/src/type/definition.js

Lines 810 to 822 in 3789a87

	export type GraphQLFieldConfig<
	TSource,
	TContext,
	TArgs = { [argument: string]: any },
	> = {|
	type: GraphQLOutputType,
	args?: GraphQLFieldConfigArgumentMap,
	resolve?: GraphQLFieldResolver<TSource, TContext, TArgs>,
	subscribe?: GraphQLFieldResolver<TSource, TContext, TArgs>,
	deprecationReason?: ?string,
	description?: ?string,
	astNode?: ?FieldDefinitionNode,
	|};

Note: {| ... |} construct in Flow it's called exact type and it explicitly forbids any other properties. Sadly TS doesn't support it: microsoft/TypeScript#12936

I would love the possibility of having an extra field for that extra metadata.

@JCMais Agree 👍 Plus it provides a clean solution to #1343 without changing directive semantics.

[jgoux]

I'm all for it! 👍

When you want to modularize the typeDefs + resolvers definitions across your codebase, it's great to be able to pass additional metadata for consumption by third-party libraries such as graphql-middleware.

Why not add an extensions key, so it makes the parallel with the GraphQLError extension point.

[JCMais]

+1 for extensions

Ideally libraries using this would allow to customize the key name, to reduce possibility of key collisions.

[JCMais]

what can be done to move this forward?

[sibelius]

I think the first part is to remove exact types on flow definitions

[flippidippi]

+1 this could help fix issues with join-monster that requires extra metadata for fields

[MichalLytek]

I would treat this exact Flow definition as a bug because this quote from @leebyron (#1262 (comment)):

You can include any arbitrary data on a GraphQLObjectType that you like - it's just a JS object after all.

So I treat this pattern as a recomended way to have some metadata on types/fields like directives allows use to do in SDL.

[IvanGoncharov]

I would treat this exact Flow definition as a bug because this quote

This quote is about actual GraphQL*Type not about config types like GraphQL*TypeConfig.

You can include any arbitrary data on a GraphQLObjectType that you like - it's just a JS object after all.

It basically says you can do:

const someType = new GraphQLObjectType({
  name: 'Foo',
  // ...
});

someType.customProperty = "Custom Value";

Personally, I think it's a workaround especially for clients who use Flow/TS at the same time I think config types should remain exact.

---

The alternative proposal would be:

const someType = new GraphQLObjectType({
  name: 'Foo',
  // ...
  extensions: {
    customProperty: "Custom Value",
  },
});

Where extensions defined as ObjMap<mixed>.

@JCMais What do you think about this proposal?

[JCMais]

@IvanGoncharov that was exactly the kind of alternative I was looking for.

Just a specific key that we can be sure it will be available later on, from the info argument inside resolvers, for example.

[MichalLytek]

This quote is about actual GraphQLType not about config types like GraphQLTypeConfig.

someType.customProperty = "Custom Value";

So how to place metadata on fields and args like directives do by using this way?
We configure fields and its args in a type config for GraphQLObjectType as a plain objects.

Should we mutate this object later (by using (schema.getType("typeName") as GraphQLObjectType).getFields()["fieldName"].complexity = 5;) or is it safe to rely on ...fieldConfig behavior in that case?

[IvanGoncharov]

Should we mutate this object later (by using (schema.getType("typeName") as GraphQLObjectType).getFields()["fieldName"].complexity = 5;) or is it safe to rely on ...fieldConfig behavior in that case?

@19majkel94 It should work fine as a workaround until we ship an extensions solution however no long-term guarantees afterward.

I will work on adding immutable extensions into types, fields, args and enum values on next week so we will be able to ship it in 14.2.0.

[nodkz]

🛑Before shipping this we should consider how extensions can be provided via SDL.

Add @extensions directive like so:

type User {
  name @extensions(cost: { complexity: 2 })
}

We MUST provide a unified solution for SDL and configObjects. And will be nice if it will be covered by the static analysis (if possible of course). For now mixed value for extensions is weak and error-prone.

Via SDL it can be extended somehow via directive config. With Flowtype/Typescript it can be validated via explicitly provided type for extensions config object.

[JCMais]

@nodkz this sounds good, since it has been some time since I last used graphql-js directly, how is the custom directive going to be extended by library authors?

That would be like a schema directive that apollo has, correct? Do we have support for that currently?

Last time I checked (which has been some time ago) custom directives were one of the most dark features from an user pov, when using graphql-js.

[nodkz]

Currently, directives cannot be extended. There was no purpose. But for unified @extensions solution it may be required option, maybe not. Maybe somebody suggests a more elegant way.

[freiksenet]

We've been using it very effectively at Gatsby through graphql-compose extensions. Adding it here as an example of "usage in the wild".

[IvanGoncharov]

Is extensions going to simply be part of the *Type config as { [key: string]: any}.

Yes, except it would be unknown (mixed in Flow) instead of any. Also, it would be { [key: string]: unknown} in config (and toConfig result) but inside object itself it would be { readonly [key: string]: mixed }

And if the PR lands this week, is there an ETA on a release? Simply because we've sort of put a freeze on our back-end until federation is supported intype-graphql. :)

I will make 14.5.0 after this merge.

Also, can/will extensions be added to the type.toConfig()?

Definitely, also I need to support it in other places, e.g. lexicographicSortSchema should sort extensions.

[cliedeman]

Maybe string and symbol for keys? This can ensure we don't have collisions

[IvanGoncharov]

@cliedeman We can definitely use Symbols for solving collisions inside in-memory schema but it will just push the problem down the road. For example, I have a long term plan to figure out how to allow exposing extensions through introspection but it's impossible to use symbols inside introspection responses.

So my proposal is to keep everything simple for the first release and figure out how to solve problems only if they will happen in the wild.

BTW, you can also have collisions of directive names so it's not a new problem in GraphQL world.

[j]

@IvanGoncharov This push will make my week. :)

[IvanGoncharov]

@j Just to give some transparency about the timeline.
At the moment I'm working on #2037 since it's security-related and this issue is second in my pipeline.
After both PRs merged I will release 14.5.0.

[IvanGoncharov]

Quick update: #2037 taking more time than I initially expected so now I'm pushing for releasing at the end of this month. Thanks for the patience.

[j]

I see movement! Yay! :p

[IvanGoncharov]

It took longer than I planned but I finally merged it in #2097
Thanks for patience 🙇
I will try to cut release today/tomorrow

[JCMais]

Thank you @IvanGoncharov !

[j]

@IvanGoncharov holy moly, the GraphQL world is going to get so much better now. :) Thanks!