Skip to content

Meta: Use variables in GraphQL #6674

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.

Sign up for GitHub

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

Jump to bottom
Merged
fregante merged 12 commits into from
May 23, 2023
Merged

Meta: Use variables in GraphQL #6674

fregante merged 12 commits into from
May 23, 2023

Conversation

@fregante
Copy link
Member

@fregante fregante commented May 20, 2023 •
edited
Loading

This PR

This is harder than anticipated:

  • you can't include variable data in the JSON and not use them (i.e. no default user/repo)
  • you can't just use variables; each query must ALSO declare them…
- query {
+ query ($owner: String!, $name: String!) {
    repository(owner: $owner, name: $name) {

Test

Extensions suggested:

@fregante fregante added the meta Related to Refined GitHub itself label May 20, 2023
@fregante fregante marked this pull request as draft May 20, 2023 18:44
@yakov116
Copy link
Member

yakov116 commented May 21, 2023

JMHO, on any non simple query I think this lowers readability

@fregante
Copy link
Member Author

fregante commented May 21, 2023

It’s definitely more verbose but there’s no alternative if we want to extract the queries:

Once extracted we’ll have syntax highlighting and potentially some validation, as well as proper types for v4 calls. I think it’s worth it, even if the query ends up being in a different file.

@fregante fregante changed the title Use variables in GraphQL Meta: Use variables in GraphQL May 21, 2023
@fregante fregante marked this pull request as ready for review May 21, 2023 12:44
fregante
fregante commented May 21, 2023
View reviewed changes
Copy link
Member Author

@fregante fregante left a comment

Choose a reason for hiding this comment

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

Everything here was tested. Some features were even restored, some tickets were created.

In a future PR:

  • Extract to .gql files
  • Update every query (I ignored the simple ones without additional variables, although they're still affected by this change)

fregante
fregante commented May 21, 2023
View reviewed changes
Copy link
Member Author

@fregante fregante left a comment •
edited
Loading

Choose a reason for hiding this comment

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

You can see what the usage of .gql files will look like in list-prs-for-file

Missing:

  • TypeScript integration that will help us use the results in the file
  • GraphQL autocomplete in the IDE thanks to the schema
  • Typed variables that make TypeScript complain if one is missing or unused
  • Prettier/formatter for GQL support (we have a .prettierignore file)

source/features/list-prs-for-file.gql
field: UPDATED_AT,
direction: DESC
}
) {
Copy link
Member Author

@fregante fregante May 21, 2023

Choose a reason for hiding this comment

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

Demo extracted GQL file

source/features/list-prs-for-file.tsx
}
`);
debugger;
const {repository} = await api.v4(listPrsForFileQuery, {
Copy link
Member Author

@fregante fregante May 21, 2023

Choose a reason for hiding this comment

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

Demo feature with extracted query

Copy link
Member

@yakov116 yakov116 May 21, 2023

Choose a reason for hiding this comment

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

Nice!

Can we put this in a new pr?

Copy link
Member

@yakov116 yakov116 May 21, 2023

Choose a reason for hiding this comment

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

Never mind just saw your post

@fregante fregante enabled auto-merge (squash) May 23, 2023 18:06
@fregante fregante merged commit f589546 into main May 23, 2023
@fregante fregante deleted the api-variables branch May 23, 2023 18:07
@fregante fregante mentioned this pull request May 23, 2023
Closed
@fregante fregante mentioned this pull request Jun 1, 2023
Closed
Copilot AI mentioned this pull request Jan 4, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@yakov116 yakov116 yakov116 left review comments

Assignees

No one assigned

Labels

meta Related to Refined GitHub itself

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Fix our GraphQL usage (use native variables)

2 participants

@fregante @yakov116