-
Notifications
You must be signed in to change notification settings - Fork 122
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
Conditionally skip REST calls #299
Comments
(side note: tried to add the |
I’d be happy to review a design proposal for how this would work. My cautious guess is that it will be pretty difficult to implement something clean for this functionality. — You’re effectively trying to squeeze in a feature that doesn’t exist in GraphQL, and you’re doing it to control functionality of an extension to GraphQL Patches on patches = pretty messy. If you can find a clean way to do this let us know! |
How do you feel about adding a |
I worry that we have to return with an object that matches the provided If skip is true, then the Maybe I’m over-cautious though. |
I think you're talking about the I would expect the Here's an example: schema {
query: Query
}
type Query {
getAllAuthors: [Author!]!
}
type Author {
# Imagine that some authors do not have ID set.
id: ID
name: String!
# This is the field fetched via REST link.
#
# Schema defines this as optional,
# since authors without IDs can't fetch their books.
bookLink: BookLink
}
type Book {
id: ID!
authorID: ID!
title: String!
}
type BookLink {
books: [Book!]!
}
query GetAuthors {
getAllAuthors {
id @export(as: "authorID")
name
bookLink
@rest(
type: "BookLink" # Type matches the schema type
path: "/booksByAuthor/{exportVariables.authorID}"
skip:
) {
books {
title
}
}
}
} Defining this type as non-optional would just mean that the schema is wrong. Even if the So I think the |
Another thing to point out is that the apollo client Extending the above example, if we had a by-ID author fetch: query GetAuthor($id: ID!) {
getAuthor(input: {id: $id}) {
name
}
} And we used this in a query where the ID might be null or undefined: const router = useRouter()
const id: string | undefined = router.query["id"]
const { data } = useQuery(GetAuthorDocument, {
variables: {
id
},
skip: !id,
}) This fails to typecheck, because the expected TS type of const idOrEmpty: string | undefined = router.query["id"]
const id = idOrEmpty ?? "" // default to string of length 0 to satisfy type constraint
const { data } = useQuery(GetAuthorDocument, {
variables: {
id
},
skip: !id,
}) for it to type check correctly. So I think there's precedent for a slight loss of ergonomics when using a |
I see where you’re coming from, and if this were just in TypeScript I’d tend to agree. This optional arg actually has to apply across two type systems which I think makes it more complicated. You’re welcome to submit a non-breaking PR for this feature. And I’d be happy to take a look. |
thanks for your consideration. I'll submit a PR. |
(accidentally hit enter, sorry!)
I have a nested
@rest()
field (e.g.author
) in my graphql query that utilizes an@export()
ed field (e.g.authorID
) to make a query. Sometimes, the exported field is not set. In those cases, I don't want to make the REST call at all. (In this example, if theauthorID
is not present, calling/get_author
doesn't make sense.)A way to skip the query conditionally would fix this.
Same as #259.
The text was updated successfully, but these errors were encountered: