Contextualize Relay mutations

Summary:== Original Description ==
This PR is another step toward making all Relay state contextual (#558). Originally this was submitted as a part of #683, but josephsavona [suggested](#683 (comment)) to split it to smaller PRs.

Currently, Relay resolve mutations props using the global `RelayStore`, and store them in the `props` instance field of `RelayMutation`. That makes it impossible to contextualize Relay mutations without changing the public API.

After analyzing different alternatives, I have found that the probably cleanest solution is not to store resolved mutation props in an instance field, but to pass them as an argument to each of the mutation methods. That way mutations can be reused in different `RelayStoreData` contexts, and also they can be reused in the same context but at different times, because props are re-resolved at each mutation execution.

== Summary of Changes ==
- Change `RelayMutation` constructor to not immediately resolve data for fragment props
- Add `bindContext` method on RelayMutation, called when the mutation is committed on a specific RelayContext
- This necessitated adding the `read` method to `RelayContextInterface`, and stubbing this method on alternative context implementations.
- Updated mutation docs to mention this method.

Closes #699

Reviewed By: yungsters

Differential Revision: D2913334

Pulled By: josephsavona

fb-gh-sync-id: 5051fddf068fc8bc7379bb55c8b1e4d97e33313c
shipit-source-id: 5051fddf068fc8bc7379bb55c8b1e4d97e33313c

docs/APIReference-Mutation.md

@@ -42,6 +42,11 @@ Relay makes use of GraphQL mutations; operations that enable us to mutate data o
       <pre>constructor(props)</pre>
     </a>
   </li>
+  <li>
+    <a href="#didresolveprops">
+      <pre>didResolveProps()</pre>
+    </a>
+  </li>
   <li>
     <a href="#getconfigs-abstract-method">
       <pre>abstract getConfigs()</pre>
@@ -198,7 +203,7 @@ See also:
 
 ### constructor
 
-Create a mutation instance using the `new` keyword, optionally passing it some props.
+Create a mutation instance using the `new` keyword, optionally passing it some props. Note that `this.props` is *not* set inside the constructor function. Props depend on data from a `RelayEnvironment`, which isn't known until the mutation is applied with `applyUpdate` or `commitUpdate`. To update the instance once props are available, implement [`didResolveProps`](#didresolveprops).
 
 #### Example
 
@@ -207,6 +212,33 @@ var bookFlightMutation = new BuyPlaneTicketMutation({airport: 'yvr'});
 Relay.Store.commitUpdate(bookFlightMutation);
 ```
 
+### didResolveProps
+
+Optional method. If implemented, it is called when a mutation has been applied to a Relay context and data for fragments is available on `this.props`.
+
+#### Example
+
+```
+class LikeStoryMutation extends Relay.Mutation {
+  static fragments = {
+    story: () => Relay.QL`
+      fragment on Story {
+        id
+      }
+    `,`
+  };
+
+  constructor(props) {
+    super(props);
+    this.props.story; // undefined, use `didResolveProps` instead
+  }
+
+  didResolveProps() {
+    this.props.story; // data is available
+  }
+}
+```
+
 ### getConfigs (abstract method)
 
 ```

src/mutation/RelayMutation.js

@@ -17,9 +17,9 @@ import type {ConcreteFragment} from 'ConcreteQuery';
 import type {RelayConcreteNode} from 'RelayQL';
 const RelayFragmentPointer = require('RelayFragmentPointer');
 const RelayFragmentReference = require('RelayFragmentReference');
+import type {RelayContextInterface} from 'RelayContext';
 const RelayMetaRoute = require('RelayMetaRoute');
 const RelayQuery = require('RelayQuery');
-const RelayStore = require('RelayStore');
 import type {
   RelayMutationConfig,
   Variables,
@@ -52,13 +52,39 @@ class RelayMutation<Tp: Object> {
   ) => Variables;
 
   props: Tp;
+  _context: RelayContextInterface;
   _didShowFakeDataWarning: boolean;
+  _unresolvedProps: Tp;
 
   constructor(props: Tp) {
     this._didShowFakeDataWarning = false;
-    this._resolveProps(props);
+    this._unresolvedProps = props;
   }
 
+  /**
+   * @internal
+   */
+  bindContext(context: RelayContextInterface): void {
+    if (!this._context) {
+      this._context = context;
+      this._resolveProps();
+      this.didResolveProps();
+    } else {
+      invariant(
+        context === this._context,
+        '%s: Mutation instance cannot be used in different Relay contexts.',
+        this.constructor.name
+      );
+    }
+  }
+
+  /**
+   * Callback that is invoked after the mutation has been applied/committed
+   * (via `relayContext.applyUpdate` or `relayContext.commitUpdate`) and when
+   * props have been resolved and can be safely referenced.
+   */
+  didResolveProps() {}
+
   /**
    * Each mutation corresponds to a field on the server which is used by clients
    * to communicate the type of mutation to be executed.
@@ -237,10 +263,11 @@ class RelayMutation<Tp: Object> {
     return null;
   }
 
-  _resolveProps(props: Tp): void {
+  _resolveProps(): void {
     const fragments = this.constructor.fragments;
     const initialVariables = this.constructor.initialVariables || {};
 
+    const props = this._unresolvedProps;
     const resolvedProps = {...props};
     forEachObject(fragments, (fragmentBuilder, fragmentName) => {
       var propValue = props[fragmentName];
@@ -306,7 +333,9 @@ class RelayMutation<Tp: Object> {
           return dataID;
         });
 
-        resolvedProps[fragmentName] = RelayStore.readAll(fragment, dataIDs);
+        resolvedProps[fragmentName] = dataIDs.map(
+          dataID => this._context.read(fragment, dataID)
+        );
       } else {
         invariant(
           !Array.isArray(propValue),
@@ -317,7 +346,7 @@ class RelayMutation<Tp: Object> {
         );
         var dataID = RelayFragmentPointer.getDataID(propValue, fragment);
         if (dataID) {
-          resolvedProps[fragmentName] = RelayStore.read(fragment, dataID);
+          resolvedProps[fragmentName] = this._context.read(fragment, dataID);
         } else {
           if (__DEV__) {
             if (!this._didShowFakeDataWarning) {

src/mutation/__tests__/RelayMutation-test.js

@@ -17,26 +17,29 @@ jest
   .dontMock('RelayMutation');
 
 const Relay = require('Relay');
+const RelayContext = require('RelayContext');
 const RelayQuery = require('RelayQuery');
 const RelayTestUtils = require('RelayTestUtils');
 
 const buildRQL = require('buildRQL');
 
 describe('RelayMutation', function() {
-  let MockMutation;
-  let initialVariables;
-  let mockBarPointer;
-  let mockFooPointer;
-  let mockRoute;
+  let mockBarFragment;
+  let mockFooFragment;
+  let mockMutation;
+  let relayContext;
 
   const {getNode, getPointer} = RelayTestUtils;
 
   beforeEach(function() {
     jest.resetModuleRegistry();
 
-    initialVariables = {isRelative: false};
+    relayContext = new RelayContext();
+    relayContext.read = jest.genMockFunction();
 
-    var makeMockMutation = () => {
+    const initialVariables = {isRelative: false};
+
+    const makeMockMutation = () => {
       class MockMutationClass extends Relay.Mutation {}
       MockMutationClass.fragments = {
         foo: () => Relay.QL`
@@ -53,42 +56,106 @@ describe('RelayMutation', function() {
       MockMutationClass.initialVariables = initialVariables;
       return MockMutationClass;
     };
-    MockMutation = makeMockMutation();
+    const MockMutation = makeMockMutation();
 
-    var mockFooRequiredFragment =
+    const mockFooRequiredFragment =
       MockMutation.getFragment('foo').getFragment({});
-    var mockBarRequiredFragment =
+    const mockBarRequiredFragment =
       MockMutation.getFragment('bar').getFragment({});
-    mockFooPointer = getPointer('foo', getNode(mockFooRequiredFragment));
-    mockBarPointer = getPointer('bar', getNode(mockBarRequiredFragment));
+    const mockFooPointer = getPointer('foo', getNode(mockFooRequiredFragment));
+    const mockBarPointer = getPointer('bar', getNode(mockBarRequiredFragment));
 
     // RelayMetaRoute.get(...)
-    mockRoute = {name: '$RelayMutation_MockMutationClass'};
-
-    jasmine.addMatchers(RelayTestUtils.matchers);
-  });
+    const mockRoute = {name: '$RelayMutation_MockMutationClass'};
 
-  it('resolves props', () => {
-    /* eslint-disable no-new */
-    new MockMutation({
+    mockMutation = new MockMutation({
       bar: mockBarPointer,
       foo: mockFooPointer,
     });
     /* eslint-enable no-new */
-    const fooFragment = RelayQuery.Fragment.create(
+    mockFooFragment = RelayQuery.Fragment.create(
       buildRQL.Fragment(MockMutation.fragments.foo, initialVariables),
       mockRoute,
       initialVariables
     );
-    const barFragment = RelayQuery.Fragment.create(
+    mockBarFragment = RelayQuery.Fragment.create(
       buildRQL.Fragment(MockMutation.fragments.bar, initialVariables),
       mockRoute,
       initialVariables
     );
-    expect(Relay.Store.read.mock.calls).toEqual([
-      [/* fragment */fooFragment, /* dataID */'foo'],
-      [/* fragment */barFragment, /* dataID */'bar'],
+
+    jasmine.addMatchers(RelayTestUtils.matchers);
+  });
+
+  it('throws if used in different Relay contexts', () => {
+    mockMutation.bindContext(relayContext);
+    expect(() => {
+      mockMutation.bindContext(new RelayContext());
+    }).toFailInvariant(
+      'MockMutationClass: Mutation instance cannot be used ' +
+      'in different Relay contexts.'
+    );
+  });
+
+  it('can be reused in the same Relay context', () => {
+    mockMutation.bindContext(relayContext);
+    expect(() => {
+      mockMutation.bindContext(relayContext);
+    }).not.toThrow();
+  });
+
+  it('does not resolve props before binding Relay context', () => {
+    expect(mockMutation.props).toBeUndefined();
+  });
+
+  it('resolves props only once', () => {
+    mockMutation.bindContext(relayContext);
+    mockMutation.bindContext(relayContext);
+    expect(relayContext.read.mock.calls).toEqual([
+      [/* fragment */mockFooFragment, /* dataID */'foo'],
+      [/* fragment */mockBarFragment, /* dataID */'bar'],
+    ]);
+  });
+
+  it('resolves props after binding Relay context', () => {
+    const resolvedProps = {
+      bar: {},
+      foo: {},
+    };
+    relayContext.read.mockImplementation((_, dataID) => resolvedProps[dataID]);
+    mockMutation.bindContext(relayContext);
+    expect(relayContext.read.mock.calls).toEqual([
+      [/* fragment */mockFooFragment, /* dataID */'foo'],
+      [/* fragment */mockBarFragment, /* dataID */'bar'],
     ]);
+    expect(mockMutation.props).toEqual(resolvedProps);
+    expect(mockMutation.props.bar).toBe(resolvedProps.bar);
+    expect(mockMutation.props.foo).toBe(resolvedProps.foo);
+  });
+
+  it('calls `didResolveProps` after resolving props', () => {
+    const resolvedProps = {
+      bar: {},
+      foo: {},
+    };
+    relayContext.read.mockImplementation((_, dataID) => resolvedProps[dataID]);
+    mockMutation.didResolveProps = jest.genMockFn().mockImplementation(
+      function () {
+        expect(this.props).toEqual(resolvedProps);
+        expect(this.props.bar).toBe(resolvedProps.bar);
+        expect(this.props.foo).toBe(resolvedProps.foo);
+      }
+    );
+    expect(mockMutation.didResolveProps).not.toBeCalled();
+    mockMutation.bindContext(relayContext);
+    expect(mockMutation.didResolveProps).toBeCalled();
+  });
+
+  it('calls `didResolveProps` only once', () => {
+    mockMutation.didResolveProps = jest.genMockFn();
+    mockMutation.bindContext(relayContext);
+    mockMutation.bindContext(relayContext);
+    expect(mockMutation.didResolveProps.mock.calls.length).toBe(1);
   });
 
   it('throws if mutation defines invalid `Relay.QL` fragment', () => {

src/store/RelayContext.js

@@ -63,6 +63,11 @@ export type RelayContextInterface = {
     querySet: RelayQuerySet,
     onReadyStateChange: ReadyStateChangeCallback
   ) => Abortable;
+  read: (
+    node: RelayQuery.Node,
+    dataID: DataID,
+    options?: StoreReaderOptions
+  ) => ?StoreReaderData;
 };
 
 /**
@@ -231,6 +236,7 @@ class RelayContext {
     mutation: RelayMutation,
     callbacks?: RelayMutationTransactionCommitCallbacks
   ): RelayMutationTransaction {
+    mutation.bindContext(this);
     return this._storeData.getMutationQueue().createTransaction(
       mutation,
       callbacks

src/store/__tests__/RelayContext-test.js

@@ -181,6 +181,15 @@ describe('RelayContext', () => {
     });
 
     describe('applyUpdate', () => {
+      it('binds context to mutation before creating transaction', () => {
+        mockMutation.bindContext.mockImplementation(() => {
+          expect(createTransactionMock).not.toBeCalled();
+        });
+        relayContext.applyUpdate(mockMutation);
+        expect(mockMutation.bindContext).toBeCalled();
+        expect(mockMutation.bindContext.mock.calls[0][0]).toBe(relayContext);
+      });
+
       it('creates a new RelayMutationTransaction without committing it', () => {
         const transaction =
           relayContext.applyUpdate(mockMutation, mockCallbacks);
@@ -194,6 +203,15 @@ describe('RelayContext', () => {
     });
 
     describe('commitUpdate', () => {
+      it('binds context to mutation before creating transaction', () => {
+        mockMutation.bindContext.mockImplementation(() => {
+          expect(createTransactionMock).not.toBeCalled();
+        });
+        relayContext.commitUpdate(mockMutation);
+        expect(mockMutation.bindContext).toBeCalled();
+        expect(mockMutation.bindContext.mock.calls[0][0]).toBe(relayContext);
+      });
+
       it('creates a new RelayMutationTransaction and commits it', () => {
         relayContext.commitUpdate(mockMutation, mockCallbacks);
         expect(createTransactionMock).toBeCalledWith(