Allow for one off query overrides

[toddtarsi]

Hello, my name is Todd and I'm a huge fan of your codebase!
An issue I'm encountering though is due to how the current solution for serverside rendering requires modifying the network global via injectNetworkLayer, so I'm pitching this change to the relay team: facebook/relay#1290

Instead of changing our serverside rendering context via injectNetworkLayer and routing queries through a FIFO system, I'd like to pass an overrides object as an additional argument, where fields are provided to modify my request as needed in the network layer. Please weigh in as you feel in both this thread and that one, as I am still getting comfortable with Relay. If this works for everyone though, this should be all that's needed to make server side rendering much cleaner.

[robrichard]

@toddtarsi what are you modifying in the requests in your override? Could you instead make a new network layer for each request?

This is how you could do it if you needed to set unique auth info

app.get('/my-route', function(req, res) {
  const networkLayer = new Relay.DefaultNetworkLayer('https://example.com/graphql', {
    headers: {
      Authorization: req.get('Authorization')
    }
  });

   IsomorphicRelay.prepareData(rootContainerProps, networkLayer).then({data, props} => {
        ...
   })
})

[toddtarsi]

You can do it that way, but I really worry about that. Doesn't that call
injectNetworkLayer with the sensitive information under the covers, which
breaks concurrency?

From this thread
denvned/isomorphic-relay-router#13 I got the
impression that this code requires wrapping the query processing system in
a FIFO system for serverside rendering, which is not good in nodejs. By
allowing for customizing the sendQueries call, we can avoid polluting the
global scope with sensitive context information.

On Tuesday, July 19, 2016, Rob Richard [email protected] wrote:

@toddtarsi https://github.com/toddtarsi what are you modifying in the
requests in your override? Could you instead make a new network layer for
each request?

This is how you could do it if you needed to set unique auth info

app.get('/my-route', function(req, res) {
const networkLayer = new Relay.DefaultNetworkLayer('https://example.com/graphql', {
headers: {
Authorization: req.get('Authorization')
}
});

IsomorphicRelay.prepareData(rootContainerProps, networkLayer).then({data, props} => {
...
})
})

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#43 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AEfUsVclkFLffGmuatZLAyPD68KD9pZlks5qXKmhgaJpZM4JPTf0
.

[robrichard]

I could be wrong, but I think it's not an issue since relay 0.8.0. Prior to that, there was only one global Relay.Store. Now you can make a new Relay.Environment for each request, each with its own networkLayer. That's exactly what the current version of isomorphic-relay does (https://github.com/denvned/isomorphic-relay/blob/master/src/prepareData.js#L10). It's calling injectNetworkLayer on the custom environment, not the global Relay.injectNetworkLayer.

[toddtarsi]

Oh, awesome! Thank you very much for taking the time to explain that.
I am still getting my feet wet with relay. That is a huge relief!

On Jul 19, 2016, at 8:24 AM, Rob Richard [email protected] wrote:

I could be wrong, but I think it's not an issue since relay 0.8.0 https://github.com/facebook/relay/releases/tag/v0.8.0. Prior to that, there was only one global Relay.Store. Now you can make a new Relay.Environment for each request, each with its own networkLayer. That's exactly what the current version of isomorphic-relay does (https://github.com/denvned/isomorphic-relay/blob/master/src/prepareData.js#L10 https://github.com/denvned/isomorphic-relay/blob/master/src/prepareData.js#L10). It's calling injectNetworkLayer on the custom environment, not the global Relay.injectNetworkLayer.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub #43 (comment), or mute the thread https://github.com/notifications/unsubscribe-auth/AEfUsX7YhmaMJH5rJCaQAPWIxJrxB6u-ks5qXNAOgaJpZM4JPTf0.

[toddtarsi]

My use case is satisfied, and if you'd like, I can close this pull. Thanks!

[toddtarsi]

Closing this, thank you again @robrichard