Fix cache policy calculation #2197
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Awesome - I am working on some cache related features this week and had noticed the same concern. Will probably merge Monday! |
glasser
reviewed
Jan 22, 2019
| @@ -156,7 +156,7 @@ export class CacheControlExtension<TContext = any> | |||
| let scope: CacheScope = CacheScope.Public; | |||
|
|
|||
| for (const hint of this.hints.values()) { | |||
| if (hint.maxAge) { | |||
| if (hint.maxAge !== undefined) { | |||
| lowestMaxAge = lowestMaxAge | |||
There was a problem hiding this comment.
Should the check here (before the ?) also be against undefined?
There was a problem hiding this comment.
Yes, you are right. Didn't check that case in the tests. 😕
Just fixed it and adjusted the test.
|
Thanks! |
glasser
added a commit
that referenced
this pull request
Jan 22, 2019
This is consistent with the old engineproxy interpretation of cache hints. We special-case scalar fields to inherit their parent field's hints for simplicity (so you don't have to hint every scalar field in a hinted object), but when the parent field is non-root that inherited hint gets defaultMaxAge applied to it. When the parent field is the root, that inherited hint doesn't get defaultMaxAge applied because we don't run willResolveField for the root query. Includes a CHANGELOG update for #2197.
glasser
added a commit
that referenced
this pull request
Jan 22, 2019
This is consistent with the old engineproxy interpretation of cache hints. We special-case scalar fields to inherit their parent field's hints for simplicity (so you don't have to hint every scalar field in a hinted object), but when the parent field is non-root that inherited hint gets defaultMaxAge applied to it. When the parent field is the root, that inherited hint doesn't get defaultMaxAge applied because we don't run willResolveField for the root query. Includes a CHANGELOG update for #2197.
glasser
added a commit
that referenced
this pull request
Jan 24, 2019
This is consistent with the old engineproxy interpretation of cache hints. We special-case scalar fields to inherit their parent field's hints for simplicity (so you don't have to hint every scalar field in a hinted object), but when the parent field is non-root that inherited hint gets defaultMaxAge applied to it. When the parent field is the root, that inherited hint doesn't get defaultMaxAge applied because we don't run willResolveField for the root query. Includes a CHANGELOG update for #2197.
abernix
pushed a commit
that referenced
this pull request
Jan 25, 2019
…2210) This is consistent with the old engineproxy interpretation of cache hints. We special-case scalar fields to inherit their parent field's hints for simplicity (so you don't have to hint every scalar field in a hinted object), but when the parent field is non-root that inherited hint gets defaultMaxAge applied to it. When the parent field is the root, that inherited hint doesn't get defaultMaxAge applied because we don't run willResolveField for the root query. Includes a CHANGELOG update for #2197.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes apollographql/apollo-cache-control#11
TODO: