Preserve referential equality from previous result in the store

CHANGELOG.md

@@ -15,6 +15,7 @@ Expect active development and potentially significant breaking changes in the `0
 - Remove lodash as a production dependency [PR #1122](https://github.com/apollostack/apollo-client/pull/1122)
 - Minor fix to write to `ROOT_SUBSCRIPTION` ID in the store for subscription results. [PR #1122](https://github.com/apollostack/apollo-client/pull/1127)
 - Remove `whatwg-fetch` polyfill dependency and instead warn when a global `fetch` implementation is not found. [PR #1134](https://github.com/apollostack/apollo-client/pull/1134)
+- New results from `watchQuery` are referentially equal (so `a === b`) to previous results if nothing changed in the store for a better UI integration experience when determining what changed. [PR #1136](https://github.com/apollostack/apollo-client/pull/1136)
 
 ### 0.6.0
 - Switch to `@types/graphql` instead of `typed-graphql` for typings. [PR 1041](https://github.com/apollostack/apollo-client/pull/1041) [PR #934](https://github.com/apollostack/apollo-client/issues/934)

package.json

@@ -18,6 +18,7 @@
     "compile:browser": "rm -rf ./dist && mkdir ./dist && browserify ./lib/apollo.umd.js -o=./dist/index.js && npm run minify:browser",
     "minify:browser": "uglifyjs --compress --mangle --screw-ie8 -o=./dist/index.min.js -- ./dist/index.js",
     "watch": "tsc -w",
+    "watch:test": "tsc -p tsconfig.test.json -w",
     "bundle": "rollup -c",
     "postcompile": "npm run bundle",
     "prepublish": "npm run compile",
@@ -61,6 +62,7 @@
     "chai": "^3.5.0",
     "chai-as-promised": "^6.0.0",
     "colors": "^1.1.2",
+    "concurrently": "^3.1.0",
     "es6-promise": "^4.0.4",
     "fetch-mock": "^5.5.0",
     "grunt": "1.0.1",

scripts/dev.sh

@@ -3,4 +3,4 @@ test_and_lint_command+="$@"
 
 echo $test_and_lint_command
 
-./node_modules/.bin/concurrently -r -k "npm run watch" "$test_and_lint_command"
+$(npm bin)/concurrently -r -k "npm run watch:test" "$test_and_lint_command"

src/core/QueryManager.ts

@@ -361,13 +361,27 @@ export class QueryManager {
                 variables: queryStoreValue.previousVariables || queryStoreValue.variables,
                 returnPartialData: options.returnPartialData || noFetch,
                 config: this.reducerConfig,
+                previousResult: lastResult && lastResult.data,
               }),
               loading: queryStoreValue.loading,
               networkStatus: queryStoreValue.networkStatus,
             };
 
             if (observer.next) {
-              if (this.isDifferentResult(lastResult, resultFromStore)) {
+              const isDifferentResult =
+                this.resultComparator ? !this.resultComparator(lastResult, resultFromStore) : !(
+                  lastResult &&
+                  resultFromStore &&
+                  lastResult.loading === resultFromStore.loading &&
+                  lastResult.networkStatus === resultFromStore.networkStatus &&
+
+                  // We can do a strict equality check here because we include a `previousResult`
+                  // with `readQueryFromStore`. So if the results are the same they will be
+                  // referentially equal.
+                  lastResult.data === resultFromStore.data
+                );
+
+              if (isDifferentResult) {
                 lastResult = resultFromStore;
                 try {
                   observer.next(this.transformResult(resultFromStore));
@@ -1007,12 +1021,6 @@ export class QueryManager {
     }
   }
 
-  // check to see if two results are the same, given our resultComparator
-  private isDifferentResult<T>(lastResult: ApolloQueryResult<T>, newResult: ApolloQueryResult<T>): boolean {
-    const comparator = this.resultComparator || isEqual;
-    return !comparator(lastResult, newResult);
-  }
-
   private broadcastQueries() {
     const queries = this.getApolloState().queries;
     Object.keys(this.queryListeners).forEach((queryId: string) => {

src/data/readFromStore.ts

@@ -5,6 +5,7 @@ import {
 import graphqlAnywhere, {
   Resolver,
   FragmentMatcher,
+  ExecInfo,
 } from 'graphql-anywhere';
 
 import {
@@ -26,6 +27,18 @@ import {
   ApolloReducerConfig,
 } from '../store';
 
+import { isEqual } from '../util/isEqual';
+
+/**
+ * The key which the cache id for a given value is stored in the result object. This key is private
+ * and should not be used by Apollo client users.
+ *
+ * Uses a symbol if available in the environment.
+ *
+ * @private
+ */
+export const ID_KEY = typeof Symbol !== 'undefined' ? Symbol('id') : '@@id';
+
 export type DiffResult = {
   result?: any;
   isMissing?: boolean;
@@ -36,6 +49,7 @@ export type ReadQueryOptions = {
   query: DocumentNode,
   variables?: Object,
   returnPartialData?: boolean,
+  previousResult?: any,
   config?: ApolloReducerConfig,
 };
 
@@ -47,37 +61,50 @@ export type CustomResolverMap = {
   },
 };
 
+/**
+ * This code needs an optional `previousResult` property on `IdValue` so that when the results
+ * returned from the store are the same, we can just return the `previousResult` and not a new
+ * value thus preserving referential equality.
+ *
+ * The `previousResult` property is added to our `IdValue`s in the `graphql-anywhere` resolver so
+ * that they can be in the right position for `resultMapper` to test equality and return whichever
+ * result is appropriate.
+ *
+ * `resultMapper` takes the `previousResult`s and performs a shallow referential equality check. If
+ * that passes then instead of returning the object created by `graphql-anywhere` the
+ * `resultMapper` function will instead return the `previousResult`. This process is bottom-up so
+ * we start at the leaf results and swap them for `previousResult`s all the way up until we get to
+ * the root object.
+ */
+interface IdValueWithPreviousResult extends IdValue {
+  previousResult?: any;
+}
+
 /**
  * Resolves the result of a query solely from the store (i.e. never hits the server).
  *
- * @param store The {@link NormalizedCache} used by Apollo for the `data` portion of the store.
+ * @param {Store} store The {@link NormalizedCache} used by Apollo for the `data` portion of the
+ * store.
  *
- * @param query The query document to resolve from the data available in the store.
+ * @param {DocumentNode} query The query document to resolve from the data available in the store.
  *
- * @param variables A map from the name of a variable to its value. These variables can be
- * referenced by the query document.
+ * @param {Object} [variables] A map from the name of a variable to its value. These variables can
+ * be referenced by the query document.
  *
- * @param returnPartialData If set to true, the query will be resolved even if all of the data
- * needed to resolve the query is not found in the store. The data keys that are not found will not
- * be present in the returned object. If set to false, an error will be thrown if there are fields
- * that cannot be resolved from the store.
+ * @param {boolean} [returnPartialData] If set to true, the query will be resolved even if all of
+ * the data needed to resolve the query is not found in the store. The data keys that are not found
+ * will not be present in the returned object. If set to false, an error will be thrown if there
+ * are fields that cannot be resolved from the store.
+ *
+ * @param {any} previousResult The previous result returned by this function for the same query.
+ * If nothing in the store changed since that previous result then values from the previous result
+ * will be returned to preserve referential equality.
  */
-export function readQueryFromStore<QueryType>({
-  store,
-  query,
-  variables,
-  returnPartialData = false,
-  config,
-}: ReadQueryOptions): QueryType {
-  const { result } = diffQueryAgainstStore({
-    query,
-    store,
+export function readQueryFromStore<QueryType>({ returnPartialData = false, ...options }: ReadQueryOptions): QueryType {
+  return diffQueryAgainstStore({
+    ...options,
     returnPartialData,
-    variables,
-    config,
-  });
-
-  return result;
+  }).result;
 }
 
 type ReadStoreContext = {
@@ -134,16 +161,17 @@ match fragments.`);
 
 const readStoreResolver: Resolver = (
   fieldName: string,
-  idValue: IdValue,
+  idValue: IdValueWithPreviousResult,
   args: any,
   context: ReadStoreContext,
+  { resultKey }: ExecInfo,
 ) => {
   assertIdValue(idValue);
 
   const objId = idValue.id;
   const obj = context.store[objId];
   const storeKeyName = storeKeyNameFromFieldNameAndArgs(fieldName, args);
-  const fieldValue = (obj || {})[storeKeyName];
+  let fieldValue = (obj || {})[storeKeyName];
 
   if (typeof fieldValue === 'undefined') {
     if (context.customResolvers && obj && (obj.__typename || objId === 'ROOT_QUERY')) {
@@ -170,11 +198,26 @@ Perhaps you want to use the \`returnPartialData\` option?`);
     return fieldValue;
   }
 
+  // if this is an object scalar, it must be a json blob and we have to unescape it
   if (isJsonValue(fieldValue)) {
-    // if this is an object scalar, it must be a json blob and we have to unescape it
+    // If the JSON blob is the same now as in the previous result, return the previous result to
+    // maintain referential equality.
+    //
+    // `isEqual` will first perform a referential equality check (with `===`) in case the JSON
+    // value has not changed in the store, and then a deep equality check if that fails in case a
+    // new JSON object was returned by the API but that object may still be the same.
+    if (idValue.previousResult && isEqual(idValue.previousResult[resultKey], fieldValue.json)) {
+      return idValue.previousResult[resultKey];
+    }
     return fieldValue.json;
   }
 
+  // If we had a previous result, try adding that previous result value for this field to our field
+  // value. This will create a new value without mutating the old one.
+  if (idValue.previousResult) {
+    fieldValue = addPreviousResultToIdValues(fieldValue, idValue.previousResult[resultKey]);
+  }
+
   return fieldValue;
 };
 
@@ -184,13 +227,15 @@ Perhaps you want to use the \`returnPartialData\` option?`);
  * @param  {DocumentNode} query A parsed GraphQL query document
  * @param  {Store} store The Apollo Client store object
  * @param  {boolean} [returnPartialData] Whether to throw an error if any fields are missing
+ * @param  {any} previousResult The previous result returned by this function for the same query
  * @return {result: Object, isMissing: [boolean]}
  */
 export function diffQueryAgainstStore({
   store,
   query,
   variables,
   returnPartialData = true,
+  previousResult,
   config,
 }: ReadQueryOptions): DiffResult {
   // Throw the right validation error by trying to find a query in the document
@@ -209,10 +254,12 @@ export function diffQueryAgainstStore({
   const rootIdValue = {
     type: 'id',
     id: 'ROOT_QUERY',
+    previousResult,
   };
 
   const result = graphqlAnywhere(readStoreResolver, query, rootIdValue, context, variables, {
     fragmentMatcher,
+    resultMapper,
   });
 
   return {
@@ -228,3 +275,126 @@ an object reference. This should never happen during normal use unless you have
 that is directly manipulating the store; please file an issue.`);
   }
 }
+
+/**
+ * Adds a previous result value to id values in a nested array. For a single id value and a single
+ * previous result then the previous value is added directly.
+ *
+ * For arrays we put all of the ids from the previous result array in a map and add them to id
+ * values with the same id.
+ *
+ * This function does not mutate. Instead it returns new instances of modified values.
+ *
+ * @private
+ */
+function addPreviousResultToIdValues (value: any, previousResult: any): any {
+  // If the value is an `IdValue`, add the previous result to it whether or not that
+  // `previousResult` is undefined.
+  //
+  // If the value is an array, recurse over each item trying to add the `previousResult` for that
+  // item.
+  if (isIdValue(value)) {
+    return {
+      ...value,
+      previousResult,
+    };
+  } else if (Array.isArray(value)) {
+    const idToPreviousResult: { [id: string]: any } = {};
+
+    // If the previous result was an array, we want to build up our map of ids to previous results
+    // using the private `ID_KEY` property that is added in `resultMapper`.
+    if (Array.isArray(previousResult)) {
+      previousResult.forEach(item => {
+        if (item[ID_KEY]) {
+          idToPreviousResult[item[ID_KEY]] = item;
+        }
+      });
+    }
+
+    // For every value we want to add the previous result.
+    return value.map((item, i) => {
+      // By default the previous result for this item will be in the same array position as this
+      // item.
+      let itemPreviousResult = previousResult && previousResult[i];
+
+      // If the item is an id value, we should check to see if there is a previous result for this
+      // specific id. If there is, that will be the value for `itemPreviousResult`.
+      if (isIdValue(item)) {
+        itemPreviousResult = idToPreviousResult[item.id] || itemPreviousResult;
+      }
+
+      return addPreviousResultToIdValues(item, itemPreviousResult);
+    });
+  }
+  // Return the value, nothing changed.
+  return value;
+}
+
+/**
+ * Maps a result from `graphql-anywhere` to a final result value.
+ *
+ * If the result and the previous result from the `idValue` pass a shallow equality test, we just
+ * return the `previousResult` to maintain referential equality.
+ *
+ * We also add a private id property to the result that we can use later on.
+ *
+ * @private
+ */
+function resultMapper (resultFields: any, idValue: IdValueWithPreviousResult) {
+  // If we had a previous result, we may be able to return that and preserve referential equality
+  if (idValue.previousResult) {
+    const currentResultKeys = Object.keys(resultFields);
+
+    const sameAsPreviousResult =
+      // Confirm that we have the same keys in both the current result and the previous result.
+      Object.keys(idValue.previousResult)
+        .reduce((sameKeys, key) => sameKeys && currentResultKeys.indexOf(key) > -1, true) &&
+
+      // Perform a shallow comparison of the result fields with the previous result. If all of
+      // the shallow fields are referentially equal to the fields of the previous result we can
+      // just return the previous result.
+      //
+      // While we do a shallow comparison of objects, but we do a deep comparison of arrays.
+      currentResultKeys.reduce((same, key) => (
+        same && areNestedArrayItemsStrictlyEqual(resultFields[key], idValue.previousResult[key])
+      ), true);
+
+    if (sameAsPreviousResult) {
+      return idValue.previousResult;
+    }
+  }
+
+  // Add the id to the result fields. It should be non-enumerable so users can’t see it without
+  // trying very hard.
+  Object.defineProperty(resultFields, ID_KEY, {
+    enumerable: false,
+    configurable: false,
+    writable: false,
+    value: idValue.id,
+  });
+
+  return resultFields;
+}
+
+type NestedArray<T> = T | Array<T | Array<T | Array<T>>>;
+
+/**
+ * Compare all the items to see if they are all referentially equal in two arrays no matter how
+ * deeply nested the arrays are.
+ *
+ * @private
+ */
+function areNestedArrayItemsStrictlyEqual (a: NestedArray<any>, b: NestedArray<any>): boolean {
+  // If `a` and `b` are referentially equal, return true.
+  if (a === b) {
+    return true;
+  }
+  // If either `a` or `b` are not an array return false. `a` and `b` are known to not be equal
+  // here, we checked above.
+  if (!Array.isArray(a) || !Array.isArray(b)) {
+    return false;
+  }
+  // Otherwise let us compare all of the array items (which are potentially nested arrays!) to see
+  // if they are equal.
+  return a.reduce((same, item, i) => same && areNestedArrayItemsStrictlyEqual(item, b[i]), true);
+}