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

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.

packages/apollo-cache-control/CHANGELOG.md

@@ -1,5 +1,15 @@
 # Changelog
 
+### vNEXT
+
+* Fix cache hints of `maxAge: 0` to mean "uncachable". (#2197)
+
+* Apply `defaultMaxAge` to scalar fields on the root object. (#???)
+
+
+(Missing release notes for 0.2.0, 0.2.1, 0.2.2, 0.2.3, 0.2.4, 0.2.5, 0.3.0,
+0.3.1, 0.3.2, 0.3.3, 0.3.4, and 0.4.0!)
+
 ### v0.1.1
 
 * Fix `defaultMaxAge` feature (introduced in 0.1.0) so that `maxAge: 0` overrides the default, as previously documented.

packages/apollo-cache-control/src/__tests__/cacheControlDirective.test.ts

@@ -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 {

packages/apollo-cache-control/src/index.ts

@@ -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;
@@ -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,