add alwaysSendQuery option for PersistedLink #1285
Conversation
ghost
added
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions. |
stale
bot
added
stale
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions. |
stale
bot
added
the
stale
|
@alexandcote this PR could use a look if you have some time after the holidays, thanks! |
stale
bot
removed
the
stale
|
First of all, Thanks @dsanders11 for your contribution! Sorry for the delay... I will take care of reviewing this PR and assigning other folks. In the meantime, do you think you could rebase your PR on master? Thanks a lot 💯 |
| @@ -46,7 +50,7 @@ export class PersistedLink extends ApolloLink { | |||
|
|
|||
| operation.setContext({ | |||
| http: { | |||
| includeQuery: false, | |||
| includeQuery: alwaysSendQuery, | |||
There was a problem hiding this comment.
I'd probably change this option to match the context field name (includeQuery). The option name still bugs me a bit, because it technically applies to more than just queries, but Apollo uses "query" and "GraphQL document" interchangeably in a lot of places so it's probably fine to match.
There was a problem hiding this comment.
Whatever we name it, it should also be documented in the readme like idFromOperation is.
There was a problem hiding this comment.
Renamed to alwaysIncludeQuery (I think that's what you were suggesting, @lemonmade?). Originally it was "send" to align verbiage wise with the Koa middleware which has a SendAlways option.
@gmcgibbon, documented it in the readme.
|
@alexandcote, rebased and addressed code review comments. |
| @@ -32,7 +33,10 @@ export class PersistedLink extends ApolloLink { | |||
| } | |||
|
|
|||
| return new Observable(observer => { | |||
| const {idFromOperation = defaultIdFromOperation} = this.options; | |||
| const { | |||
| alwaysIncludeQuery = false, | |||
There was a problem hiding this comment.
I was proposing just includeQuery, since this directly maps to an option with that name in Apollo.
There was a problem hiding this comment.
That name seems confusing to me? It loses the explicitness that the query will always be included. I'm not sure what's really gained by excluding the word always. Especially since, as I pointed out, the Koa middleware uses always to describe similar behavior.
Description
Adds an option for
createPersistedLinkto always send the query, rather than waiting for the server to ask.Two motivating use-cases:
Development
This is useful for development, as it makes debugging easier when the query is visible in requests.
SSR
It's also useful when doing SSR, as upload link speed isn't a factor there, so always sending the query avoids an extra round-trip with the API server.
Usage
Type of change
Checklist