Relay: Provide an easy mechanism for accessing the Relay environment so that mutations can run correctly

[AndrewIngram]

Now that multiple environments can exist, the primary way of executing mutations in a component can't be relied on, eg Relay.Store.commitUpdate(new MyMutation(...)). This is because Relay.Store refers to the default environment, and not necessarily the one in use by the current component.

(I'd set up a custom environment to get rid of a warning about injecting a network layer, and spent a long time trying to figure out why all my mutations had stopped getting their props correctly)

The obvious solutions are to pass the environment down the tree are via props or context, but neither are particularly attractive solutions - i'm already in a situation where I have a somewhat silly amount of higher-order components wrapping most of my components.

There may be technical reasons why it can't be done, but I think adding an environment field or getEnvironment method to RelayContainer would be a nice API - and could reasonably replace the current pattern even when not using a custom environment:

const { relay, ...etc } = this.props;
relay.environment.commitUpdate(new MyMutation(etc));

[wincent]

This makes sense. I don't know if we need to expose the whole RelayEnvironment. It might be better to just expose applyUpdate and commitUpdate directly.

Something like this:

diff --git a/src/container/RelayContainer.js b/src/container/RelayContainer.js
index 67addf5..6d1fb83 100644
--- a/src/container/RelayContainer.js
+++ b/src/container/RelayContainer.js
@@ -142,6 +142,8 @@ function createContainerComponent(
       this.state = {
         queryData: {},
         relayProp: {
+          applyUpdate: this.commitUpdate.bind(this),
+          commitUpdate: this.commitUpdate.bind(this),
           forceFetch: this.forceFetch.bind(this),
           getPendingTransactions: this.getPendingTransactions.bind(this),
           hasFragmentData: this.hasFragmentData.bind(this),
@@ -154,6 +156,20 @@ function createContainerComponent(
       };
     }

+    applyUpdate(
+      mutation: RelayMutation,
+      callbacks?: RelayMutationTransactionCommitCallbacks
+    ): RelayMutationTransaction {
+      return this.context.relay.applyUpdate(mutation, callbacks);
+    }
+
+    commitUpdate(
+      mutation: RelayMutation,
+      callbacks?: RelayMutationTransactionCommitCallbacks
+    ): RelayMutationTransaction {
+      return this.context.relay.commitUpdate(mutation, callbacks);
+    }
+
     /**
      * Requests an update to variables. This primes the cache for the new
      * variables and notifies the caller of changes via the callback. As data

Completely untested, so might not work; would need Flow types etc.

[IwanKaramazow]

Indeed, components don't (shouldn't ?) need to access the whole environment.
How does Facebook internally pass the env? I assume with some parts on the new mutation api, this is already the case?

[wincent]

@IwanKaramazow, we don't pass the env (we were able to spin up a new JavaScript environment for each request).

[IwanKaramazow]

I'm not sure I fully understand, or we're talking about different things.
The components need to have access to some part of the env for mutations, right?
I mean, do you pass it down as props, or make it globally available or require it in the right files?

[wincent]

The Relay.Store is a globally accessible, singleton instance of RelayEnvironment, so there is no need to pass it anywhere.

[IwanKaramazow]

Oh I'm definitely doing something wrong 😄

If I don't do this, Relay doesn't get the right props:

const login = new LoginMutation({user: this.props.user,
                       mail: this.state.mail, password: this.state.password});

env.applyUpdate(login); // everything ok
Relay.Store.applyUpdate(login); // Cannot read property 'id' of undefined, (user is fetched by Relay)

[AndrewIngram]

@IwanKaramazow what you're experiencing is the exact problem I was trying to solve when opening this issue. The global Relay.Store won't work for mutations if you're using a custom environment, it won't have access to the data in the store, and it won't have any custom network layers. The solution @wincent has proposed should fix the problem.

[wincent]

FYI I have a diff for this internally which does basically what I pasted above. Just awaiting review.

[ianstormtaylor]

+1 just running into this problem now, seems like mutations can't be used properly without this? Unless I stored a copy of the environment and referenced it everywhere.

[taion]

Wait, what? I've just been doing this.context.relay.commitUpdate. Is that not right? It works...

[taion]

I guess the idea is that it feels "wrong" to use this.context.relay when this.props.relay is available?

[josephsavona]

I guess the idea is that it feels "wrong" to use this.context.relay when this.props.relay is available?

Also accessing relay via context requires knowing which property to use, what type to use for the contextTypes definition, adding contextTypes, etc. Adding these on props.relay felt natural given that there are many other methods there already.

[taion]

That makes sense, and it's a nice convenience.

I was just confused by the implication here and in https://github.com/facebook/relay/issues/233#issuecomment-220143451 that it somehow wasn't possible, since I've been happily using:

contextTypes: {
  relay: Relay.PropTypes.Environment,
}

for a while now.

[josephsavona]

Yeah, it is absolutely possible to execute a mutation on an arbitrary RelayEnvironment today, you just have to make sure that you're executing it against the same environment that you're using for data fetching. I think some folks tried to mix RelayStore and a separate env instance.