Skip to content

Conversation

@bbakerman
Copy link
Member

For runtimes like GraalVm we dont want to introduce PropertyDataFetcher reflection to find the system __Type style objects which are really based of the graphql.schema.xxx objects

This adds explcit data fetchers for the small amount of fields that ended up using PropertyDataFetcher

@bbakerman bbakerman added this to the 20.0 milestone Oct 31, 2022
register(__InputValue, "defaultValue", defaultValueDataFetcher);
register(__InputValue, "isDeprecated", isDeprecatedDataFetcher);
register(__InputValue, "deprecationReason", deprecationReasonDataFetcher);
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reorganised the registration to that the register call is done in simple lines and the fetchers are defined above.

I also ordered them in the order of field definition from above.

So you can see that we defined name field first, so its DF registration is first and so on.

.deprecate("see `specifiedByURL`")
.name("specifiedByUrl")
.type(GraphQLString)
.deprecate("This legacy name has been replaced by `specifiedByURL`")
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tweaked the comment while I was here - boy scout!

.field(newFieldDefinition()
.name("onField")
.type(GraphQLBoolean)
.deprecate("Use `locations`."))
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

onOperation / onFragment and onField was removed from the spec in 2016 - this has just sat around. I think it was broken. We have NO data fetcher defined AND there is not getOnFragment method that would give a value so it would always be null

I took the opportunity to remove this very old code

import graphql.Assert;
import graphql.GraphQLContext;
import graphql.Internal;
import graphql.PublicApi;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tests pick up the fields in place - we have great coverage here.

I put a debug point in PropertyDataFetcher when running the main tests and I never hit it for introspection related calls - so one more reason on why its not all covered by actual DFs

@bbakerman bbakerman merged commit 0264f30 into master Nov 29, 2022
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