Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove the clientMutationId requirement by creating a new root id for each executed mutation #2349

Closed
wants to merge 4 commits into from
Closed

Remove the clientMutationId requirement by creating a new root id for each executed mutation #2349

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
81e0fb7
working modern mutations without clientMutationId
robrichard Feb 26, 2018
8f69073
Add test case that fails without dynamic root id
robrichard Feb 26, 2018
6bd5cd4
remove unused variable
robrichard Feb 26, 2018
8264a1e
friendlier mutationUidPrefix
robrichard Feb 28, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/react-relay/classic/environment/RelayCombinedEnvironmentTypes.js
View file
Open in desktop
Expand Up @@ -231,6 +231,7 @@ export interface CUnstableEnvironmentCore<
request: TRequest,
variables: Variables,
operation?: TOperation,
dataID?: DataID,
) => COperationSelector<TNode, TRequest>;

/**
Expand Down
6 changes: 3 additions & 3 deletions packages/relay-runtime/mutations/RelayRecordSourceProxy.js
View file
Open in desktop
Expand Up @@ -139,10 +139,10 @@ class RelayRecordSourceProxy implements RecordSourceProxy {
return this._proxies[dataID];
}

getRoot(): RecordProxy {
let root = this.get(ROOT_ID);
getRoot(dataID: DataID = ROOT_ID): RecordProxy {
let root = this.get(dataID);
if (!root) {
root = this.create(ROOT_ID, ROOT_TYPE);
root = this.create(dataID, ROOT_TYPE);
}
invariant(
root && root.getType() === ROOT_TYPE,
Expand Down
2 changes: 1 addition & 1 deletion packages/relay-runtime/mutations/RelayRecordSourceSelectorProxy.js
View file
Open in desktop
Expand Up @@ -54,7 +54,7 @@ class RelayRecordSourceSelectorProxy implements RecordSourceSelectorProxy {
}

getRoot(): RecordProxy {
return this.__recordSource.getRoot();
return this.__recordSource.getRoot(this._readSelector.dataID);
}

_getRootField(
Expand Down
187 changes: 176 additions & 11 deletions packages/relay-runtime/mutations/__tests__/commitRelayModernMutation-test.js
View file
Open in desktop
Expand Up @@ -17,6 +17,7 @@ const {createOperationSelector} = require('RelayModernOperationSelector');
const {generateAndCompile} = require('RelayModernTestUtils');
const {ROOT_ID} = require('RelayStoreUtils');
const {commitMutation} = require('react-relay/modern/ReactRelayPublic');
const ConnectionHandler = require('RelayConnectionHandler');

describe('Configs: NODE_DELETE', () => {
jest.resetModules();
Expand Down Expand Up @@ -106,14 +107,14 @@ describe('Configs: NODE_DELETE', () => {
];
const optimisticUpdater = jest.fn();
const updater = jest.fn();
const operationSelector = createOperationSelector(FeedbackCommentQuery, {});
environment.commitPayload(operationSelector, payload);
const snapshot = store.lookup({
dataID: ROOT_ID,
node: FeedbackCommentQuery.fragment,
variables: {},
});
const callback = jest.fn();
const operationSelector = createOperationSelector(FeedbackCommentQuery, {});
environment.commitPayload(operationSelector, payload);
store.subscribe(snapshot, callback);
commitRelayModernMutation(environment, {
configs,
Expand Down Expand Up @@ -417,7 +418,7 @@ describe('Configs: RANGE_DELETE', () => {
});
});

describe('Configs: RANGE_ADD', () => {
describe('Adding to connections', () => {
let callback,
CommentQuery,
data,
Expand Down Expand Up @@ -532,7 +533,7 @@ describe('Configs: RANGE_ADD', () => {
};
});

it('appends new edge', () => {
it('Configs: RANGE_ADD appends new edge', () => {
const configs = [
{
type: 'RANGE_ADD',
Expand All @@ -547,13 +548,13 @@ describe('Configs: RANGE_ADD', () => {
edgeName: 'feedbackCommentEdge',
},
];
const operationSelector = createOperationSelector(CommentQuery, {});
environment.commitPayload(operationSelector, payload);
const snapshot = store.lookup({
dataID: ROOT_ID,
node: CommentQuery.fragment,
variables: {},
});
const operationSelector = createOperationSelector(CommentQuery, {});
environment.commitPayload(operationSelector, payload);
store.subscribe(snapshot, callback);
commitMutation(environment, {
configs,
Expand All @@ -575,7 +576,7 @@ describe('Configs: RANGE_ADD', () => {
expect(callback.mock.calls.length).toBe(0);
});

it('does not overwrite previous edge when appended multiple times', () => {
it('Configs: RANGE_ADD does not overwrite previous edge when appended multiple times', () => {
const configs = [
{
type: 'RANGE_ADD',
Expand Down Expand Up @@ -742,7 +743,171 @@ describe('Configs: RANGE_ADD', () => {
});
});

it('prepends new edge', () => {
it('Updater function not overwrite previous edge when appended multiple times', () => {
const updater = (store) => {
const payload = store.getRootField('commentCreate');
const newEdge = payload.getLinkedRecord('feedbackCommentEdge');
const feedbackProxy = store.get(feedbackID);
const conn = ConnectionHandler.getConnection(
feedbackProxy,
'Feedback_topLevelComments',
);
ConnectionHandler.insertEdgeAfter(conn, newEdge);
};
// prepare existing data
const operationSelector = createOperationSelector(CommentQuery, {});
environment.commitPayload(operationSelector, {
node: {
id: feedbackID,
__typename: 'Feedback',
topLevelComments: {
count: 1,
edges: [
{
cursor: 'comment1:cursor',
node: {
id: 'comment1',
},
},
],
},
},
});

// send mutation
commitMutation(environment, {
updater,
mutation,
variables,
});

let serverResponse = {
data: {
commentCreate: {
feedbackCommentEdge: {
__typename: 'CommentsEdge',
cursor: 'comment2:cursor',
node: {
id: 'comment2',
// these are extra fields which should be stripped off before appending
// to the connection.
body: {
text: variables.input.message.text,
},
},
},
},
},
};
const node = environment.executeMutation.mock.calls[0][0].operation.node;
environment.mock.resolve(node, serverResponse);
jest.runAllTimers();

let snapshot = store.lookup({
dataID: ROOT_ID,
node: CommentQuery.fragment,
variables: {},
});
expect(snapshot.data).toEqual({
node: {
topLevelComments: {
edges: [
{
cursor: 'comment1:cursor',
node: {
__typename: 'Comment',
id: 'comment1',
},
},
{
cursor: 'comment2:cursor',
node: {
__typename: 'Comment',
id: 'comment2',
},
},
],
// The following fields are not quite related. Though not explicted requested in the query,
// Relay now automatically adds the page info.
pageInfo: {
endCursor: null,
hasNextPage: false,
},
},
},
});

serverResponse = {
data: {
commentCreate: {
feedbackCommentEdge: {
__typename: 'CommentsEdge',
cursor: 'comment3:cursor',
node: {
id: 'comment3',
// these are extra fields which should be stripped off before appending
// to the connection.
body: {
text: variables.input.message.text,
},
},
},
},
},
};
// send the same mutation again
commitMutation(environment, {
updater,
mutation,
variables,
});
environment.mock.resolve(node, serverResponse);
jest.runAllTimers();

snapshot = store.lookup({
dataID: ROOT_ID,
node: CommentQuery.fragment,
variables: {},
});

expect(snapshot.data).toEqual({
node: {
topLevelComments: {
edges: [
{
cursor: 'comment1:cursor',
node: {
__typename: 'Comment',
id: 'comment1',
},
},
{
cursor: 'comment2:cursor',
node: {
__typename: 'Comment',
id: 'comment2',
},
},
{
cursor: 'comment3:cursor',
node: {
__typename: 'Comment',
id: 'comment3',
},
},
],
// The following fields are not quite related. Though not explicted requested in the query,
// Relay now automatically adds the page info.
pageInfo: {
endCursor: null,
hasNextPage: false,
},
},
},
});
});

it('Configs: RANGE_ADD prepends new edge', () => {
const configs = [
{
type: 'RANGE_ADD',
Expand All @@ -757,13 +922,13 @@ describe('Configs: RANGE_ADD', () => {
edgeName: 'feedbackCommentEdge',
},
];
const operationSelector = createOperationSelector(CommentQuery, {});
environment.commitPayload(operationSelector, payload);
const snapshot = store.lookup({
dataID: ROOT_ID,
node: CommentQuery.fragment,
variables: {},
});
const operationSelector = createOperationSelector(CommentQuery, {});
environment.commitPayload(operationSelector, payload);
store.subscribe(snapshot, callback);
commitMutation(environment, {
configs,
Expand All @@ -785,7 +950,7 @@ describe('Configs: RANGE_ADD', () => {
expect(callback.mock.calls.length).toBe(0);
});

it('filters connections then applies the rangeBehavior', () => {
it('Configs: RANGE_ADD filters connections then applies the rangeBehavior', () => {
const configs = [
{
type: 'RANGE_ADD',
Expand Down
10 changes: 9 additions & 1 deletion packages/relay-runtime/mutations/commitRelayModernMutation.js
View file
Open in desktop
Expand Up @@ -48,14 +48,15 @@ function commitRelayModernMutation<T>(
'commitRelayModernMutation: expect `environment` to be an instance of ' +
'`RelayModernEnvironment`.',
);
const mutationUid = nextMutationUid();
const {createOperationSelector, getRequest} = environment.unstable_internal;
const mutation = getRequest(config.mutation);
if (mutation.operationKind !== 'mutation') {
throw new Error('commitRelayModernMutation: Expected mutation operation');
}
let {optimisticResponse, optimisticUpdater, updater} = config;
const {configs, onError, variables, uploadables} = config;
const operation = createOperationSelector(mutation, variables);
const operation = createOperationSelector(mutation, variables, undefined, mutationUid);
// TODO: remove this check after we fix flow.
if (typeof optimisticResponse === 'function') {
optimisticResponse = optimisticResponse();
Expand Down Expand Up @@ -110,4 +111,11 @@ function commitRelayModernMutation<T>(
});
}


let mutationUidCounter = 0;
const mutationUidPrefix = 'mutationRoot';
function nextMutationUid() {
return mutationUidPrefix + mutationUidCounter++;
}

module.exports = commitRelayModernMutation;
4 changes: 2 additions & 2 deletions packages/relay-runtime/store/RelayModernEnvironment.js
View file
Open in desktop
Expand Up @@ -197,7 +197,7 @@ class RelayModernEnvironment implements Environment {
.execute(operation.node, operation.variables, cacheConfig || {})
.do({
next: executePayload => {
const responsePayload = normalizePayload(executePayload);
const responsePayload = normalizePayload(executePayload, operation);
const {source, fieldPayloads, deferrableSelections} = responsePayload;
for (const selectionKey of deferrableSelections || new Set()) {
this._deferrableSelections.add(selectionKey);
Expand Down Expand Up @@ -303,7 +303,7 @@ class RelayModernEnvironment implements Environment {
}
this._publishQueue.commitPayload(
operation,
normalizePayload(payload),
normalizePayload(payload, operation),
updater,
);
this._publishQueue.run();
Expand Down
4 changes: 2 additions & 2 deletions packages/relay-runtime/store/RelayModernOperationSelector.js
View file
Open in desktop
Expand Up @@ -16,7 +16,7 @@ const RelayConcreteNode = require('RelayConcreteNode');
const {getOperationVariables} = require('RelayConcreteVariables');
const {ROOT_ID} = require('RelayStoreUtils');

import type {Variables} from '../util/RelayRuntimeTypes';
import type {Variables, DataID} from '../util/RelayRuntimeTypes';
import type {RequestNode, ConcreteOperation} from 'RelayConcreteNode';
import type {OperationSelector} from 'RelayStoreTypes';

Expand All @@ -30,6 +30,7 @@ function createOperationSelector(
request: RequestNode,
variables: Variables,
operationFromBatch?: ConcreteOperation,
dataID: DataID = ROOT_ID,
): OperationSelector {
const operation =
operationFromBatch ||
Expand All @@ -38,7 +39,6 @@ function createOperationSelector(
: request.operation);

const operationVariables = getOperationVariables(operation, variables);
const dataID = ROOT_ID;
return {
fragment: {
dataID,
Expand Down
2 changes: 1 addition & 1 deletion packages/relay-runtime/store/RelayStoreTypes.js
View file
Open in desktop
Expand Up @@ -186,7 +186,7 @@ export interface RecordSourceProxy {
create(dataID: DataID, typeName: string): RecordProxy;
delete(dataID: DataID): void;
get(dataID: DataID): ?RecordProxy;
getRoot(): RecordProxy;
getRoot(dataID?: DataID): RecordProxy;
}

/**
Expand Down