Skip to content

Commit

Permalink
Contextualize RelayNetworkLayer
Browse files Browse the repository at this point in the history
Summary:== Original Description ==
This PR is another step toward making all Relay state contextual (#558).

Contextual `RelayNetworkLayer` is important for server-side rendering. It will allow to inject a contextual network layer initialized with client request specific params (e.g. authentication cookies) on the server. Currently it is impossible because the injected network layer is global, and shared between client requests.

== Summary of Changes ==
- Moves `fetchRelayQuery` to a method on `RelayNetworkLayer`, which is now a class with an instance held by each `RelayStoreData`, thereby making the network layer contextual to the context instance.
- Changes `Relay.injectNetworkLayer` to a facade for `RelayStore.injectNetworkLayer()`.
- Changes the FB/OSS `Relay` module to inject a network layer directly on `RelayStore`.
- Re-create `fetchRelayQuery` in relay-fb for convenience

Closes #704

Reviewed By: wincent

Differential Revision: D3016293

Pulled By: josephsavona

fb-gh-sync-id: 2e5f3163bc048a089776477b3d2dac2546fd9f48
shipit-source-id: 2e5f3163bc048a089776477b3d2dac2546fd9f48
  • Loading branch information
denvned authored and Facebook Github Bot 0 committed Mar 7, 2016
1 parent fb9c89a commit 5fb67ed
Show file tree
Hide file tree
Showing 17 changed files with 191 additions and 200 deletions.
3 changes: 1 addition & 2 deletions src/RelayPublic.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@

const RelayContainer = require('RelayContainer');
const RelayMutation = require('RelayMutation');
const RelayNetworkLayer = require('RelayNetworkLayer');
const RelayPropTypes = require('RelayPropTypes');
const RelayQL = require('RelayQL');
const RelayRootContainer = require('RelayRootContainer');
Expand Down Expand Up @@ -47,7 +46,7 @@ var RelayPublic = {
createContainer: RelayContainer.create,
createQuery: createRelayQuery,
getQueries: getRelayQueries,
injectNetworkLayer: RelayNetworkLayer.injectNetworkLayer,
injectNetworkLayer: RelayStore.injectNetworkLayer.bind(RelayStore),
injectTaskScheduler: RelayTaskScheduler.injectScheduler,
isContainer: isRelayContainer,
};
Expand Down
4 changes: 2 additions & 2 deletions src/__forks__/Relay.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,10 @@

var RelayDefaultNetworkLayer = require('RelayDefaultNetworkLayer');
var RelayPublic = require('RelayPublic');
const RelayStore = require('RelayStore');

// By default, assume that GraphQL is served at `/graphql` on the same domain.
// $FlowFixMe(>=0.16.0)
RelayPublic.injectNetworkLayer(new RelayDefaultNetworkLayer('/graphql'));
RelayStore.injectNetworkLayer(new RelayDefaultNetworkLayer('/graphql'));

module.exports = {
...RelayPublic,
Expand Down
6 changes: 3 additions & 3 deletions src/legacy/store/GraphQLQueryRunner.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ const RelayFetchMode = require('RelayFetchMode');
import type {FetchMode} from 'RelayFetchMode';
import type {RelayQuerySet} from 'RelayInternalTypes';
import type {PendingFetch} from 'RelayPendingQueryTracker';
const RelayNetworkLayer = require('RelayNetworkLayer');
const RelayProfiler = require('RelayProfiler');
import type RelayQuery from 'RelayQuery';
const RelayReadyState = require('RelayReadyState');
Expand Down Expand Up @@ -92,9 +91,10 @@ function hasItems(map: Object): boolean {
}

function splitAndFlattenQueries(
storeData: RelayStoreData,
queries: Array<RelayQuery.Root>
): Array<RelayQuery.Root> {
if (!RelayNetworkLayer.supports('defer')) {
if (!storeData.getNetworkLayer().supports('defer')) {
if (__DEV__) {
queries.forEach(query => {
warning(
Expand Down Expand Up @@ -202,7 +202,7 @@ function runQueries(
});
}

splitAndFlattenQueries(queries).forEach(query => {
splitAndFlattenQueries(storeData, queries).forEach(query => {
var pendingFetch = storeData.getPendingQueryTracker().add(
{query, fetchMode, forceIndex, storeData}
);
Expand Down
13 changes: 7 additions & 6 deletions src/legacy/store/__tests__/GraphQLQueryRunner-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ jest

const Relay = require('Relay');
const RelayFetchMode = require('RelayFetchMode');
const RelayNetworkLayer = require('RelayNetworkLayer');
const RelayStoreData = require('RelayStoreData');
const RelayTestUtils = require('RelayTestUtils');

Expand All @@ -30,6 +29,7 @@ const splitDeferredRelayQueries = require('splitDeferredRelayQueries');
const warning = require('warning');

describe('GraphQLQueryRunner', () => {
var networkLayer;
var queryRunner;
var pendingQueryTracker;

Expand Down Expand Up @@ -62,14 +62,15 @@ describe('GraphQLQueryRunner', () => {
beforeEach(() => {
jest.resetModuleRegistry();

RelayNetworkLayer.injectNetworkLayer({
supports: () => true,
});

var storeData = new RelayStoreData();
networkLayer = storeData.getNetworkLayer();
queryRunner = storeData.getQueryRunner();
pendingQueryTracker = storeData.getPendingQueryTracker();

networkLayer.injectNetworkLayer({
supports: () => true,
});

mockCallback = jest.genMockFunction();
mockQuerySet = {
foo: getNode(Relay.QL`query{viewer{actor{id,name}}}`),
Expand Down Expand Up @@ -108,7 +109,7 @@ describe('GraphQLQueryRunner', () => {
it('warns and uses fallback when defer is unsupported', () => {
diffRelayQuery.mockImplementation(query => [query]);
checkRelayQueryData.mockImplementation(() => false);
RelayNetworkLayer.injectNetworkLayer({
networkLayer.injectNetworkLayer({
supports: () => false,
});

Expand Down
3 changes: 1 addition & 2 deletions src/mutation/RelayMutationQueue.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ const RelayMutationQuery = require('RelayMutationQuery');
const RelayMutationRequest = require('RelayMutationRequest');
const RelayMutationTransaction = require('RelayMutationTransaction');
const RelayMutationTransactionStatus = require('RelayMutationTransactionStatus');
const RelayNetworkLayer = require('RelayNetworkLayer');
import type RelayStoreData from 'RelayStoreData';
import type {FileMap} from 'RelayMutation';
import type RelayMutation from 'RelayMutation';
Expand Down Expand Up @@ -238,7 +237,7 @@ class RelayMutationQueue {
transaction.getQuery(this._storeData),
transaction.getFiles(),
);
RelayNetworkLayer.sendMutation(request);
this._storeData.getNetworkLayer().sendMutation(request);

request.getPromise().done(
result => this._handleCommitSuccess(transaction, result.response),
Expand Down
52 changes: 25 additions & 27 deletions src/mutation/__tests__/RelayMutationQueue-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,19 +30,17 @@ const flattenRelayQuery = require('flattenRelayQuery');
const fromGraphQL = require('fromGraphQL');

describe('RelayMutationQueue', () => {
var RelayNetworkLayer;
var storeData;
var mutationQueue;
var networkLayer;

beforeEach(() => {
jest.resetModuleRegistry();

RelayNetworkLayer = jest.genMockFromModule('RelayNetworkLayer');
jest.setMock('RelayNetworkLayer', RelayNetworkLayer);

RelayStoreData.prototype.handleUpdatePayload = jest.genMockFunction();
storeData = RelayStore.getStoreData();
mutationQueue = storeData.getMutationQueue();
networkLayer = storeData.getNetworkLayer();

jasmine.addMatchers(RelayTestUtils.matchers);
});
Expand Down Expand Up @@ -178,9 +176,9 @@ describe('RelayMutationQueue', () => {
{onSuccess: successCallback1}
);
transaction1.commit();
expect(RelayNetworkLayer.sendMutation.mock.calls.length).toBe(1);
expect(networkLayer.sendMutation.mock.calls.length).toBe(1);

var request = RelayNetworkLayer.sendMutation.mock.calls[0][0];
var request = networkLayer.sendMutation.mock.calls[0][0];
request.resolve({response: {'res': 'ponse'}});
jest.runAllTimers();
expect(successCallback1.mock.calls).toEqual([[{'res': 'ponse'}]]);
Expand All @@ -200,8 +198,8 @@ describe('RelayMutationQueue', () => {
var mockError = new Error('error');
transaction1.commit();

expect(RelayNetworkLayer.sendMutation.mock.calls.length).toBe(1);
var request = RelayNetworkLayer.sendMutation.mock.calls[0][0];
expect(networkLayer.sendMutation.mock.calls.length).toBe(1);
var request = networkLayer.sendMutation.mock.calls[0][0];
request.reject(mockError);
jest.runAllTimers();
expect(failureCallback1).toBeCalled();
Expand All @@ -225,17 +223,17 @@ describe('RelayMutationQueue', () => {
expect(transaction2.getStatus()).toBe(
RelayMutationTransactionStatus.COMMIT_QUEUED
);
expect(RelayNetworkLayer.sendMutation.mock.calls.length).toBe(1);
expect(networkLayer.sendMutation.mock.calls.length).toBe(1);

var request = RelayNetworkLayer.sendMutation.mock.calls[0][0];
var request = networkLayer.sendMutation.mock.calls[0][0];
request.resolve({response: {}});
jest.runAllTimers();

expect(successCallback1).toBeCalled();
expect(transaction2.getStatus()).toBe(
RelayMutationTransactionStatus.COMMITTING
);
expect(RelayNetworkLayer.sendMutation.mock.calls.length).toBe(2);
expect(networkLayer.sendMutation.mock.calls.length).toBe(2);
});

it('does not queue commits for non-colliding transactions', () => {
Expand All @@ -245,15 +243,15 @@ describe('RelayMutationQueue', () => {
expect(transaction1.getStatus()).toBe(
RelayMutationTransactionStatus.COMMITTING
);
expect(RelayNetworkLayer.sendMutation.mock.calls.length).toBe(1);
expect(networkLayer.sendMutation.mock.calls.length).toBe(1);

var transaction2 = mutationQueue.createTransaction(mockMutation2);
transaction2.commit();

expect(transaction2.getStatus()).toBe(
RelayMutationTransactionStatus.COMMITTING
);
expect(RelayNetworkLayer.sendMutation.mock.calls.length).toBe(2);
expect(networkLayer.sendMutation.mock.calls.length).toBe(2);
});

it('does not queue commits for `null` collision key transactions', () => {
Expand All @@ -263,15 +261,15 @@ describe('RelayMutationQueue', () => {
expect(transaction1.getStatus()).toBe(
RelayMutationTransactionStatus.COMMITTING
);
expect(RelayNetworkLayer.sendMutation.mock.calls.length).toBe(1);
expect(networkLayer.sendMutation.mock.calls.length).toBe(1);

var transaction2 = mutationQueue.createTransaction(mockMutation3);
transaction2.commit();

expect(transaction2.getStatus()).toBe(
RelayMutationTransactionStatus.COMMITTING
);
expect(RelayNetworkLayer.sendMutation.mock.calls.length).toBe(2);
expect(networkLayer.sendMutation.mock.calls.length).toBe(2);
});

it('empties collision queue after a failure', () => {
Expand All @@ -292,7 +290,7 @@ describe('RelayMutationQueue', () => {
expect(transaction1.getStatus()).toBe(
RelayMutationTransactionStatus.COMMITTING
);
expect(RelayNetworkLayer.sendMutation.mock.calls.length).toBe(1);
expect(networkLayer.sendMutation.mock.calls.length).toBe(1);

var failureCallback2 = jest.genMockFunction().mockImplementation(
(transaction, preventAutoRollback) => {
Expand All @@ -313,9 +311,9 @@ describe('RelayMutationQueue', () => {
expect(transaction2.getStatus()).toBe(
RelayMutationTransactionStatus.COMMIT_QUEUED
);
expect(RelayNetworkLayer.sendMutation.mock.calls.length).toBe(1);
expect(networkLayer.sendMutation.mock.calls.length).toBe(1);

var request = RelayNetworkLayer.sendMutation.mock.calls[0][0];
var request = networkLayer.sendMutation.mock.calls[0][0];
request.reject(new Error('error'));
jest.runAllTimers();

Expand All @@ -334,7 +332,7 @@ describe('RelayMutationQueue', () => {
expect(transaction3.getStatus()).toBe(
RelayMutationTransactionStatus.COMMITTING
);
expect(RelayNetworkLayer.sendMutation.mock.calls.length).toBe(2);
expect(networkLayer.sendMutation.mock.calls.length).toBe(2);
});

it('rolls back colliding transactions on failure unless prevented', () => {
Expand Down Expand Up @@ -412,9 +410,9 @@ describe('RelayMutationQueue', () => {
expect(transaction5.getStatus()).toBe(
RelayMutationTransactionStatus.COMMIT_QUEUED
);
expect(RelayNetworkLayer.sendMutation.mock.calls.length).toBe(2);
expect(networkLayer.sendMutation.mock.calls.length).toBe(2);

var request = RelayNetworkLayer.sendMutation.mock.calls[0][0];
var request = networkLayer.sendMutation.mock.calls[0][0];
request.reject(new Error('error'));
jest.runAllTimers();

Expand Down Expand Up @@ -476,8 +474,8 @@ describe('RelayMutationQueue', () => {
);
transaction1.commit();

expect(RelayNetworkLayer.sendMutation.mock.calls.length).toBe(1);
var request = RelayNetworkLayer.sendMutation.mock.calls[0][0];
expect(networkLayer.sendMutation.mock.calls.length).toBe(1);
var request = networkLayer.sendMutation.mock.calls[0][0];
request.reject(new Error('error'));
jest.runAllTimers();

Expand All @@ -493,24 +491,24 @@ describe('RelayMutationQueue', () => {
);
transaction2.commit();

expect(RelayNetworkLayer.sendMutation.mock.calls.length).toBe(2);
expect(networkLayer.sendMutation.mock.calls.length).toBe(2);

transaction1.recommit();
expect(transaction1.getStatus()).toBe(
RelayMutationTransactionStatus.COMMIT_QUEUED
);

request = RelayNetworkLayer.sendMutation.mock.calls[1][0];
request = networkLayer.sendMutation.mock.calls[1][0];
request.resolve({response: {}});
jest.runAllTimers();
expect(successCallback2).toBeCalled();

expect(RelayNetworkLayer.sendMutation.mock.calls.length).toBe(3);
expect(networkLayer.sendMutation.mock.calls.length).toBe(3);
expect(transaction1.getStatus()).toBe(
RelayMutationTransactionStatus.COMMITTING
);

request = RelayNetworkLayer.sendMutation.mock.calls[2][0];
request = networkLayer.sendMutation.mock.calls[2][0];
request.resolve({response: {}});
jest.runAllTimers();

Expand Down
Loading

5 comments on commit 5fb67ed

@josephsavona
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@denvned and this!

@denvned
Copy link
Contributor Author

@denvned denvned commented on 5fb67ed Mar 7, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yay!!! Can we have a new point release of Relay that would please users of isomorphic-relay by resolving the last major issue for server-side rendering? 😉

@josephsavona
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@denvned Please be patient just a bit longer - I have some follow-up diffs that contextualize the remaining modules and need to refine the API & add documentation before we release this.

@josephsavona
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the meantime, feel free to try using master and let us know how that works!

@filod
Copy link

@filod filod commented on 5fb67ed Mar 8, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great work!

Please sign in to comment.