Change clientMutationID invariant to a conditional check to work with subscriptions

[eyston]

I'm working on #541 and handleRangeAdd in writeRelayUpdatePayload has an invariant on clientMutationID being in the payload. This won't be the case for a subscription payload configured as a RANGE_ADD.

From what I can tell the clientMutationID is used in order to link up a temporary node ID added by an optimistic mutation with the real one from the mutation payload. This won't be necessary for subscriptions.

I replaced the invariant with a conditional check on the clientMutationID. I also duplicated the test the for non-optimistic RANGE_ADD's without a clientMutationID which I'm not sure is necessary or not.

[eyston]

I updated the tests to use a subscription query to make it clear what functionality is being tested. This required adding a subscription root with a field. I'm not sure the proper naming conventions that would like to be maintained for the subscription schema but can change them to match. Also, Relay.QL requires subscriptions to have a single input with at least clientSubscriptionID so that was done. I'd be interested in the requirement of clientSubscriptionID if time permits :).

[josephsavona]

How about changing the outer if condition to if (operation instanceof RelayQuery.Mutation), and move the invariant on clientMutationId inside the if block? This will get the intended effect of not running the invariant for subscriptions, while keeping the validation for mutations.

[josephsavona]

@eyston Sorry for the delay in providing feedback: this is great. Other than the one piece of feedback above, this is ready to import.

[eyston]

Updated with feedback -- which was way smarter check condition. Also rebased off master to fix CI. I can squish this or something too, dunno conventions!

Sorry for delay myself. Busy week! Good times.

[josephsavona]

@eyston thanks for this, looks great and we'll import next week. There's no need to squash commits for a single PR, our tool imports the combined patch. Thanks for offering though ;-)

[Globegitter]

@josephsavona Any update on getting this merged? Same for #779.

[taion]

Following up on this – did this code ever get pulled in? Right now it's not possible to apply a RANGE_ADD subscription response without setting a dummy clientMutationId.

[edvinerikson]

I think we can solve this by moving: writeRelayUpdatePayload.js#L320-L326 to writeRelayUpdatePayload.js#L376-L383

like it is done here: RelayStoreData.js#L376-L383

[taion]

I feel like this PR does the trick pretty well, though.

[taion]

Ping! We're working around this in relay-subscriptions right now by adding a dummy clientMutationId that won't conflict, but it'd be good if we could remove that hack.