ericelliott / speculation

JavaScript promises are made to be broken. Speculations are cancellable promises.
MIT License
197 stars 9 forks source link

Static initialization method #10

Open atticoos opened 7 years ago

atticoos commented 7 years ago

This adds support for #9 by introducing speculation.fromPromise.

A dependent can compose a speculation instance by providing a promise task and a cancellation promise as two arguments, as well as an onCancel callback as a final third argument.

This affords the ability to work directly with existing promises and not have to interact with the Promise constructor whenever creating a speculation.

const wait = time => new Promise(resolve => setTimeout(resolve, time));

speculation.fromPromise(wait(20), wait(10))
.then(..)
.catch(..)

There's two questions I have with this:

  1. Does this belong within the promise, or should the dependent be responsible composing these ways to initialize a speculation? I find this to be a useful utility.
  2. Would we prefer a different interface, such as:
    speculation.fromPromise(cancellationPromise, onCancelled)(taskPromise)

    I don't see much gain from this, as the cancellationPromise may only be used once. So it wouldn't be able to compose more than one usage of the speculation instance.

atticoos commented 7 years ago

Thanks for the feedback. Are we good on the module naming of fromPromise?

ericelliott commented 7 years ago

Yes

ericelliott commented 7 years ago

Were you planning to make the requested changes?

atticoos commented 7 years ago

Ty for the bump - yes, had to wait for this weekend due to an intense work week

atticoos commented 7 years ago

One issue I discovered is that onCancel must throw an error for speculation to work properly.

Currently, the only way to to reject is when onCancel catches a rejection from handleCancel.

I'm wondering if cancel.then should incorporate default promise rejection as well. cancel is expected to resolve to stop the pending promise early, but it will only happen if onCancel throws an error.

I'm imagining along the lines of:

var handleCancellation = function (value) {
  if (isSettled) return;

  try {
    handleCancel(value);
    reject(SPECULATION_CANCELLED); // ensure a rejection happens by default
  } catch (e) {
    reject(e); // if `handleCancel` throws, reject with that error
  }
}

return cancel.then(handleCancellation, noop)

The first test has a false positive. It rejects because handleCancel is not defined, yet we call it either way

ericelliott commented 7 years ago

My intention was that the user would call reject() in their own onCancel() handler. See the example in the docs. Maybe that's not necessary, though. Maybe we can always reject?