Add promise to `commitUpdate` and `applyUpdate`

[nodkz]

Right now Relay.Store have such syntax for commitUpdate method

static commitUpdate(mutation: RelayMutation, callbacks: {
  onFailure?: (transaction: RelayMutationTransaction) => void;
  onSuccess?: (response: Object) => void;
}): RelayMutationTransaction

// Argument to `onFailure` callback
type Transaction = {
  getError(): ?Error;
}

It will be cool, if commitUpdate and applyUpdate also returns promise, to avoid callback hell.

// Current version
Relay.Store.commitUpdate(
  new AddUserMutation({ login, age }), {
    onSuccess: () => { this._userAdded(login); },
    onFailure: () => { this._someProblems(login); },
  }
);

// Promise-based version
Relay.Store.commitUpdate(new AddUserMutation({ login, age })
.then( response => this._userAdded(login) );
.catch( err =>  this._someProblems(login) );  // err.getTransaction() for getting RelayMutationTransaction

So with promises I get more understandable code and can clearly chain several mutations. Thoughts?

[nodkz]

Due latest meeting notes this issue is more related to @wincent. Greg, can you provide such great feature?

I can wait next vim screencast 😉. But I can not live without promises with committing mutations 😜!

[wincent]

can clearly chain several mutations

What exactly do you mean? Generally you create one mutation, run it, and throw it away. Do you have a example scenario in mind?

[josephsavona]

I understand the desire to avoid callback hell, but mutations don't really fit the mold of exactly 1 callback for success/failure. For example, mutations support cancellation (rollback), and may need more notifications than just success/failure: there's a proposal for onProgress style callbacks for file uploads (#789) and an onOptimistic style callback for when the optimistic payload is applied.

If anything, an Observable interface seems more appropriate here. Perhaps something like applyUpdate(mutation): Transaction where

type RelayEnvironment = {
  ...
  applyUpdate(mutation): Transaction;
};
type Transaction = {
  rollback(): void;
  commit(): void;
  ...
  subscribe({onCompleted, onError, onNext}): Disposable;
}

The next callback would be unused in the current implementation but provide a place for optimistic or file upload notifications.

I'd encourage you to wrap applyUpdate/commitUpdate in a function that returns a Promise and send a gist with what that looks like and how it helps in product code so we can discuss with more context.

[nodkz]

Example scenarios: 1) Oauth authorization via winchan, when promise succeed, then call createUserMutation, after it success can be called other actions. And at the end catch error.

2) Checkout order. Again via winchan open payment gateway, and after success do some mutations or querying data.

3) I think on react-native also can be found such situations, where need wait user or device reaction via promise. And introduce mutation to such promise chain.

4) My simple current scenario, add emails to user private area. When mutation in progress, I add email to list via optimistic update and turn on/off spinner via _addPending/_removePending.

_addPending = (email) => {
  const pendingEmails = this.state.pendingEmails.slice();
  if (pendingEmails.indexOf(email) === -1) {
    pendingEmails.push(email);
    this.setState({ pendingEmails }); // changing state to show spinner, instead of trash button
  }
};

...

cabinetAddEmail = (email) => {
  this._addPending(email);

  Relay.Store.commitUpdate(
    new CabinetAddEmailMutation({
      cabinet: this.props.cabinet,
      email,
    }), {
      onSuccess: () => { this._removePending(email); },
      onFailure: () => { this._removePending(email); },
    }
  );
};

So first of all, I found in docs this two callbacks onSuccess, onFailure and can not understood why you not provide promise for mutations (internally network layer works on promises, but in components API used simple CB).

So Joseph perfectly explained purpose of such solution: onOptimistic - very cool callback onProgress - this is great feature. I want work with files on next week, so now I know how I'll do progress in future. Yay!

And I totally agree, that applyUpdate should not be promise-based. But may be commitUpdate or for backward compatibility creating a new method promiseUpdate will be good catch for common tasks and nice code style in react-components.

cabinetAddEmail = (email) => {
  this._addPending(email);

  Relay.Store.promiseUpdate(new CabinetAddEmailMutation({
    cabinet: this.props.cabinet,
    email,
  }))
  .then(() => this._removePending(email))
  .catch(() => this._removePending(email));
};

PS. But after you have opened my eyes on applyUpdate (before I thought that it is redundant). I think how I can totally switch to it, for serving offline/repeat situations. Or may be somebody share their solution.

[josephsavona]

Thanks for the examples! It's always good toget more examples of concrete use cases. It looks like these can all be expressed by wrapping the call to apply/commit in a new Promise(...), which is the best approach for now if you want to use Promises.

Note that commitUpdate doesn't really fit the 1:1 request/response model of Promises, as there are other useful notifications we may want to provide (file upload, notifications when the mutation is queued due to a collision key, etc).

More generally, Promises are a very limiting API - no cancellation, no ability to return synchronously when that makes sense, only a single callback. Because of this, we've found callbacks to be the most robust and flexible solution until a better standard emerges.