Skip to content
This repository has been archived by the owner on Sep 26, 2019. It is now read-only.

Why was this proposal withdrawn? #70

Closed
SEAPUNK opened this issue Dec 15, 2016 · 61 comments
Closed

Why was this proposal withdrawn? #70

SEAPUNK opened this issue Dec 15, 2016 · 61 comments

Comments

@SEAPUNK
Copy link

SEAPUNK commented Dec 15, 2016

My apologizes in advance if I've missed something, but it seems this proposal has been abruptly withdrawn without much explanation. Is there a reason why?

@domenic
Copy link
Member

domenic commented Dec 15, 2016

This proposal experienced significant opposition from within Google and so I am unable to continue working on it.

@SEAPUNK
Copy link
Author

SEAPUNK commented Dec 15, 2016

That sucks. In theory, somebody else could pick this proposal back up and champion it though, right?

@domenic
Copy link
Member

domenic commented Dec 15, 2016

They could, but they would be blocked by other Googlers in TC39, so it would be fruitless.

@SEAPUNK
Copy link
Author

SEAPUNK commented Dec 15, 2016

Could you tell us why they are opposed to this, please?

@domenic
Copy link
Member

domenic commented Dec 15, 2016

I'm sorry, I cannot really participate in these discussions any more, for my own mental health; it has been draining enough pursuing the fight internally, and losing. (In addition to the plethora of issues opened here by various people who believe they have a superior proposal, which was a constant drain.) They'll have to speak for themselves.

I'll be unsubscribing from this thread and I ask that nobody @-mention me.

@SEAPUNK
Copy link
Author

SEAPUNK commented Dec 15, 2016

I see. Thank you, and I hope you get to feeling better!

@styfle
Copy link

styfle commented Dec 15, 2016

😞

I've been following this repo for awhile and seen all the noise of others.
It's a lot of pressure trying to get this one right.

I leave you with an internet hug 🤗

hugs

@leobalter
Copy link
Member

I'm sorry, I cannot really participate in these discussions any more, for my own mental health

I'm not @-mentioning you here, but I'm glad you're taking time to yourself and I know the efforts you invested on this in the meetings.

I hope you feel better soon.

@benjamingr
Copy link

🤗 feel well, people do appreciate the work you do.

@bigopon
Copy link

bigopon commented Dec 16, 2016

Found this link explanation

Can someone please show me the link to the counter arguments please ?

@landonpoch
Copy link

Counter arguments would be nice if anyone is aware of them. Sounds like some Google folk objected for some reason:

They could, but they would be blocked by other Googlers in TC39, so it would be fruitless.

I wonder what the Googlers objections were.

This proposal getting withdrawn seems a bit mysterious at the moment, and I know there are APIs (like fetch) that were hoping to get a solution soon.

@mqklin
Copy link

mqklin commented Dec 18, 2016

From which stage this propose was dropped?

@dinoboff
Copy link

stage 1?

@landonpoch
Copy link

@mqklin and @dinoboff yes it was stage 1

@mqklin
Copy link

mqklin commented Dec 18, 2016

@landonpoch thanks. Do you know any proposes that was dropped at stage 2 or upper? Sorry for the offtop

@ljharb
Copy link
Member

ljharb commented Dec 18, 2016

Object.observe was stage 2.

@landonpoch
Copy link

Generally speaking though it seems like at stage 2:

"The committee expects the feature to be developed and eventually included in the standard"

https://tc39.github.io/process-document/

@ljharb
Copy link
Member

ljharb commented Dec 18, 2016

Expectations are not certainties.

@Vheissu
Copy link

Vheissu commented Dec 19, 2016

This is sad news. I really believe that there needs to be support for cancelling a promise in the specification. The fact that employees of Google on the TC39 committee are able to block proposals like this kind of worries me. I thought the whole point of TC39 was to make unbiased decisions on what goes into Javascript.

As far as I could find, I couldn't find any real valid reasons as to why this proposal should not go through. I saw some of the other contributions and they were flawed, this was the only thought out proposal. Incredibly sad.

@ljharb
Copy link
Member

ljharb commented Dec 19, 2016

@Vheissu i'm sure by the next meeting in January, the reasons will be made apparent.

One of the important priorities in TC39 is to achieve consensus - which means that any employee of any member company can always block any proposal. It's far far better for anyone to be able to block, than to arrive at a spec that "not everyone" is willing to implement - this is, by a large large margin, the lesser evil, I assure you.

@adrianmcli
Copy link

@ljharb as much as I agree with what you're saying, it would be nice to have a little transparency so that we could at least know of the arguments against this proposal.

@ljharb
Copy link
Member

