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

Bump watershed to 2030? #322

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Bump watershed to 2030? #322

wants to merge 1 commit into from

Conversation

benjie
Copy link
Member

@benjie benjie commented Jan 23, 2025

We can't really have a watershed that's before the spec itself is ratified. I've allowed us ~2 years to get the spec out, and then ~3 years for people to make the relevant changes. Though many of the prominent GraphQL server frameworks may already support this, there are likely a lot of people stuck on legacy servers out there, and having their clients stop working when they update would not be pleasant.

Also (and for a separate PR)... I wonder if we should pick a time other than midnight on New Years Day as the day that this kicks in... 🤔

cc @abernix @balshor @benjie @deinok @ErikWittern @jaydenseric @michaelstaib @mike-marcacci @mmatsa @Shane32 @sjparsons @spawnia @sungam3r @Touchstone117 @enisdenjo @JoviDeCroock

@martinbonnin
Copy link
Contributor

2030 feels really far. I'm a bit wary it may send the wrong signal that this is never going to happen. What about moving it 2 years (2027)?

We can always push the date again in 2 years time if needed so that preserves out option value.

@benjie
Copy link
Member Author

benjie commented Jan 23, 2025

What about moving it 2 years (2027)?

I think it should be at least 2 years after the spec is ratified. Which even if we get the spec out this year means 2028 would be the earliest I'd be comfortable with. I added a bit of buffer time to both sides.

2030 feels really far. I'm a bit wary it may send the wrong signal that this is never going to happen.

GraphQL has been out for 10 years this year, and still doesn't have a ratified GraphQL-over-HTTP spec, so 5 years doesn't seem that far away to me 😅

What specifically are you concerned about never happening? The main watershed change is the ability for a client to not specify application/json in its Accept header since all servers have to support application/graphql-response+json? It's already recommended that servers support application/graphql-response+json now, so I don't think trimming those few characters from the header is that big of a deal?

@martinbonnin
Copy link
Contributor

GraphQL has been out for 10 years this year, and still doesn't have a ratified GraphQL-over-HTTP spec

"Past performance is no guarantee of future results." Let's try/hope to get this out faster?

I don't think trimming those few characters from the header is that big of a deal?

It's not that much about the extra bytes than the extra cognitive load. Being able to tell everyone "this is how it works from now on" has lots of educational value. If not, the message gets complicated.

What's more, supporting application/graphql-response+json shouldn't be too much of an ask for server maintainers out there, or is it? The faster we get it out after the spec is ratified, the better IMO.

@Shane32
Copy link
Contributor

Shane32 commented Jan 23, 2025

I think it should depend on what’s commonly in use. For example, if servers already implement the draft spec, the date need not change but rather when the date passes we simple omit the old pre-watershed language.

In general I’m in favor of pushing it out a couple years, if need be; 2030 is a long way off.

@gmac
Copy link

gmac commented Jan 23, 2025

FWIW – https://shopify.dev/changelog/graphql-over-http. We've rolled this out for most major APIs (albeit, not Storefront API which has latent mobile app traffic). There is some complexity in rollout given that many clients already request application/graphql-response+json and don't get that format back. Shopify has versioned APIs though, so I was able to hook the response cutover to an API version. Providers without this sort of versioning strategy do not have this luxury. That said, it'll be equally hard for them to do the cutoff in 5 years as it is now.

IMO, just ink it.

@Shane32
Copy link
Contributor

Shane32 commented Jan 23, 2025

Just for reference, GraphQL.NET 7.1.0+, released 9/3/2022, uses application/graphql-response+json; charset=utf-8 as the default response content type regardless of system date (the default is configurable). In addition, it also supports an Accept header of application/json or the obsolete application/graphql+json with any charset supported by the operating system, providing a corresponding response.

How does the reference server function? Do typical clients in use today omit the Accept header and require an application/json response content type to function correctly?

While I'm fine pushing the watershed period back a bit, I would also support omitting the watershed immediately. After all, this is a draft spec. The spec as written would still require support of application/json responses, so any client that does send a specific Accept header should still function the same.

@JoviDeCroock
Copy link
Member

JoviDeCroock commented Jan 23, 2025

I agree here with @Shane32

I personally don't see the point in pushing this back. The spec is in draft mode so to publish it with this caveat feels odd for me personally. All servers will support application/json for the foreseeable future, I don't have any doubt about that honestly. Most GraphQL clients that I've seen send Accept: application/json, application/graphql+json, the one exception here that I know of is GraphiQL.

A big point people make about GraphQL is that the HTTP spec disregards existing HTTP standards like status-codes, ... well omitting application/json and formally having this in the HTTP spec feels a step in the right direction to point these people at an official resource saying it is in fact changing.

The legacy is in there, the date won't matter for most implementors, what implementors care about is supporting all the cases. Which as far as I can see, we aren't twisting any arms by just leaving this be.

I think we historically have been reluctant in changing things but as far as I see that hasn't really helped the case of the community moving towards these goals. I think giving a non-breaking push in the back like this actually helps.

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.

7 participants