Merge pull request #3964 from apollographql/include-root-ID-in-execut…

…eStoreQuery-cache-key

Fix various apollo-cache-inmemory 1.3.x bugs.

CHANGELOG.md

@@ -15,13 +15,17 @@
   [@billfienberg](https://github.com/billfienberg) in [#3886](https://github.com/apollographql/apollo-client/pull/3886)  <br/>
   [@TLadd](https://github.com/TLadd) in [#3884](https://github.com/apollographql/apollo-client/pull/3884)
 
-### Apollo Cache In-Memory (1.3.0)
+### Apollo Cache In-Memory (1.3.3)
 
 - Optimize repeated `apollo-cache-inmemory` reads by caching partial query
   results, for substantial performance improvements. As a consequence, watched
   queries will not be rebroadcast unless the data have changed.
   [PR #3394](https://github.com/apollographql/apollo-client/pull/3394)
 
+- Include root ID and fragment matcher in `StoreReader#executeStoreQuery`
+  and `#executeSelectionSet` cache keys.
+  [PR #3964](https://github.com/apollographql/apollo-client/pull/3964)
+
 ### Apollo GraphQL Anywhere (vNext)
 
 - Make `graphql-anywhere` `filter` function generic (typescript).  <br/>

packages/apollo-cache-inmemory/package.json

@@ -1,6 +1,6 @@
 {
   "name": "apollo-cache-inmemory",
-  "version": "1.3.0",
+  "version": "1.3.5",
   "description": "Core abstract of Caching layer for Apollo Client",
   "author": "James Baxley <james@meteor.com>",
   "contributors": [

packages/apollo-cache-inmemory/src/fixPolyfills.ts

@@ -0,0 +1,32 @@
+const frozen = {};
+const frozenTestMap = new Map();
+
+if (typeof Object.freeze === 'function') {
+  Object.freeze(frozen);
+}
+
+try {
+  // If non-extensible objects can't be stored as keys in a Map, make sure we
+  // do not freeze/seal/etc. an object without first attempting to put it in a
+  // Map. For example, this gives the React Native Map polyfill a chance to tag
+  // objects before they become non-extensible:
+  // https://github.com/facebook/react-native/blob/98a6f19d7c/Libraries/vendor/core/Map.js#L44-L50
+  // https://github.com/apollographql/react-apollo/issues/2442#issuecomment-426489517
+  frozenTestMap.set(frozen, frozen).delete(frozen);
+} catch {
+  const wrap = (method: <T>(obj: T) => T): typeof method => {
+    return method && (obj => {
+      try {
+        // If .set succeeds, also call .delete to avoid leaking memory.
+        frozenTestMap.set(obj, obj).delete(obj);
+      } finally {
+        // If .set or .delete fails, the exception will be silently swallowed
+        // by this return-from-finally statement:
+        return method.call(Object, obj);
+      }
+    });
+  };
+  Object.freeze = wrap(Object.freeze);
+  Object.seal = wrap(Object.seal);
+  Object.preventExtensions = wrap(Object.preventExtensions);
+}

packages/apollo-cache-inmemory/src/inMemoryCache.ts

@@ -1,3 +1,6 @@
+// Make builtins like Map and Set safe to use with non-extensible objects.
+import './fixPolyfills';
+
 import { DocumentNode } from 'graphql';
 
 import { Cache, DataProxy, ApolloCache, Transaction } from 'apollo-cache';
@@ -77,14 +80,8 @@ export class InMemoryCache extends ApolloCache<NormalizedCacheObject> {
     this.addTypename = this.config.addTypename;
     this.data = defaultNormalizedCacheFactory();
 
-    this.storeReader = new StoreReader({
-      addTypename: this.config.addTypename,
-      cacheKeyRoot: this.cacheKeyRoot,
-    });
-
-    this.storeWriter = new StoreWriter({
-      addTypename: this.config.addTypename,
-    });
+    this.storeReader = new StoreReader(this.cacheKeyRoot);
+    this.storeWriter = new StoreWriter();
 
     const cache = this;
     const { maybeBroadcastWatch } = cache;
@@ -143,7 +140,7 @@ export class InMemoryCache extends ApolloCache<NormalizedCacheObject> {
 
     return this.storeReader.readQueryFromStore({
       store,
-      query: query.query,
+      query: this.transformDocument(query.query),
       variables: query.variables,
       rootId: query.rootId,
       fragmentMatcherFunction: this.config.fragmentMatcher.match,
@@ -157,7 +154,7 @@ export class InMemoryCache extends ApolloCache<NormalizedCacheObject> {
       dataId: write.dataId,
       result: write.result,
       variables: write.variables,
-      document: write.query,
+      document: this.transformDocument(write.query),
       store: this.data,
       dataIdFromObject: this.config.dataIdFromObject,
       fragmentMatcherFunction: this.config.fragmentMatcher.match,
@@ -173,7 +170,7 @@ export class InMemoryCache extends ApolloCache<NormalizedCacheObject> {
 
     return this.storeReader.diffQueryAgainstStore({
       store: store,
-      query: query.query,
+      query: this.transformDocument(query.query),
       variables: query.variables,
       returnPartialData: query.returnPartialData,
       previousResult: query.previousResult,
@@ -288,9 +285,8 @@ export class InMemoryCache extends ApolloCache<NormalizedCacheObject> {
     optimistic: boolean = false,
   ): FragmentType | null {
     return this.read({
-      query: getFragmentQueryDocument(
-        options.fragment,
-        options.fragmentName,
+      query: this.transformDocument(
+        getFragmentQueryDocument(options.fragment, options.fragmentName),
       ),
       variables: options.variables,
       rootId: options.id,
@@ -304,7 +300,7 @@ export class InMemoryCache extends ApolloCache<NormalizedCacheObject> {
     this.write({
       dataId: 'ROOT_QUERY',
       result: options.data,
-      query: options.query,
+      query: this.transformDocument(options.query),
       variables: options.variables,
     });
   }
@@ -315,9 +311,8 @@ export class InMemoryCache extends ApolloCache<NormalizedCacheObject> {
     this.write({
       dataId: options.id,
       result: options.data,
-      query: getFragmentQueryDocument(
-        options.fragment,
-        options.fragmentName,
+      query: this.transformDocument(
+        getFragmentQueryDocument(options.fragment, options.fragmentName),
       ),
       variables: options.variables,
     });

packages/apollo-cache-inmemory/src/queryKeyMaker.ts

@@ -1,7 +1,9 @@
 import { CacheKeyNode } from "./optimism";
 import { DocumentNode, SelectionSetNode, FragmentSpreadNode, FragmentDefinitionNode } from "graphql";
+import { QueryDocumentKeys } from "graphql/language/visitor";
 
 const CIRCULAR = Object.create(null);
+const objToStr = Object.prototype.toString;
 
 export class QueryKeyMaker {
   private perQueryKeyMakers = new Map<DocumentNode, PerQueryKeyMaker>();
@@ -98,21 +100,58 @@ class PerQueryKeyMaker {
   private lookupArray(array: any[]): object {
     const elements = array.map(this.lookupAny, this);
     return this.cacheKeyRoot.lookup(
-      Array.prototype,
+      objToStr.call(array),
       this.cacheKeyRoot.lookupArray(elements),
     );
   }
 
   private lookupObject(object: { [key: string]: any }): object {
-    const keys = Object.keys(object);
-    const locIndex = keys.indexOf("loc");
-    if (locIndex >= 0) keys.splice(locIndex, 1);
-    keys.sort();
+    const keys = safeSortedKeys(object);
     const values = keys.map(key => this.lookupAny(object[key]));
     return this.cacheKeyRoot.lookup(
-      Object.getPrototypeOf(object),
+      objToStr.call(object),
       this.cacheKeyRoot.lookupArray(keys),
       this.cacheKeyRoot.lookupArray(values),
     );
   }
 }
+
+const queryKeyMap: {
+  [key: string]: { [key: string]: boolean }
+} = Object.create(null);
+
+Object.keys(QueryDocumentKeys).forEach(parentKind => {
+  const childKeys = queryKeyMap[parentKind] = Object.create(null);
+
+  (QueryDocumentKeys as {
+    [key: string]: any[]
+  })[parentKind].forEach(childKey => {
+    childKeys[childKey] = true;
+  });
+
+  if (parentKind === "FragmentSpread") {
+    // A custom key that we include when looking up FragmentSpread nodes.
+    childKeys["fragment"] = true;
+  }
+});
+
+function safeSortedKeys(object: { [key: string]: any }): string[] {
+  const keys = Object.keys(object);
+  const keyCount = keys.length;
+  const knownKeys = typeof object.kind === "string" && queryKeyMap[object.kind];
+
+  // Remove unknown object-valued keys from the array, but leave keys with
+  // non-object values untouched.
+  let target = 0;
+  for (let source = target; source < keyCount; ++source) {
+    const key = keys[source];
+    const value = object[key];
+    const isObjectOrArray = value !== null && typeof value === "object";
+    if (! isObjectOrArray || ! knownKeys || knownKeys[key] === true) {
+      keys[target++] = key;
+    }
+  }
+  keys.length = target;
+
+  return keys.sort();
+}