__ strictness breaks postgraphql introspection

[martinheidegger]

With the publication of 0.9.0 which includes #600, postgraphql broke. Postgraphql implemented __id in advance in order to support the customers use case, there is a pending request open here that refers to the validity of it (even though it is not yet implemented). Might it be good to add an exception case if __ is used for __id: GraphQLID in order to not-break that case?

[leebyron]

The __id case is a great example of an introspection field that that we expect could be added into the spec in the future and is the reason the clause in the spec exists.

This is especially important to solve sooner rather than later if __id is added to the spec, as most of the proposals to add it would conflict if fields were already defined to use it. In fact the whole reason for choosing the name __id was that it should not appear in anyone's schema directly.

[leebyron]

If this ends up being a backwards breaking change for enough production services, perhaps we could add in some work around to enable spec-non-compliant behavior (that would also signal disabling related future features). What do folks think of that?

[robzhu]

perhaps we could add in some work around to enable spec-non-compliant behavior

Are you imaging such a flag to mean the entire GraphQL server is non-compliant and signals an "all bets are off" situation or can we be more granular, for example: "introspection is non-compliant" or "directive support is non-compliant due to overriden behavior".

[leebyron]

It wasn't a fully formed thought, but I agree that any spec non-compliance could result in a sort of cascading "all bets are off". Though perhaps if we're granular we can trade for performance rather than trading for correctness?

[leebyron]

It seems like this is breaking too many people - while it remains a strict rule within the spec, I'm going to remove the invariant for now - perhaps we can add it back in a way that's far more ergonomic.

[wincent]

Yeah, maybe we can do a warning then upgrade it to an invariant in a later release.

[leebyron]

Good idea, it looks like that's what GraphQL Ruby did as well