Schema Definition Language Support

[NeedleInAJayStack]

This adds full Schema Definition Language (SDL) support to this package, including:

• Printing SDLs for existing schemas
• Building new schemas directly from SDLs
• Using SDL to extend already-defined schemas
• Validating SDL schemas

It does so primarily by porting graphql-js.

Unfortunately, this is a breaking change, even though I worked hard to keep the breakages to a minimum. Notably:

• TypeReference was deprecated. Almost all existing usage is back-supported, but recursively defined types can be problematic. Most of users won't be impacted after we update Graphiti.
• Some public Definition type properties have changed slightly (become optional or were converted to closures). I back-supported in most cases, but it wasn't possible everywhere.

Since this is a breaking change, I'd be interested in opinions on whether we should just drop TypeReference support instead of keeping it around as deprecated. And while we're at it, should we make any other public interface changes?

Fixes#55

[paulofaria]

The main difference between the canonical implementation and ours is the need for type references. If I recall correctly, it was because JS used thunks because it allows a symbol to be used inside a closure before its value is bound. I looked for other implementations for inspiration, like Java. I think that's where I got TypeReference from. When you say TypeReference was deprecated, what exactly do you mean. Maybe I'm misremembering stuff, though?

[NeedleInAJayStack]

The main difference between the canonical implementation and ours is the need for type references. If I recall correctly, it was because JS used thunks because it allows a symbol to be used inside a closure before its value is bound. I looked for other implementations for inspiration, like Java. I think that's where I got TypeReference from.

Oh thanks, that's very good background! In this PR I was able to get all existing GraphQL and Graphiti tests running without TypeReferences after changing the GraphQLObject field property to a closure-based API. A pretty good example of the change can be seen in this comment diff: https://github.com/GraphQLSwift/GraphQL/pull/154/files#diff-a5342873cae3c47e54aedd945d19280fa6c7d03f66ad1f7ab13b16ec79c9acf1L288-L299 I'd be interested in your thoughts and if you see any gaps with that approach!

When you say TypeReference was deprecated, what exactly do you mean.

I marked it with @available(*, deprecated, ...)here. By doing so, it will give compiler warnings to anyone that is using it, but will still compile. I did this back when I thought I would be able to make this PR a minor version bump, but if it requires a major it seems like we maybe ought to just delete it.

Also, sorry - I know this is a humongous pull request. Unfortunately all these features use each other in their tests so it was hard to decouple them into separate pull requests.

[paulofaria]

How would cyclical or recursive references be defined without TypeReference? Can you point me to examples?

About the PR size, no worries, I am famous for such PRs myself. 😂

[NeedleInAJayStack]

How would cyclical or recursive references be defined without TypeReference? Can you point me to examples?

Sure, here's an example of a recursive type with TypeReference removed. This works out-of-the-box because CharacterInterface is a global variable.

For non-globals, you can do hit the issue of referencing a variable in a closure before it is created. This is solved by just creating the GraphQL type with empty fields, and then setting the fields closure in a subsequent line. This function contains a test schema that uses this approach on a recursive type. Note that this has the added benefit of allowing the user to avoid the memory cycle caused by recursive types.

Downstream in Graphiti, this then works exactly as you'd expect, where type references are no longer necessary to handle creation ordering/etc

[paulofaria]

Oh, that's great news. I'm not sure if SSWG has any guidelines regarding deprecation. I imagine that as long as we follow SEMVER we're fine. So, if we're going to bump to a major version anyway, I'd say let's remove it altogether. Maybe just add some documentation somewhere explaining how to deal with its absence like you just explained.

[NeedleInAJayStack]

I'm not sure if SSWG has any guidelines regarding deprecation.

I looked through the docs here, but it doesn't appear they have any deprecation requirements, so it seems like we're just okay bumping major.

just add some documentation somewhere explaining how to deal with its absence like you just explained.

Great point. I've added a MIGRATION.md file and linked it from the README, and I've removed GraphQLTypeReference and all usage.

You mind giving it one more quick look? Thanks for the review!

[paulofaria]

Perfect! Awesome work, man! 😄

[NeedleInAJayStack]

This adds full Schema Definition Language (SDL) support to this package, including:

• Printing SDLs for existing schemas
• Building new schemas directly from SDLs
• Using SDL to extend already-defined schemas
• Validating SDL schemas

It does so primarily by porting graphql-js.

Unfortunately, this is a breaking change, even though I worked hard to keep the breakages to a minimum. Notably:

• TypeReference was deprecated. Almost all existing usage is back-supported, but recursively defined types can be problematic. Most of users won't be impacted after we update Graphiti.
• Some public Definition type properties have changed slightly (become optional or were converted to closures). I back-supported in most cases, but it wasn't possible everywhere.

Since this is a breaking change, I'd be interested in opinions on whether we should just drop TypeReference support instead of keeping it around as deprecated. And while we're at it, should we make any other public interface changes?

Fixes#55

[paulofaria]

The main difference between the canonical implementation and ours is the need for type references. If I recall correctly, it was because JS used thunks because it allows a symbol to be used inside a closure before its value is bound. I looked for other implementations for inspiration, like Java. I think that's where I got TypeReference from. When you say TypeReference was deprecated, what exactly do you mean. Maybe I'm misremembering stuff, though?

[NeedleInAJayStack]

The main difference between the canonical implementation and ours is the need for type references. If I recall correctly, it was because JS used thunks because it allows a symbol to be used inside a closure before its value is bound. I looked for other implementations for inspiration, like Java. I think that's where I got TypeReference from.

Oh thanks, that's very good background! In this PR I was able to get all existing GraphQL and Graphiti tests running without TypeReferences after changing the GraphQLObject field property to a closure-based API. A pretty good example of the change can be seen in this comment diff: https://github.com/GraphQLSwift/GraphQL/pull/154/files#diff-a5342873cae3c47e54aedd945d19280fa6c7d03f66ad1f7ab13b16ec79c9acf1L288-L299 I'd be interested in your thoughts and if you see any gaps with that approach!

When you say TypeReference was deprecated, what exactly do you mean.

I marked it with @available(*, deprecated, ...)here. By doing so, it will give compiler warnings to anyone that is using it, but will still compile. I did this back when I thought I would be able to make this PR a minor version bump, but if it requires a major it seems like we maybe ought to just delete it.

Also, sorry - I know this is a humongous pull request. Unfortunately all these features use each other in their tests so it was hard to decouple them into separate pull requests.

[paulofaria]

How would cyclical or recursive references be defined without TypeReference? Can you point me to examples?

About the PR size, no worries, I am famous for such PRs myself. 😂

[NeedleInAJayStack]

How would cyclical or recursive references be defined without TypeReference? Can you point me to examples?

Sure, here's an example of a recursive type with TypeReference removed. This works out-of-the-box because CharacterInterface is a global variable.

For non-globals, you can do hit the issue of referencing a variable in a closure before it is created. This is solved by just creating the GraphQL type with empty fields, and then setting the fields closure in a subsequent line. This function contains a test schema that uses this approach on a recursive type. Note that this has the added benefit of allowing the user to avoid the memory cycle caused by recursive types.

Downstream in Graphiti, this then works exactly as you'd expect, where type references are no longer necessary to handle creation ordering/etc

[paulofaria]

Oh, that's great news. I'm not sure if SSWG has any guidelines regarding deprecation. I imagine that as long as we follow SEMVER we're fine. So, if we're going to bump to a major version anyway, I'd say let's remove it altogether. Maybe just add some documentation somewhere explaining how to deal with its absence like you just explained.

[NeedleInAJayStack]

I'm not sure if SSWG has any guidelines regarding deprecation.

I looked through the docs here, but it doesn't appear they have any deprecation requirements, so it seems like we're just okay bumping major.

just add some documentation somewhere explaining how to deal with its absence like you just explained.

Great point. I've added a MIGRATION.md file and linked it from the README, and I've removed GraphQLTypeReference and all usage.

You mind giving it one more quick look? Thanks for the review!

[paulofaria]

