Skip to content
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

apollo-cache-control: consider hintless root fields to be uncached #2210

Merged
merged 2 commits into from
Jan 25, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions packages/apollo-cache-control/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,24 @@
# Changelog

### vNEXT

* Fix cache hints of `maxAge: 0` to mean "uncachable". (#2197)

* Apply `defaultMaxAge` to scalar fields on the root object. (#2210)

### v0.3.0

* Support calculating Cache-Control HTTP headers when used by `[email protected]`.

(There are a number of other 0.3.x releases as well as 0.4.0 with no code
changes due to how the `apollo-server` release process works.)

### v0.2.0

Moved to the `apollo-server` git repository. No code changes. (There are a
number of other 0.2.x releases with no code changes due to how the
`apollo-server` release process works.)

### v0.1.1

* Fix `defaultMaxAge` feature (introduced in 0.1.0) so that `maxAge: 0` overrides the default, as previously documented.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,25 @@ describe('@cacheControl directives', () => {
expect(hints).toContainEqual({ path: ['droid'], maxAge: 0 });
});

it('should set maxAge: 0 and no scope for a top-level scalar field without cache hints', async () => {
const schema = buildSchemaWithCacheControlSupport(`
type Query {
name: String
}
`);

const hints = await collectCacheControlHints(
schema,
`
query {
name
}
`,
);

expect(hints).toContainEqual({ path: ['name'], maxAge: 0 });
});

it('should set maxAge to the default and no scope for a field without cache hints', async () => {
const schema = buildSchemaWithCacheControlSupport(`
type Query {
Expand Down
35 changes: 19 additions & 16 deletions packages/apollo-cache-control/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,26 +75,27 @@ export class CacheControlExtension<TContext = any>
}
}

// If this field is a field on an object, look for hints on the field
// itself, taking precedence over previously calculated hints.
const parentType = info.parentType;
if (parentType instanceof GraphQLObjectType) {
const fieldDef = parentType.getFields()[info.fieldName];
if (fieldDef.astNode) {
hint = mergeHints(
hint,
cacheHintFromDirectives(fieldDef.astNode.directives),
);
}
// Look for hints on the field itself (on its parent type), taking
// precedence over previously calculated hints.
const fieldDef = info.parentType.getFields()[info.fieldName];
if (fieldDef.astNode) {
hint = mergeHints(
hint,
cacheHintFromDirectives(fieldDef.astNode.directives),
);
}

// If this resolver returns an object and we haven't seen an explicit maxAge
// hint, set the maxAge to 0 (uncached) or the default if specified in the
// constructor. (Non-object fields by default are assumed to inherit their
// cacheability from their parents.)
// If this resolver returns an object or is a root field and we haven't seen
// an explicit maxAge hint, set the maxAge to 0 (uncached) or the default if
// specified in the constructor. (Non-object fields by default are assumed
// to inherit their cacheability from their parents. But on the other hand,
// while root non-object fields can get explicit hints from their definition
// on the Query/Mutation object, if that doesn't exist then there's no
// parent field that would assign the default maxAge, so we do it here.)
if (
(targetType instanceof GraphQLObjectType ||
targetType instanceof GraphQLInterfaceType) &&
targetType instanceof GraphQLInterfaceType ||
!info.path.prev) &&
hint.maxAge === undefined
) {
hint.maxAge = this.defaultMaxAge;
Expand Down Expand Up @@ -167,6 +168,8 @@ export class CacheControlExtension<TContext = any>
}
}

// If maxAge is 0, then we consider it uncacheable so it doesn't matter what
// the scope was.
return lowestMaxAge
? {
maxAge: lowestMaxAge,
Expand Down