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

feat: add QueryVariableLiterals #1009

Merged
merged 1 commit into from
Aug 23, 2024
Merged

feat: add QueryVariableLiterals #1009

merged 1 commit into from
Aug 23, 2024

Conversation

obmarg
Copy link
Owner

@obmarg obmarg commented Aug 22, 2024

Why are we making this change?

In #1000 & #1004 a user is wanting to build a query that doesn't make use of variables without hardcoding the values of arguments. This is to make use of some shopify functionality that requires you to submit a query without variables.

My initial instinct was that this should be done externally to cynic, but I did plan on documenting how. #1007 was added to cater to this - it contained an example that used the graphql-query crate to post-process a cynic query and inline all of the variables into arguments in the query.

However, when writing this I discovered it's not quite as smooth as I'd hoped: you can't just dump the variables into JSON and put those into the query because enums are serialised as strings in JSON but enum format in GraphQL. Being unable to use JSON means you're also unable to dynamically look up variables from their names - you have to hardcode every variable that you want to substitute. Which is pretty crap.

What effects does this change have?

This PR introduces a new QueryVariableLiterals trait & derive that aims to help with this: it exposes a single function get that can be passed the name of a variable and it will return the appropriate InputLiteral for that variable.

This can be used to make a graphql-query based transform much nicer, but it should also make it easy enough to add support for variable inlining into cynic itself.

cynic/src/variables.rs Outdated Show resolved Hide resolved
@obmarg
Copy link
Owner Author

obmarg commented Aug 22, 2024

Need to write some tests

Copy link

netlify bot commented Aug 22, 2024

Deploy Preview for cynic-querygen-web canceled.

Name Link
🔨 Latest commit 3b97142
🔍 Latest deploy log https://app.netlify.com/sites/cynic-querygen-web/deploys/66c8dd5be9694a00084fcd95

@obmarg obmarg force-pushed the obmarg/mxpnrvyorsqv branch 2 times, most recently from 3a5e43a to b2cb76b Compare August 23, 2024 18:08
@obmarg obmarg changed the title feat: add QueryVariables::get_literal feat: add QueryVariableLiterals Aug 23, 2024
@obmarg obmarg marked this pull request as ready for review August 23, 2024 18:17
@obmarg obmarg enabled auto-merge (squash) August 23, 2024 19:00
@obmarg obmarg disabled auto-merge August 23, 2024 19:00
@obmarg obmarg enabled auto-merge (squash) August 23, 2024 19:00
@obmarg obmarg merged commit a49710c into main Aug 23, 2024
6 checks passed
@obmarg obmarg deleted the obmarg/mxpnrvyorsqv branch August 23, 2024 19:07
@cynic-releaser cynic-releaser bot mentioned this pull request Aug 23, 2024
@Bobrokus
Copy link
Contributor

Thank you so so much! 🚀

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.

2 participants