Perfect! Awesome work, man! 😄

[NeedleInAJayStack]

This adds full Schema Definition Language (SDL) support to this package, including:

• Printing SDLs for existing schemas
• Building new schemas directly from SDLs
• Using SDL to extend already-defined schemas
• Validating SDL schemas

It does so primarily by porting graphql-js.

Unfortunately, this is a breaking change, even though I worked hard to keep the breakages to a minimum. Notably:

• TypeReference was deprecated. Almost all existing usage is back-supported, but recursively defined types can be problematic. Most of users won't be impacted after we update Graphiti.
• Some public Definition type properties have changed slightly (become optional or were converted to closures). I back-supported in most cases, but it wasn't possible everywhere.

Since this is a breaking change, I'd be interested in opinions on whether we should just drop TypeReference support instead of keeping it around as deprecated. And while we're at it, should we make any other public interface changes?

Fixes#55

[paulofaria]

The main difference between the canonical implementation and ours is the need for type references. If I recall correctly, it was because JS used thunks because it allows a symbol to be used inside a closure before its value is bound. I looked for other implementations for inspiration, like Java. I think that's where I got TypeReference from. When you say TypeReference was deprecated, what exactly do you mean. Maybe I'm misremembering stuff, though?

[NeedleInAJayStack]

The main difference between the canonical implementation and ours is the need for type references. If I recall correctly, it was because JS used thunks because it allows a symbol to be used inside a closure before its value is bound. I looked for other implementations for inspiration, like Java. I think that's where I got TypeReference from.

Oh thanks, that's very good background! In this PR I was able to get all existing GraphQL and Graphiti tests running without TypeReferences after changing the GraphQLObject field property to a closure-based API. A pretty good example of the change can be seen in this comment diff: https://github.com/GraphQLSwift/GraphQL/pull/154/files#diff-a5342873cae3c47e54aedd945d19280fa6c7d03f66ad1f7ab13b16ec79c9acf1L288-L299 I'd be interested in your thoughts and if you see any gaps with that approach!

When you say TypeReference was deprecated, what exactly do you mean.

I marked it with @available(*, deprecated, ...)here. By doing so, it will give compiler warnings to anyone that is using it, but will still compile. I did this back when I thought I would be able to make this PR a minor version bump, but if it requires a major it seems like we maybe ought to just delete it.

Also, sorry - I know this is a humongous pull request. Unfortunately all these features use each other in their tests so it was hard to decouple them into separate pull requests.

[paulofaria]

How would cyclical or recursive references be defined without TypeReference? Can you point me to examples?

About the PR size, no worries, I am famous for such PRs myself. 😂

[NeedleInAJayStack]

How would cyclical or recursive references be defined without TypeReference? Can you point me to examples?

Sure, here's an example of a recursive type with TypeReference removed. This works out-of-the-box because CharacterInterface is a global variable.

For non-globals, you can do hit the issue of referencing a variable in a closure before it is created. This is solved by just creating the GraphQL type with empty fields, and then setting the fields closure in a subsequent line. This function contains a test schema that uses this approach on a recursive type. Note that this has the added benefit of allowing the user to avoid the memory cycle caused by recursive types.

Downstream in Graphiti, this then works exactly as you'd expect, where type references are no longer necessary to handle creation ordering/etc

[paulofaria]

Oh, that's great news. I'm not sure if SSWG has any guidelines regarding deprecation. I imagine that as long as we follow SEMVER we're fine. So, if we're going to bump to a major version anyway, I'd say let's remove it altogether. Maybe just add some documentation somewhere explaining how to deal with its absence like you just explained.

[NeedleInAJayStack]

I'm not sure if SSWG has any guidelines regarding deprecation.

I looked through the docs here, but it doesn't appear they have any deprecation requirements, so it seems like we're just okay bumping major.

just add some documentation somewhere explaining how to deal with its absence like you just explained.

Great point. I've added a MIGRATION.md file and linked it from the README, and I've removed GraphQLTypeReference and all usage.

You mind giving it one more quick look? Thanks for the review!

[paulofaria]

Perfect! Awesome work, man! 😄

[NeedleInAJayStack]

This adds full Schema Definition Language (SDL) support to this package, including:

• Printing SDLs for existing schemas
• Building new schemas directly from SDLs
• Using SDL to extend already-defined schemas
• Validating SDL schemas

It does so primarily by porting graphql-js.

Unfortunately, this is a breaking change, even though I worked hard to keep the breakages to a minimum. Notably:

• TypeReference was deprecated. Almost all existing usage is back-supported, but recursively defined types can be problematic. Most of users won't be impacted after we update Graphiti.
• Some public Definition type properties have changed slightly (become optional or were converted to closures). I back-supported in most cases, but it wasn't possible everywhere.

Since this is a breaking change, I'd be interested in opinions on whether we should just drop TypeReference support instead of keeping it around as deprecated. And while we're at it, should we make any other public interface changes?

Fixes#55

[paulofaria]

The main difference between the canonical implementation and ours is the need for type references. If I recall correctly, it was because JS used thunks because it allows a symbol to be used inside a closure before its value is bound. I looked for other implementations for inspiration, like Java. I think that's where I got TypeReference from. When you say TypeReference was deprecated, what exactly do you mean. Maybe I'm misremembering stuff, though?

[NeedleInAJayStack]

The main difference between the canonical implementation and ours is the need for type references. If I recall correctly, it was because JS used thunks because it allows a symbol to be used inside a closure before its value is bound. I looked for other implementations for inspiration, like Java. I think that's where I got TypeReference from.

Oh thanks, that's very good background! In this PR I was able to get all existing GraphQL and Graphiti tests running without TypeReferences after changing the GraphQLObject field property to a closure-based API. A pretty good example of the change can be seen in this comment diff: https://github.com/GraphQLSwift/GraphQL/pull/154/files#diff-a5342873cae3c47e54aedd945d19280fa6c7d03f66ad1f7ab13b16ec79c9acf1L288-L299 I'd be interested in your thoughts and if you see any gaps with that approach!

When you say TypeReference was deprecated, what exactly do you mean.

I marked it with @available(*, deprecated, ...)here. By doing so, it will give compiler warnings to anyone that is using it, but will still compile. I did this back when I thought I would be able to make this PR a minor version bump, but if it requires a major it seems like we maybe ought to just delete it.

Also, sorry - I know this is a humongous pull request. Unfortunately all these features use each other in their tests so it was hard to decouple them into separate pull requests.

[paulofaria]

How would cyclical or recursive references be defined without TypeReference? Can you point me to examples?

About the PR size, no worries, I am famous for such PRs myself. 😂

[NeedleInAJayStack]

How would cyclical or recursive references be defined without TypeReference? Can you point me to examples?

Sure, here's an example of a recursive type with TypeReference removed. This works out-of-the-box because CharacterInterface is a global variable.

For non-globals, you can do hit the issue of referencing a variable in a closure before it is created. This is solved by just creating the GraphQL type with empty fields, and then setting the fields closure in a subsequent line. This function contains a test schema that uses this approach on a recursive type. Note that this has the added benefit of allowing the user to avoid the memory cycle caused by recursive types.

Downstream in Graphiti, this then works exactly as you'd expect, where type references are no longer necessary to handle creation ordering/etc

[paulofaria]

Oh, that's great news. I'm not sure if SSWG has any guidelines regarding deprecation. I imagine that as long as we follow SEMVER we're fine. So, if we're going to bump to a major version anyway, I'd say let's remove it altogether. Maybe just add some documentation somewhere explaining how to deal with its absence like you just explained.

[NeedleInAJayStack]

I'm not sure if SSWG has any guidelines regarding deprecation.

I looked through the docs here, but it doesn't appear they have any deprecation requirements, so it seems like we're just okay bumping major.

just add some documentation somewhere explaining how to deal with its absence like you just explained.

Great point. I've added a MIGRATION.md file and linked it from the README, and I've removed GraphQLTypeReference and all usage.

You mind giving it one more quick look? Thanks for the review!

[paulofaria]

Perfect! Awesome work, man! 😄

[NeedleInAJayStack]

This adds full Schema Definition Language (SDL) support to this package, including:

• Printing SDLs for existing schemas
• Building new schemas directly from SDLs
• Using SDL to extend already-defined schemas
• Validating SDL schemas

It does so primarily by porting graphql-js.

Unfortunately, this is a breaking change, even though I worked hard to keep the breakages to a minimum. Notably:

• TypeReference was deprecated. Almost all existing usage is back-supported, but recursively defined types can be problematic. Most of users won't be impacted after we update Graphiti.
• Some public Definition type properties have changed slightly (become optional or were converted to closures). I back-supported in most cases, but it wasn't possible everywhere.

Since this is a breaking change, I'd be interested in opinions on whether we should just drop TypeReference support instead of keeping it around as deprecated. And while we're at it, should we make any other public interface changes?

Fixes#55

[paulofaria]

The main difference between the canonical implementation and ours is the need for type references. If I recall correctly, it was because JS used thunks because it allows a symbol to be used inside a closure before its value is bound. I looked for other implementations for inspiration, like Java. I think that's where I got TypeReference from. When you say TypeReference was deprecated, what exactly do you mean. Maybe I'm misremembering stuff, though?

[NeedleInAJayStack]

The main difference between the canonical implementation and ours is the need for type references. If I recall correctly, it was because JS used thunks because it allows a symbol to be used inside a closure before its value is bound. I looked for other implementations for inspiration, like Java. I think that's where I got TypeReference from.

Oh thanks, that's very good background! In this PR I was able to get all existing GraphQL and Graphiti tests running without TypeReferences after changing the GraphQLObject field property to a closure-based API. A pretty good example of the change can be seen in this comment diff: https://github.com/GraphQLSwift/GraphQL/pull/154/files#diff-a5342873cae3c47e54aedd945d19280fa6c7d03f66ad1f7ab13b16ec79c9acf1L288-L299 I'd be interested in your thoughts and if you see any gaps with that approach!

When you say TypeReference was deprecated, what exactly do you mean.

I marked it with @available(*, deprecated, ...)here. By doing so, it will give compiler warnings to anyone that is using it, but will still compile. I did this back when I thought I would be able to make this PR a minor version bump, but if it requires a major it seems like we maybe ought to just delete it.

Also, sorry - I know this is a humongous pull request. Unfortunately all these features use each other in their tests so it was hard to decouple them into separate pull requests.

[paulofaria]

How would cyclical or recursive references be defined without TypeReference? Can you point me to examples?

About the PR size, no worries, I am famous for such PRs myself. 😂

[NeedleInAJayStack]

How would cyclical or recursive references be defined without TypeReference? Can you point me to examples?

Sure, here's an example of a recursive type with TypeReference removed. This works out-of-the-box because CharacterInterface is a global variable.

For non-globals, you can do hit the issue of referencing a variable in a closure before it is created. This is solved by just creating the GraphQL type with empty fields, and then setting the fields closure in a subsequent line. This function contains a test schema that uses this approach on a recursive type. Note that this has the added benefit of allowing the user to avoid the memory cycle caused by recursive types.

Downstream in Graphiti, this then works exactly as you'd expect, where type references are no longer necessary to handle creation ordering/etc

[paulofaria]

Oh, that's great news. I'm not sure if SSWG has any guidelines regarding deprecation. I imagine that as long as we follow SEMVER we're fine. So, if we're going to bump to a major version anyway, I'd say let's remove it altogether. Maybe just add some documentation somewhere explaining how to deal with its absence like you just explained.

[NeedleInAJayStack]

I'm not sure if SSWG has any guidelines regarding deprecation.

I looked through the docs here, but it doesn't appear they have any deprecation requirements, so it seems like we're just okay bumping major.

just add some documentation somewhere explaining how to deal with its absence like you just explained.

Great point. I've added a MIGRATION.md file and linked it from the README, and I've removed GraphQLTypeReference and all usage.

You mind giving it one more quick look? Thanks for the review!

[paulofaria]

Perfect! Awesome work, man! 😄

[NeedleInAJayStack]

This adds full Schema Definition Language (SDL) support to this package, including:

• Printing SDLs for existing schemas
• Building new schemas directly from SDLs
• Using SDL to extend already-defined schemas
• Validating SDL schemas

It does so primarily by porting graphql-js.

Unfortunately, this is a breaking change, even though I worked hard to keep the breakages to a minimum. Notably:

• TypeReference was deprecated. Almost all existing usage is back-supported, but recursively defined types can be problematic. Most of users won't be impacted after we update Graphiti.
• Some public Definition type properties have changed slightly (become optional or were converted to closures). I back-supported in most cases, but it wasn't possible everywhere.

Since this is a breaking change, I'd be interested in opinions on whether we should just drop TypeReference support instead of keeping it around as deprecated. And while we're at it, should we make any other public interface changes?

Fixes#55

[paulofaria]

The main difference between the canonical implementation and ours is the need for type references. If I recall correctly, it was because JS used thunks because it allows a symbol to be used inside a closure before its value is bound. I looked for other implementations for inspiration, like Java. I think that's where I got TypeReference from. When you say TypeReference was deprecated, what exactly do you mean. Maybe I'm misremembering stuff, though?

[NeedleInAJayStack]

The main difference between the canonical implementation and ours is the need for type references. If I recall correctly, it was because JS used thunks because it allows a symbol to be used inside a closure before its value is bound. I looked for other implementations for inspiration, like Java. I think that's where I got TypeReference from.

Oh thanks, that's very good background! In this PR I was able to get all existing GraphQL and Graphiti tests running without TypeReferences after changing the GraphQLObject field property to a closure-based API. A pretty good example of the change can be seen in this comment diff: https://github.com/GraphQLSwift/GraphQL/pull/154/files#diff-a5342873cae3c47e54aedd945d19280fa6c7d03f66ad1f7ab13b16ec79c9acf1L288-L299 I'd be interested in your thoughts and if you see any gaps with that approach!

When you say TypeReference was deprecated, what exactly do you mean.

I marked it with @available(*, deprecated, ...)here. By doing so, it will give compiler warnings to anyone that is using it, but will still compile. I did this back when I thought I would be able to make this PR a minor version bump, but if it requires a major it seems like we maybe ought to just delete it.

Also, sorry - I know this is a humongous pull request. Unfortunately all these features use each other in their tests so it was hard to decouple them into separate pull requests.

[paulofaria]

How would cyclical or recursive references be defined without TypeReference? Can you point me to examples?

About the PR size, no worries, I am famous for such PRs myself. 😂

[NeedleInAJayStack]

How would cyclical or recursive references be defined without TypeReference? Can you point me to examples?

Sure, here's an example of a recursive type with TypeReference removed. This works out-of-the-box because CharacterInterface is a global variable.

For non-globals, you can do hit the issue of referencing a variable in a closure before it is created. This is solved by just creating the GraphQL type with empty fields, and then setting the fields closure in a subsequent line. This function contains a test schema that uses this approach on a recursive type. Note that this has the added benefit of allowing the user to avoid the memory cycle caused by recursive types.

Downstream in Graphiti, this then works exactly as you'd expect, where type references are no longer necessary to handle creation ordering/etc

[paulofaria]

Oh, that's great news. I'm not sure if SSWG has any guidelines regarding deprecation. I imagine that as long as we follow SEMVER we're fine. So, if we're going to bump to a major version anyway, I'd say let's remove it altogether. Maybe just add some documentation somewhere explaining how to deal with its absence like you just explained.

[NeedleInAJayStack]

I'm not sure if SSWG has any guidelines regarding deprecation.

I looked through the docs here, but it doesn't appear they have any deprecation requirements, so it seems like we're just okay bumping major.

just add some documentation somewhere explaining how to deal with its absence like you just explained.

Great point. I've added a MIGRATION.md file and linked it from the README, and I've removed GraphQLTypeReference and all usage.

You mind giving it one more quick look? Thanks for the review!

[paulofaria]

Perfect! Awesome work, man! 😄

[NeedleInAJayStack]

This adds full Schema Definition Language (SDL) support to this package, including:

• Printing SDLs for existing schemas
• Building new schemas directly from SDLs
• Using SDL to extend already-defined schemas
• Validating SDL schemas

It does so primarily by porting graphql-js.

Unfortunately, this is a breaking change, even though I worked hard to keep the breakages to a minimum. Notably:

• TypeReference was deprecated. Almost all existing usage is back-supported, but recursively defined types can be problematic. Most of users won't be impacted after we update Graphiti.
• Some public Definition type properties have changed slightly (become optional or were converted to closures). I back-supported in most cases, but it wasn't possible everywhere.

Since this is a breaking change, I'd be interested in opinions on whether we should just drop TypeReference support instead of keeping it around as deprecated. And while we're at it, should we make any other public interface changes?

Fixes#55

[paulofaria]

The main difference between the canonical implementation and ours is the need for type references. If I recall correctly, it was because JS used thunks because it allows a symbol to be used inside a closure before its value is bound. I looked for other implementations for inspiration, like Java. I think that's where I got TypeReference from. When you say TypeReference was deprecated, what exactly do you mean. Maybe I'm misremembering stuff, though?

[NeedleInAJayStack]

The main difference between the canonical implementation and ours is the need for type references. If I recall correctly, it was because JS used thunks because it allows a symbol to be used inside a closure before its value is bound. I looked for other implementations for inspiration, like Java. I think that's where I got TypeReference from.

Oh thanks, that's very good background! In this PR I was able to get all existing GraphQL and Graphiti tests running without TypeReferences after changing the GraphQLObject field property to a closure-based API. A pretty good example of the change can be seen in this comment diff: https://github.com/GraphQLSwift/GraphQL/pull/154/files#diff-a5342873cae3c47e54aedd945d19280fa6c7d03f66ad1f7ab13b16ec79c9acf1L288-L299 I'd be interested in your thoughts and if you see any gaps with that approach!

When you say TypeReference was deprecated, what exactly do you mean.

I marked it with @available(*, deprecated, ...)here. By doing so, it will give compiler warnings to anyone that is using it, but will still compile. I did this back when I thought I would be able to make this PR a minor version bump, but if it requires a major it seems like we maybe ought to just delete it.

Also, sorry - I know this is a humongous pull request. Unfortunately all these features use each other in their tests so it was hard to decouple them into separate pull requests.

[paulofaria]

How would cyclical or recursive references be defined without TypeReference? Can you point me to examples?

About the PR size, no worries, I am famous for such PRs myself. 😂

[NeedleInAJayStack]

How would cyclical or recursive references be defined without TypeReference? Can you point me to examples?

Sure, here's an example of a recursive type with TypeReference removed. This works out-of-the-box because CharacterInterface is a global variable.

For non-globals, you can do hit the issue of referencing a variable in a closure before it is created. This is solved by just creating the GraphQL type with empty fields, and then setting the fields closure in a subsequent line. This function contains a test schema that uses this approach on a recursive type. Note that this has the added benefit of allowing the user to avoid the memory cycle caused by recursive types.

Downstream in Graphiti, this then works exactly as you'd expect, where type references are no longer necessary to handle creation ordering/etc

[paulofaria]

Oh, that's great news. I'm not sure if SSWG has any guidelines regarding deprecation. I imagine that as long as we follow SEMVER we're fine. So, if we're going to bump to a major version anyway, I'd say let's remove it altogether. Maybe just add some documentation somewhere explaining how to deal with its absence like you just explained.

[NeedleInAJayStack]

I'm not sure if SSWG has any guidelines regarding deprecation.

I looked through the docs here, but it doesn't appear they have any deprecation requirements, so it seems like we're just okay bumping major.

just add some documentation somewhere explaining how to deal with its absence like you just explained.

Great point. I've added a MIGRATION.md file and linked it from the README, and I've removed GraphQLTypeReference and all usage.

You mind giving it one more quick look? Thanks for the review!

[paulofaria]

Perfect! Awesome work, man! 😄

[NeedleInAJayStack]

This adds full Schema Definition Language (SDL) support to this package, including:

• Printing SDLs for existing schemas
• Building new schemas directly from SDLs
• Using SDL to extend already-defined schemas
• Validating SDL schemas

It does so primarily by porting graphql-js.

Unfortunately, this is a breaking change, even though I worked hard to keep the breakages to a minimum. Notably:

• TypeReference was deprecated. Almost all existing usage is back-supported, but recursively defined types can be problematic. Most of users won't be impacted after we update Graphiti.
• Some public Definition type properties have changed slightly (become optional or were converted to closures). I back-supported in most cases, but it wasn't possible everywhere.

Since this is a breaking change, I'd be interested in opinions on whether we should just drop TypeReference support instead of keeping it around as deprecated. And while we're at it, should we make any other public interface changes?

Fixes#55

[paulofaria]

The main difference between the canonical implementation and ours is the need for type references. If I recall correctly, it was because JS used thunks because it allows a symbol to be used inside a closure before its value is bound. I looked for other implementations for inspiration, like Java. I think that's where I got TypeReference from. When you say TypeReference was deprecated, what exactly do you mean. Maybe I'm misremembering stuff, though?

[NeedleInAJayStack]

The main difference between the canonical implementation and ours is the need for type references. If I recall correctly, it was because JS used thunks because it allows a symbol to be used inside a closure before its value is bound. I looked for other implementations for inspiration, like Java. I think that's where I got TypeReference from.

Oh thanks, that's very good background! In this PR I was able to get all existing GraphQL and Graphiti tests running without TypeReferences after changing the GraphQLObject field property to a closure-based API. A pretty good example of the change can be seen in this comment diff: https://github.com/GraphQLSwift/GraphQL/pull/154/files#diff-a5342873cae3c47e54aedd945d19280fa6c7d03f66ad1f7ab13b16ec79c9acf1L288-L299 I'd be interested in your thoughts and if you see any gaps with that approach!

When you say TypeReference was deprecated, what exactly do you mean.

I marked it with @available(*, deprecated, ...)here. By doing so, it will give compiler warnings to anyone that is using it, but will still compile. I did this back when I thought I would be able to make this PR a minor version bump, but if it requires a major it seems like we maybe ought to just delete it.

Also, sorry - I know this is a humongous pull request. Unfortunately all these features use each other in their tests so it was hard to decouple them into separate pull requests.

[paulofaria]

How would cyclical or recursive references be defined without TypeReference? Can you point me to examples?

About the PR size, no worries, I am famous for such PRs myself. 😂

[NeedleInAJayStack]

How would cyclical or recursive references be defined without TypeReference? Can you point me to examples?

Sure, here's an example of a recursive type with TypeReference removed. This works out-of-the-box because CharacterInterface is a global variable.

For non-globals, you can do hit the issue of referencing a variable in a closure before it is created. This is solved by just creating the GraphQL type with empty fields, and then setting the fields closure in a subsequent line. This function contains a test schema that uses this approach on a recursive type. Note that this has the added benefit of allowing the user to avoid the memory cycle caused by recursive types.

Downstream in Graphiti, this then works exactly as you'd expect, where type references are no longer necessary to handle creation ordering/etc

[paulofaria]

Oh, that's great news. I'm not sure if SSWG has any guidelines regarding deprecation. I imagine that as long as we follow SEMVER we're fine. So, if we're going to bump to a major version anyway, I'd say let's remove it altogether. Maybe just add some documentation somewhere explaining how to deal with its absence like you just explained.

[NeedleInAJayStack]

I'm not sure if SSWG has any guidelines regarding deprecation.

I looked through the docs here, but it doesn't appear they have any deprecation requirements, so it seems like we're just okay bumping major.

just add some documentation somewhere explaining how to deal with its absence like you just explained.

Great point. I've added a MIGRATION.md file and linked it from the README, and I've removed GraphQLTypeReference and all usage.

You mind giving it one more quick look? Thanks for the review!

[paulofaria]

Perfect! Awesome work, man! 😄