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

docs: add explanation about GET Vs POST queries #99

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

theodesp
Copy link
Member

@theodesp theodesp commented Mar 12, 2025

#53

@theodesp theodesp requested a review from a team as a code owner March 12, 2025 13:22
@colinmurphy
Copy link
Contributor

@theodesp LGTM 🚀

Should we include anything about caching too. Would there be a reason why someone might use GET over POST (or visa versa) in relation to caching?

colinmurphy
colinmurphy previously approved these changes Mar 12, 2025
colinmurphy
colinmurphy previously approved these changes Mar 13, 2025
Copy link
Contributor

@colinmurphy colinmurphy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @theodesp for adding info about caching.

LGTM 🚀 🚀 🚀

Copy link
Member

@moonmeister moonmeister left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is indeed great info, but it doesn't satisfy #52. This satisfies #53. Let's add this article under the appropriate title but also do #52.

@theodesp
Copy link
Member Author

This is indeed great info, but it doesn't satisfy #52. This satisfies #53. Let's add this article under the appropriate title but also do #52.

Makes sense. I will update the title and provide anotehr explanation document for the queries.

@theodesp theodesp changed the title docs: add explanation about which url to use when fetching data docs: add explanation about GET Vs POST queries Mar 18, 2025
Copy link
Member

@moonmeister moonmeister left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good work. Let's add a brief section on [Automatic] Persisted Queries and how APQs solve the issues with GET requests with links out to Apollo docs or the GraphQL spec (if there is one) where folks can get more info. a link to the how-to could be good too.


Each property is provided as an HTTP query parameter, with values separated by an ampersand (&).

GraphQL requests with variables don't work in GET requests because they need to be parsed as JSON.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not true:

https://graphql-cache.local/graphql?query=query($slug:ID!){post(id:$slug,idType:SLUG,asPreview:false){title,content}}&variables={%22slug%22:%22hello-world%22}

https://arc.net/l/quote/xzprsdhw

| Method | Security | Complexity | Support | Use Case |
|--------------------------|------------------------------------------|-----------------|------------|------------------------|
| POST | More secure, hides query in request body | Complex queries | Production | complex data retrieval |
| GET with Query Parameter | Less secure due to query exposure in URL | Simple queries (no mutations) | Testing | simple data retrieval |
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HTTPS secures URL information only domain is exposed due to DNS and TLS being established. see: https://stackoverflow.com/a/499594

theodesp and others added 3 commits March 18, 2025 18:00
Co-authored-by: Alex Moon <moonmeister@users.noreply.github.com>
Co-authored-by: Alex Moon <moonmeister@users.noreply.github.com>
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.

3 participants