ljharb commented Dec 19, 2016

@adrianmcli it's worth waiting at least until the January TC39 meeting to learn what those are.

@ablakey
Copy link

ablakey commented Dec 19, 2016

Beyond all else, what raises alarm in me is the comment Domenic made about his mental health. Does anyone have any insight on what he means? I can empathize with how draining making a logical case can be when others truly believe in their alternatives the way you believe in yours. Is this that or is this something more?

Maybe this is just me not having spent time at a larger corporation, or as part of a technical committee of a major language, but anyone coming to feel like that as an outcome of trying to do their best seems ugly and unacceptable.

@nicotoumetis
Copy link

nicotoumetis commented Dec 19, 2016

It's possible that this was blocked in favour of Observables:
https://github.com/tc39/proposal-observable

(I don't yet have an opinion on the merits such a decision if it was made for this reason. Observables bring quite a lot to the table, but they do also bring added complexity.)

@Robbert
Copy link

Robbert commented Dec 19, 2016

@ablakey Someones health is a persons own business, so it is best not ask — either them or third parties — to publicly share information about a private affair. That also includes speculating, you don't want people to feel the need to set the record straight about stuff that was no one elses business in the first place.

@ablakey
Copy link

ablakey commented Dec 19, 2016

@Robbert you're right. I should rephrase to not target a specific person or private matter, lest I accidentally push them to talk about it publicly. But if it's to be more general, I may as well raise the discussion in a different forum.

@caub
Copy link

caub commented Dec 19, 2016

I think the way a ReadableStream can be cancelled with fetch is good enough, simply reader.cancel()

@sstelfox
Copy link

Cancellation is a valuable primitive state as it allows preventing downstream work when other factors intervene. No longer need to render a report? Cancel the long and expensive request to the API. Saves resources on the client. The server can do the same thing. Client closed the connection before our long running database query finished? Stop the query to release the resources for other requests.

This proposal was well thought out and clean. I'm really looking forward to hearing the counter arguments.

@dotproto
Copy link

Just a quick thought regarding @tarruda's suggestion:

OTOH It seems the main argument in favor of the cancel state(according to the presentation) is that the following snippet is not very elegant:

try {
  await fetch(...);
} catch (e) {
  if (!(e instanceof CancelError)) {
    showUserMessage("The site is down.");
  }
} finally {
  stopLoadingSpinner();
}

I agree that his doesn't feel particularly elegant. But the bigger problem that sticks out to me is interoperability.

This approach may work well inside a single author's code, but there's no guarantee that multiple authors will share an implementation of a CancelPromise class (or whatever you call it). Symbols seem like a better fit, but even if the community standardize on something like const CancelPromise = Symbol.for('CancelPromise') there would still be some outliers that weren't familiar with the convention.

@balefrost
Copy link

@tarruda Actually, adding "canceled" would be a fourth state: { in-flight, succeeded, rejected, canceled }.

OK, so you could combine rejected and canceled. Fine, but you can go further. Why not combine succeeded and rejected, too? Essentially, a promise is in one of two states: { in-flight, completed }. When it transitions from in-flight to completed, user code needs to be invoked. Whether the promise succeeded or failed could be carried in the completion payload. Success and error completions could be handled via convention, similar to how Node callbacks typically accept errors in their first parameter.

The argument for keeping succeeded separate from rejected is that it's extremely convenient. But it's certainly not essential. Regarding cancellation, the question is really whether the convenience of making it first-class outweighs the conceptual complexity.

@ghost
Copy link

ghost commented Dec 19, 2016

The process for the TC39 committee is here: https://tc39.github.io/process-document/

The champion of this proposal withdrew it and maybe there is some other champion within the committee who will pick it up. If there are objections to this proposal it would be much more transparent to have had a vote on it rather than have its champion withdraw it. It's a shame that the committee member who was the champion was stressed out and it looks like burnt out by the discussion around this proposal.

I would recommend that everyone who has an opinion on this to post a blog somewhere about it and continue the discussion there or on Freenode IRC (something like channel #tc39-promises?). If someone has a really strong opinion they should continue the work; polyfills are something we can live with in the JavaScript world, what with babel and other transpilers around. When the issue is closed the conversation here will be closed but definitely should continue elsewhere.

What I like about TC39 is that they are making an effort to have polyfills to make it possible to use these new language features today and this proposal wasn't any different. So instead of idle talk, we can download the code, try it ourselves and then write a blog about it with all the technical details and reasoning as to why we think this should be picked up by another champion or why you think it's ok to drop this proposal.

@tarruda
Copy link

tarruda commented Dec 19, 2016

@sstelfox you made some good points as to why cancelling some async operations can be useful, but can you give an argument as to why it needs to be part of the promise API?

I'm really looking forward to hearing the counter arguments.

  • A third state is an unnecessary when cancellation can already be represented as a special case of accept/reject.
  • Cancellation does not make sense for every async operation. For example, would it make sense cancelling a database fetch or a fast filesystem operation(eg: fs.stat)?
  • Programming languages have to be careful when adding new features/APIs(Promises now being part of javascript). There's always a risk someone will come up with a better solution in the future, deprecating the use of Promise.cancel after a bunch of code already depends on it, at which point it can no longer be removed.
  • Has no direct correspondence with synchronous programming(async/await, as explained in the presentation). For example, how do you cancel await timeout(5000);? It would be possible with a intermediary object used to manage the timer:
let timer = new Timer();
emitter.on('some-event', () => timer.cancel());
await timer.timeout(5000);

But having a standardized Promise.cancel() brings nothing to the table here, as only the implementation of Timer is calling it directly.

@tarruda
Copy link

tarruda commented Dec 19, 2016

@tarruda Actually, adding "canceled" would be a fourth state: { in-flight, succeeded, rejected, canceled }.

OK, so you could combine rejected and canceled. Fine, but you can go further. Why not combine succeeded and rejected, too? Essentially, a promise is in one of two states: { in-flight, completed }. When it transitions from in-flight to completed, user code needs to be invoked. Whether the promise succeeded or failed could be carried in the completion payload. Success and error completions could be handled via convention, similar to how Node callbacks typically accept errors in their first parameter.

The argument for keeping succeeded separate from rejected is that it's extremely convenient. But it's certainly not essential.

Agreed.

Regarding cancellation, the question is really whether the convenience of making it first-class outweighs the conceptual complexity.

IMHO it doesn't, mainly because it doesn't fit into synchronous programming style using async/await, which is already seeing widespread use.

@landonpoch
Copy link

But having a standardized Promise.cancel() brings nothing to the table here

@tarruda This proposal did not include a "standardized Promise.cancel()" method. It was based on cancelation tokens. It was also very compatible with async / await syntax IMO.

@sstelfox
Copy link

@tarruda I agree that languages have to be careful but that's what these discussions are for. I also agree not all operations can or should be cancellable, but this mechanism doesn't require all promises to support cancellation.

The proposal here is to provide a standardized mechanism of cancelling in flight async requests, and allow for specific callbacks to (optionally) be triggered under the circumstances. In a lot of cases neither success or rejected are correct ways to reason about something that no longer needs to happen.

Quite a few developers will take a success message to trigger additional processing, storage, event notifications and will not take into account the case where the information is no longer needed. Perhaps this is an oversight on their part, but I don't personally consider it reasonable to assume cancelled as a successful state. Similarly with a rejected state, this is most commonly associated with an error state which cancellation isn't. It was a designed and intended action and shouldn't go through error handling logic while still allowing a path through a cleanup and optional callback mechanism.

The cancellation token mechanism allows for a consistent way for users of the language to chain these cancellations together as well downstream through libraries and chains of events, controlled by the original caller. You can create a CancelToken passing it through a tree of calls and upon triggering of that cancellation handle, it will propagate to the leafs of the call tree and cease performing any needless work that supports the mechanism.

By having that a standard part of the language, library and framework authors can implement it as an optional but consistent feature to save resources.

@rictic
Copy link

rictic commented Dec 19, 2016

I'm sad that this was withdrawn. I was just discovering this proposal and getting excited about it. I hope that my feedback didn't contribute to the stress.

When I added cancellation to the polymer static analyzer I found that I didn't really need much. cancelToken.throwIfRequested() and a clear way of distinguishing a Cancel from a normal exception was sufficient.

I put together a polyfill of the parts of this proposal that don't require new syntax here:

Feedback very welcome.

@finnigantime
Copy link

finnigantime commented Dec 19, 2016

So, the "cancellation tokens" proposal is being withdrawn here. What about promise cancellation in general? We rely heavily on cancellation in our client apps, and we use the https://github.com/petkaantonov/bluebird library for promises that support cancellation. We won't move to built-in JS promises until cancellation support is there.

@tarruda I think it makes a lot of sense for cancellation to be a separate API, for developer convenience.

Example cancellation scenarios for us:

  • aborting a chain of async operations for building up UI when that UI gets disposed (e.g. when navigating away from a page)
  • aborting animations (since our visual transitions are modeled as promises)
  • aborting service requests

We were originally on Bluebird2 which bundled cancellation with rejection, which gave us cancellation but it was a huge pain to use. We encountered so many subtle bugs that we ended up writing some utility code to work around them. Also, developers had to consciously think about cancellation whenever writing a rejection handler or calling cancel(), since cancellation would bubble up as an unhandled promise rejection if we didn't explicitly handle it. And it was a pain to unit test all these cases.

Bluebird3 made things a lot easier for us by making cancellation a separate top-level concept with different semantics than rejection - see changes here: http://bluebirdjs.com/docs/api/cancellation.html

Now, our rejection handlers just have to worry about rejection, and when we call cancel() we don't have to worry about it being unintentionally raised as rejection, or subtle timing issues caused by cancellation being async. The separate syntax is nice, but the main reason I see for having cancellation be a separate concept is the semantic differences.

@btnwtn
Copy link

btnwtn commented Dec 19, 2016

tfw google has a monopoly on JS.

@rictic
Copy link

rictic commented Dec 19, 2016

If cancellation was baked into promises from the start then promise.cancel() would be pretty nice, but if we added it in today it would break existing code. e.g. this is a fairly common pattern:

class PromiseCache {
  constructor() {
    this._cache = new Map();
  }
  get(key) {
    if (!this._cache.has(key)) {
      this._cache.set(key, this._compute(key));
    }
    return this._cache.get(key);
  }
  async _compute(key) {
    // some work that computes a value
  }
}

That code is correct today, and if we added promise.cancel() and cancellation propagated to a promise in the cache, then it in effect cancels all future requests for the corresponding key.

Breaking the invariants of existing code is a non-starter here, which pretty much leaves us with cancellation causing either a resolve or a reject with a special Cancel object. Thus, CancelTokens and Cancel objects that are kinda like exceptions.

@tarruda
Copy link

tarruda commented Dec 19, 2016

I missed the idea on the first read, sorry for the confusion regarding async/await. Now I can certainly see how powerful the concept of cancellation tokens can be for chaining async operations that can be cancelled.

Still, making changes to the language or Promise API(adding "cancelled" state) seems unnecessary, more so when you consider that most of the benefits of cancellation tokens come from following an API convention and not from changes in the existing tools. Maybe I'm missing something, but can someone highlight what the proposal would bring to the following example?

async function job1(token) {
  let result = await job2(token);
  if (token.cancelled) return;
  return result;
}

async function job2(token) {
  let result = 0;
  for (let i = 0; i < 1000000; i++) {
     result += await someAsyncComputation();
     if (token.cancelled) return;
  }
  return result;
}

async function job(maxTimeout) {
  let token = new CancelToken(async (cancel) => {
    await timeout(maxTimeout);
    cancel();
  });
  let result = job1(token);
  if (token.cancelled) {
    throw new Error("timed out");
  }
  return result;
}

This is indeed a powerful pattern, but it can done without any changes to the language or ties with the promise API.

@prabirshrestha
Copy link

@tarruda Based on the above sample, if someAsyncComputation was an xhr request it can't be aborted. It is just that the caller doesn't get the response.

@mjerez-radical
Copy link

mjerez-radical commented Dec 19, 2016

As far as i know you can't cancel a promise, you either success or fail miserably, but you can not step back and say, hey no I was joking it wasn't really a promise!

P.S. just joking

@caffeinewriter
Copy link

@mjerez-radical My thought on this is success means the promise has fulfilled its obligation, failure means it was unable to, and cancellation means the original requestor of a promise no longer needs it, so it's pointless for the promise to continue to try and fulfill it. It's not really "breaking a promise", as much as it is a state that indicates that the result/sequence of events is no longer needed.

@born2net
Copy link

born2net commented Dec 19, 2016

my thoughts are that things are being pushed toward rxjs (functional programming, and it is part of TC) thus the reluctance to add features around promises...

@dtipson
Copy link

dtipson commented Dec 19, 2016

The only true "original" requestor is really the constructor itself. And it, by current design, doesn't expose any external api resulting from the async operation, nor does it yet have a reason to cancel its own operation. The only thing it externally offers is an interface for chaining on things that depend on its eventual state. So either you have to statefully keep track of all of the things later chained onto it (like Bluebird does) or smuggle an out-of-scope hook for cancelation into the scope of the original constructor somehow (like cancel tokens do).

@oallouch
Copy link

I personally like the simple Promise/async-await mapping: resolve is the returned value and reject throws an exception...and that's all

@MiracleBlue
Copy link

Wishing for domenics speedy recovery, I know just what a toll mental health issues can take on a person. It's tough. Thank you for your hard work.

@benjamingr
Copy link

@tarruda no offense, but people who come to the repo and don't read the prior discussion and reading list are causing a real problem and exhaust collaborators. Your whole discussion here in much more depth is available in different documents throughout this repo together with motivating usage examples that show the design decision. In addition this proposal has already majorly changed 3 times and cancellation semantics to reach "consensus" at least 4 times in its existence.

I know you're just trying to help - but this is why people like Domenic get so worn out from this work.

I don't do nearly as much spec work as he does but I feel it too.

@benjamingr
Copy link

And everyone dissing Google here for shutting down the proposal without knowing the reasons involved, people stop. It's hurtful.

Google is full of people (Including Domenic btw, who wrote this proposal) and disagreement is taken very seriously. If an implementor won't implement a specification that's a problem for everyone - not just that implementor - it could have just as easily been Mozilla, Microsoft or Apple.

@chrisregnier
Copy link

Instead of trying to deal with all the problems associated with adding a cancelled state, why not side step them by working with how promises work right now. Just make Promise.cancel a back propagating resolve/reject?

let p = fetch().then(r => console.log('resolved')).catch(r => console.err('rejected'));
p.cancel('fake response'); // prints 'resolved'
p.cancel(new Error('uhoh')); //prints 'rejected'

The key here is the caller is responsible for what should get passed down the chain, which solves many of the problems posed by whoever cancels things.
So cancel(Error) passes the error down the rejection path, a function cancel((resolve, reject) => {}) allows either, and anything else goes down the resolved path.

The other important thing here is that it should work it's way back through the chain until it finds a CancellablePromise, thus allowing a way for multiple consumers to still work properly.

let onCancelled = (cancelledValue, resolve, reject, cancel) => {}
let f = fetch();
let cancellable = f.then(onFulfilled, onRejected, onCancelled);
let c1 = cancellable.then(r => console.log('c1'));
let c2 = cancellable.then(r => console.log('c2'));

c1.cancel(); // passes undefined to onCancelled's cancelledValue and does nothing
f.cancel(); // resolves fetch with undefined, passing it down the chains, eventually prints c1 & c2

And the default onCancelled on non cancellable promises would be
(cancelledValue, resolve, reject, cancel) => { cancel(cancelledValue) }
which just back propagates the cancelledValue up the chain.

@benjamingr
Copy link

@chrisregnier that was this proposal a while ago. See the history and discussions in the git history - for example: #8.

Also please refer to #70 (comment) . Thanks.

People - please, if you have a proposal bring it up at esdiscuss (https://esdiscuss.org/) and not here.

@indolering
Copy link

indolering commented Dec 19, 2016

Great recap over at HN:

Promises have always been extremely contentious in JS for a couple reasons and Domenic has had the patience of a saint getting the original proposal through so it's totally understandable that he might not want to deal with this anymore especially if people in his org are against it.
There were a couple reasons the original promises were so contentious to standardize.

  • Monads: there was a very vocal faction on TC-39 that really wanted a Promises to be monadic and very much emphasized mathematical purity over usefulness and any attempt to discuss things tended to get side tracked into a discussion about monads, see brouhaha over Promise.cast [1].
  • Error handling: Promises have a bit of a footgun where by default they can swallow errors unless you explicitly add a listener for them. As you can theoretically add an error listener latter on all other options had downsides (not always obvious to advocates) leading to a lot of arguing, must of it after things had shipped and realistically couldn't be changed. Fixed by platforms and libraries agreeing on a standardization of uncaught promise events [2].
  • Just an absurd amount of bike shedding probably the 2nd most bike shedded feature of the spec (behind modules).
    So cancelable promises reignite all the old debates plus there's no obvious right way to do it, like should it be a 3rd state which could cause compatibility issues and would add complexity, or should cancellations just be a sort of forced rejection, which would be a lot more backwards compatibility but with less features.
    Additionally there is a bizarre meme that somehow observables (think event emitters restricted to a single event or synchronous streams that don't handle back pressure) are a replacement for promises or are a better fit for async tasks they should be used instead.
    edit: patients => patience
  1. https://esdiscuss.org/topic/promise-cast-and-promise-resolve
  2. https://gist.github.com/benjamingr/0237932cee84712951a2

@benjamingr Please close this thread and restrict commenting.

@benjamingr
Copy link

@indolering I don't have the ability to do so and even if I did - I'm not a TC39 member and would defer to a member anyway. I appreciate the linking to my gist in that comment - it's very nice to be recognized for pushing for things that help people.

@wycats wycats closed this as completed Dec 20, 2016
@wycats
Copy link

wycats commented Dec 20, 2016

I closed comments and restricted to members.

@tc39 tc39 locked and limited conversation to collaborators Dec 20, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests