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

Comments on InputUnion RFC #817

Merged
merged 5 commits into from
Sep 2, 2021

Conversation

ericvergnaud
Copy link
Contributor

Hi, thanks for this extensive and in-depth work.

I will try to keep my comments concise (which might make them sound agressive, apologies if that's the case, it's definitely not the intent)

This PR touches one of the most challengeable founding philosophical choices of GraphQL: having 2 different type systems for input and output.

I understand that the sender -> receiver relation is an unbalanced one: the sender has obligations, the receiver has options.

However:

  • ALL GraphQL users use programming languages (Javascript, Java, C#, Python...) which make no difference between input and output types
  • I actually don't know any other protocol that does that (is there one?)
  • having 2 different type systems for input and output solves (does it?) an implementer problem, not a user problem

The lack of polymorphism on input is only a side-effect of the aformentioned original choice.
In an unreal world, rather than tweak GraphQL to fix that, it would be time for GraphQL2, unifying input and output types (amongst other improvements).

That's very unlikely to happen, but saying so helps forming an opinion on the various proposals:

  • adding yet another polymorphic construct only available on input 'smells' like increasing confusion
  • it would increase the gap between input and output type systems, rather than reduce it

From there, I think proposal #5 @OneOf is the most useful one:

  • it acts as a constraint on existing type constructs rather than yet another type construct
  • it expresses the required behavior much better than proposal Fix typo #7

!!! IMPORTANT !!!

Please Read https://github.com/graphql/graphql-spec/blob/master/CONTRIBUTING.md before creating a Pull Request.

Hi, thanks for this extensive and in-depth work.

I will try to keep my comments concise (which might make them sound agressive, apologies if that's the case, it's definitely not the intent)

This PR touches one of the most challengeable founding philosophical choices of GraphQL: having 2 different type systems for input and output.

I understand that the sender -> receiver relation is an unbalanced one: the sender has obligations, the receiver has options.

However:
 - ALL GraphQL users use programming languages (Javascript, Java, C#, Python...) which make no difference between input and output types
 - I actually don't know any other protocol that does that (is there one?)
 - having 2 different type systems for input and output solves (does it?) an implementer problem, not a user problem

The lack of polymorphism on input is only a side-effect of the aformentioned original choice.
In an unreal world, rather than tweak GraphQL to fix that, it would be time for GraphQL2, unifying input and output types (amongst other improvements).

That's very unlikely to happen, but saying so helps forming an opinion on the various proposals:
 - adding yet another polymorphic construct only available on input 'smells' like increasing confusion 
 - it would increase the gap between input and output type systems, rather than reduce it

From there, I think proposal graphql#5 @OneOf is the most useful one:
 - it acts as a constraint on existing type constructs rather than yet another type construct
 - it expresses the required behavior much better than proposal graphql#7
Hi, thanks for this extensive and in-depth work.

I will try to keep my comments concise (which might make them sound agressive, apologies if that's the case, it's definitely not the intent)

This PR touches one of the most challengeable founding philosophical choices of GraphQL: having 2 different type systems for input and output.

I understand that the sender -> receiver relation is an unbalanced one: the sender has obligations, the receiver has options.

However:
 - ALL GraphQL users use programming languages (Javascript, Java, C#, Python...) which make no difference between input and output types
 - I actually don't know any other protocol that does that (is there one?)
 - having 2 different type systems for input and output solves (does it?) an implementer problem, not a user problem

The lack of polymorphism on input is only a side-effect of the aformentioned original choice.
In an unreal world, rather than tweak GraphQL to fix that, it would be time for GraphQL2, unifying input and output types (amongst other improvements).

That's very unlikely to happen, but saying so helps forming an opinion on the various proposals:
 - adding yet another polymorphic construct only available on input 'smells' like increasing confusion 
 - it would increase the gap between input and output type systems, rather than reduce it

From there, I think proposal graphql#5 @OneOf is the most useful one:
 - it acts as a constraint on existing type constructs rather than yet another type construct
 - it expresses the required behavior much better than proposal graphql#7
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Jan 28, 2021

CLA Signed

The committers are authorized under a signed CLA.

@benjie
Copy link
Member

benjie commented Jan 28, 2021

Solution 5 is the solution we’re currently pushing forwards; that was the conclusion of the latest input unions working group (which you can watch on the GraphQL Foundation’s YouTube) 👍

If you’d like this feedback factored into the RFC document to help add more reasoning behind solution 5, it should be done in the existing form; for example adding a goal that “No additional types are introduced” and then evaluating it against the various proposals.

Base automatically changed from master to main February 3, 2021 04:50
@leebyron leebyron added the 📣 RFC document PR creates or changes document inside "rfc" folder label Apr 6, 2021
@leebyron leebyron force-pushed the main branch 4 times, most recently from e5d241d to 6c81ed8 Compare April 23, 2021 19:15
@leebyron
Copy link
Collaborator

leebyron commented Sep 2, 2021

Merging since this is RFC Doc and I'm about to move them

@leebyron leebyron merged commit 04cd832 into graphql:main Sep 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📣 RFC document PR creates or changes document inside "rfc" folder
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants