Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make sure the name property is on the field spec #488

Closed

Conversation

singingwolfboy
Copy link
Contributor

I noticed that some fields didn't have the name field properly populated on the spec -- which is ironic, considering that fieldWithHooks must be called with a fieldName argument. This pull request uses that argument to set that name on the spec, although other specs can override it as necessary.

@benjie
Copy link
Member

benjie commented Jul 19, 2019

Hey DB 😃

The spec here is a GraphQLFieldConfig from graphql-js which does not accept a name field:

type GraphQLFieldConfig = {
  type: GraphQLOutputType;
  args?: GraphQLFieldConfigArgumentMap;
  resolve?: GraphQLFieldResolveFn;
  deprecationReason?: string;
  description?: ?string;
}

The field name is available on the context a few lines down (line 650) via context.field.name context.scope.fieldName which will be available in args hooks.

I'm going to close this, but feel free to follow up with further questions.

EDIT: corrected mistake for future readers.

@benjie benjie closed this Jul 19, 2019
@singingwolfboy
Copy link
Contributor Author

Ah! I made this pull request because I noticed that PgConnectionArgCondition is trying to access field.name when logging a message, and I was getting undefined, which I assumed was a bug in the field definition. But maybe it's a bug in the logging code, instead!

@benjie
Copy link
Member

benjie commented Jul 19, 2019

Ah, indeed a bug in the logging code. 😳

@singingwolfboy
Copy link
Contributor Author

The field name is available on the context a few lines down (line 650) via context.field.name which will be available in args hooks.

I just did some more investigation, and I discovered that the field variable in PgConnectionArgCondition is context.field.name! And since that value does not have a name field, it seems like your statement here is incorrect (unintentionally, I'm sure). Is there another way of getting the field name from a GraphQLObjectType:fields:field:args hook?

@benjie
Copy link
Member

benjie commented Aug 6, 2019

(I believe @singingwolfboy answered his own question in #489 )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants