Skip to content
This repository has been archived by the owner on Apr 13, 2023. It is now read-only.

fix: ssr should support network-only & cache-and-network fetchPolicies. disabled ssr queries were still issuing requests #3435

Merged
merged 7 commits into from Sep 4, 2019

Conversation

mikebm
Copy link
Contributor

@mikebm mikebm commented Aug 30, 2019

Related to issues discovered in topic #3338
Might be related to #3390

This PR addresses 2 issues and some cleanup.
Issue 1) network-only fetchPolicy during a SSR was resulting in the component being stuck in a loading state.

Issue 2) Disabling a query using ssr:false would result in the query still being executed on the server even though it was never used.

Cleanup:
GetOptions returns new objects with each call. This PR will reduce the amount of times it is called and thus reduces some garbage. In addition, with the changes on where the SSR observable was initialize, fetchData was greatly reduced in needed code.

…s. disabling ssr queries were still issuing the requests
@mikebm mikebm changed the title fix: ssr should support network-only & cache-and-network fetchPolicies. disabling ssr queries were still issuing the requests fix: ssr should support network-only & cache-and-network fetchPolicies. disabled ssr queries were still issuing the requests Aug 30, 2019
@mikebm mikebm changed the title fix: ssr should support network-only & cache-and-network fetchPolicies. disabled ssr queries were still issuing the requests fix: ssr should support network-only & cache-and-network fetchPolicies. disabled ssr queries were still issuing requests Aug 30, 2019
}
}

private updateObservableQuery() {
// If we skipped initially, we may not have yet created the observable
if (!this.currentObservable.query) {
this.initializeObservableQuery();
return;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this was added as an optimization. updating the options doesn't need to happen because it was just initialized.

Copy link
Member

@hwillson hwillson 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 awesome @mikebm! Thanks very much for working on this! 🙇

@hwillson hwillson merged commit 331a68c into apollographql:master Sep 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants