Port changes from graphql-js v0.5.0 (April 2016 spec)
#123
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Commit: b3a512787618cfbed20c9717daf2d69444bf7f33 [b3a5127] Parents: 5ea2ff1e17 Author: Tim Griesser <tgriesser10@gmail.com> Date: 9 March 2016 at 10:02:07 A
|
When is this and #117 likely to be merged in? |
|
I was checking to see whether 0.6.0 would be supported :-) there are breaking changes and first class support for https://gist.github.com/steveluscher/ffc1dfefbb10ad280c8a4c520a5c201c Issue is |
This proposes a change to how we represent the ability to validate the locations of directives via introspection. Specifically, this deprecates `onField`, `onFragment`, and `onOperation` in favor of `locations` which is a list of `__DirectiveLocation`. **Rationale:** This allows for a more fine-grained validation of directive placement, now you can assert that a directive is allowed on queries but not mutations, or allowed on fragment definitions but not on fragment spreads. Also, this makes expanding the locations a directive is allowed to be placed easier to do, as future expansions will not affect the introspection API. This should be considered a prereq to graphql-go#265. Finally, this is a prereq to a forthcoming RFC to add directives to the type schema language, one of the last missing pieces to represent a full schema using this language. Currently considering something like: ``` directive @Skip(if: Boolean) on FIELD, FRAGMENT_SPREAD, INLINE_FRAGMENT ``` **Drawbacks:** Any change to the introspection API is a challenge. Especially so for graphql-js which is used as both client tools via Graph*i*QL and as a node.js server. To account for this, I've left the existing fields as deprecated, and continued to support these deprecated fields in `buildClientSchema`, which is used by Graph*i*QL. While graphql-js will likely continue to expose these deprecated fields for some time to come, the spec itself should not include these fields if this change is reflected. Commit: e89c19d2ce584f924c2e0e472a61bb805ce260d4 [e89c19d] Parents: 57d71e108a Author: Lee Byron <lee@leebyron.com> Date: 18 March 2016 at 7:33:28 AM SGT Commit Date: 18 March 2016 at 7:57:11 AM SGT Labels: HEAD
|
Hi @bsr203 Man, it was barely a month ago graphql-js 0.5.0 was released, now its 0.6.0, phew~ lol Yeah I'll definitely try to bring up this library up to parity as quickly as possible when I get free time off work. First 0.5.0, then 0.6.0 😅 Re: breaking changes from 0.5.0 -> 0.6.0, it looks promising so far:
Cheers! |
Add tests confirming graphql-go#319 Commit: 3278e861837cd3f7d17eaec54f3f04b175300826 [3278e86] Parents: 3ce90faf26 Author: Lee Byron <lee@leebyron.com> Date: 23 March 2
This adds directives to schema language and to the utilities that use it (schema parser, and buildASTSchema). Directives are one of the few missing pieces from representing a full schema in the schema language. Note: the schema language is still experimental, so there is no corresponding change to the spec yet. DirectiveDefinition : - directive @ Name ArgumentsDefinition? on DirectiveLocations DirectiveLocations : - Name - DirectiveLocations | Name Example: ``` directive @Skip(if: Boolean!) on FIELD | FRAGMENT_SPREAD | INLINE_FRAGMENT directive @include(if: Boolean!) on FIELD | FRAGMENT_SPREAD | INLINE_FRAGMENT ``` Commit: fdafe32724f2d6dac1ba360f1a440c0e633e75d9 [fdafe32] Parents: bf763b6f53 Author: Lee Byron <lee@leebyron.com> Date: 18 March 2016 at 10:47:06 AM SGT Commit Date: 23 March 2016 at 6:19:46 AM SGT
graphql/graphql-spec#90 Commit: b0885a038ec0e654962d69fb910ac86659279579 [b0885a0] Parents: fdafe32724 Author: Lee Byron <lee@leebyron.com> Date: 23 March 2016 at 6:35:37 AM SGT Commit Date: 23 March 2016 at 6:35:39 AM SGT
This implements the schema definition in the spec proposal in graphql/graphql-spec#90 Adjusts AST, parser, printer, visitor, but also changes the API of buildASTSchema to require a schema definition instead of passing the type names into the function. Commit: 8379e71f7011fe044574f4bdef2a761d18d6cf2c [8379e71] Parents: 176076c8a6 Author: Lee Byron <lee@leebyron.com> Date: 23 March 2016 at 7:38:15 AM SGT Labels: HEAD
Commit: 533fc43f7d5836fc9b6a3eafde9801db9a3ee011 [533fc43] Parents: 371a5a0908 Author: Lee Byron <lee@leebyron.com> Date: 23 March 2016 at 8:21:04 AM SGT Commit Date: 23 March 2016 at 8:22:42 AM SGT
Commit: 43992e3a60d7bf8f5e7d2f29dfae302b5ba72eec [43992e3] Parents: 533fc43f7d Author: Lee Byron <lee@leebyron.com> Date: 23 March 2016 at 8:58:05 AM SGT
Commit: 43992e3a60d7bf8f5e7d2f29dfae302b5ba72eec [43992e3] Parents: 533fc43f7d Author: Lee Byron <lee@leebyron.com> Date: 23 March 2016 at 8:58:05 AM SGT
…rror defaultResolveFn Commit: edc405a11508a110759ce53c9efb2eb6dd2d181c [edc405a] Parents: 3f6a7f4ca9 Author: jeff@procata.com <jeff@procata.com> Date: 24 March 2016 at 3:15:33 PM SGT
… coverage. Should be removed once `onOperation`, `onFragment`, `onField` are dropped from spec.
…TypeFn to mirror defaultResolveFn
This is a rebased and updated version of @tgriesser's graphql-go#199. I further extended it to completely remove the side-effectful mutation of Interface types rather than just deferring that mutation to schema creation time. This introduces a *breaking* change to the Type System API. Now, any individual Interface type does not have the required information to answer `getPossibleTypes` or `isPossibleType` without knowing the other types in the Schema. These methods were moved to the Schema API, accepting the abstract type as the first parameter. This also introduces a *breaking* change to the type comparator functions: `isTypeSubTypeOf` and `doTypesOverlap` which now require a Schema as a first argument. Commit: 6a1f23e1f9c1e6bf4cea837bc9bb6eae0fb5c382 [6a1f23e] Parents: a781b556ee Author: Lee Byron <lee@leebyron.com> Date: 23 March 2016 at 1:17:43 PM SGT Commit Date: 25 March 2016 at 6:35:39 AM SGT
This **BREAKING** change introduces a new argument to the GraphQL execution API which is presented to resolution functions: `context`. Removed `Schema` from `ResolveParams`, already available in `ResolveParams.Info`. `IsTypeOf` and `ResolveType` params are now struct, similar to `FieldResolveFn`. `context` is now available for resolving types in `IsTypeOf` and `ResolveType`, similar to `FieldResolveFn`. Commit: d7cc6f9aed462588291bc821238650c98ad53580 [d7cc6f9] Parents: 576b6a15d1 Author: Lee Byron <lee@leebyron.com> Date: 23 March 2016 at 10:30:13 AM SGT Commit Date: 25 March 2016 at 8:20:10 AM SGT
* Replace silent conversion to null with an invariant and a useful error message Commit: dea5aacca01f4429026bea3d97ea7cbc88b33bcf [dea5aac] Parents: 39744381d5 Author: Jeff Moore <jeff@procata.com> Date: 5 April 2016 at 12:48:14 PM SGT Committer: Lee Byron <lee@leebyron.com>
- Similar to `context` test
Commit: d6da0bff7f877e6a4fb66119796809f9c207f841 [d6da0bf] Parents: 136630f8fd Author: Dylan Thacker-Smith <dylan.smith@shopify.com> Date: 5 April 2016 at 12:56:27 PM SGT Committer: Lee Byron <lee@leebyron.com> Labels: HEAD
Commit: 47f87fa701cb33fc1fb0ca65b4668c7a14a5ad11 [47f87fa] Parents: 9e5d959353 Author: Lee Byron <lee@leebyron.com> Date: 5 April 2016 at 1:00:00 PM SGT remove non spec compliant test Commit: 07e627adda2b236e720ec352b21eb3043170c60f [07e627a] Parents: cf5b2340f9 Author: Lee Byron <lee@leebyron.com> Date: 5 April 2016 at 1:09:42 PM SGT
Commit: 3201ebb825f7bab4ca4856238ade4603e15a5abc [3201ebb] Parents: c7e6a75277 Author: Lee Byron <lee@leebyron.com> Date: 6 April 2016 at 11:08:06 AM SGT Labels: HEAD
- `completeValue` would already have resolved the value for `func() interface{}`
Implements graphql/graphql-spec#162 This alters the "Overlapping Fields Can Be Merged" validation rule to better express return type validation issues. The existing rule has presented some false-positives in some schema where interfaces with co-variant types are commonly used. The "same return type" check doesn't remove any ambiguity. Instead what that "same return type" check is attempting to prevent is spreading two fragments (or inline fragments) which have fields with return types where ambiguity would be introduced in the response. In order to curb false-positives, we later changed this rule such that if two fields were known to never apply simultaneously, then we would skip the remainder of the rule. ``` { ... on Person { foo: fullName } ... on Pet { foo: petName } } ``` However this can introduce false-negatives! ``` { ... on Person { foo: birthday { bar: year } } ... on Business { foo: location { bar: street } } } ``` In the above example, `data.foo.bar` could be of type `Int` or type `String`, it's ambiguous! This differing return type breaks some client model implementations (Fragment models) in addition to just being confusing. Commit: c034de91acce10d5c06d03bd332c6ebd45e2213c [c034de9] Parents: ffe76c51c4 Author: Lee Byron <lee@leebyron.com> Date: 7 April 2016 at 2:00:31 PM SGT Labels: HEAD
Merged
sogko
changed the title
graphql-js v0.5.0(April 2016 spec)graphql-js v0.5.0 (April 2016 spec)
|
Hi guys, Calling for comments / reviews for this PR. I know this is a huge dump of commits at the moment, because we are basically playing catch up to I've listed notable big changes in the original PR description, along with anything that introduces breaking changes to the API. /cc: @chris-ramon @bsr203 |
This was referenced Jun 2, 2016
Closed
This was referenced Jun 11, 2016
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Started work on porting changes from graphql-js v0.5.0 which is based on the latest April 2016 spec.
Previous port was based on v0.4.18 (pending PR #117 merge)
Notable changes:
graphql.NewDirective(*graphql.Directive)=>graphql.NewDirective(graphql.DirectiveConfig)ObjectType(interface{}, ResolveInfo) *ObjectfromAbstractinterface,UnionandInterfacestructs and moved toexecutorObjectTypeof anAbstracttype (Union/Interface)GetPossibleTypesorIsPossibleTypewithout knowing the other types in the Schema. These methods were moved to the Schema API, accepting the abstract type as the first parameter.IsTypeSubTypeOfandDoTypesOverlapwhich now require a Schema as a first argument.SchemafromResolveParams, already available inResolveParams.InfoIsTypeOfFnandResolveTypeFnparams are now struct, similar toFieldResolveFncontextis now available for resolving types inIsTypeOfFnandResolveTypeFn, similar toFieldResolveFnChanges to public/exported vars / func
graphql.NewDirective(*graphql.Directive)graphql.NewDirective(graphql.DirectiveConfig)graphql.Abstract.ObjectType(interface{}, ResolveInfo) *Objectremovedgraphql.Union.ObjectType(interface{}, ResolveInfo) *Objectremovedgraphql.Interface.ObjectType(interface{}, ResolveInfo) *Objectremovedgraphql.IsTypeOfFn(interface{}, ResolveInfo)graphql.IsTypeOfFn(IsTypeOfParams)graphql.ResolveTypeFn(interface{}, ResolveInfo)graphql.ResolveTypeFn(ResolveTypeParams)graphql.ResolveParams.Schemaremoved, already available ingraphql.ResolveInfo