TaskCancelation => TaskCancellation?

[spencer516]

This feels like such a troll issue to open...so my apologies if it is.

It looks like, by some of your examples, that if we're putting a try/catch in our tasks, we can check for e.name === 'TaskCancelation' to determine if it was an error in the task vs. external cancellation for some other reason (maxConcurrency, manually canceled, etc).

But, should it be TaskCancelation or TaskCancellation? (link to dictionary.com which I had to use to check the spelling because I wasn't sure)

I can open a PR to fix if you'd like — but, but because of the potential breaking change nature of it, I figured I'd do a "wait and see".

[machty]

I decide a month or two ago that this was going to be a single L repo, whether it's in the code/api or in the documentation. Originally it was two Ls but enough stuff I read online suggested single L was more common in America. And then just now I read:

Canceled is the recommended spelling in Webster's 1898 dictionary. Likewise, The AP Stylebook prefers the use of cancel, canceled, and canceling, but it favors cancellation over cancelation.

Either way, it's definitely a really annoying thing to even have to think about, but I figure it's safest at this point to just stick with single L as the consistent rule unless it can be shown that lots of popular APIs go with double L.

[spencer516]

English is trolling even us english speakers. haha.

This is somewhat anecdotal data...but nevertheless searching "cancelation" on github produces 265k results in code:

...whereas "cancellation" produces 4.3 million.

Also, the conversation regarding the Promises A+ Cancellation spec is using the double L: https://github.com/promises-aplus/cancellation-spec/ (Also, while they weren't considering the same form, they did have a similar conversation)

(Did I say that I feel like such a troll for opening this issue? ...le sigh...anyway...thanks)

[machty]

hahah oh man

Well, I think I wanna keep it single L for now, but I do think there will be more robust ways in the future for detecting cancelation errors so you're not comparing strings.

Honestly it's pretty crappy API that we require the user to double check error.name (or error && error.name if it's falsy). Maybe there's a function we could expose that avoids the double L problem all together, like isCancel, that you can import. Hopefully we can think of a better name than isCancel.

[spencer516]

Yea — that sounds like a better approach in the long run. Cool beans.

(I'd close this; but not sure if you want to leave this open as a placeholder for that work?)

[machty]

You can leave it open. Would also appreciate your brainstorming on how such a function should be named. (and whether it should even exist)

[spencer516]

So you're thinking something along the lines of this...yes?:

import { task, isCancel } from 'ember-concurrency';

export default Ember.Component.extend({
  myTask: task(function *() {
    try {
      // do the thing
    } catch (e) {
      if (!isCancel(e)) {
        // Non cancellation error. 
      }
    }
  })
});

In terms of naming...isCancelError is the only name I can think of. Or, support both isCancelled and isCanceled so you don't have to have an opinion on spelling?

Or maybe just offer up TaskCancelation as an importable constant?:

import { task, taskStatus: { CANCEL } } from 'ember-concurrency';

//...
    if (e && e.name === CANCEL) {
//...

Contextual brainstorming question: I'm assuming we don't have access to the taskInstance from within the task's function, (so the only way to detect isCancel is in the catch block)? ...perhaps there's a use case where we would want to know in the finally block if the active task instance was cancelled? Aside from adding an argument to the end of the invoking-supplied arguments (yuck), I'm not sure how you could even get the instance into that context.

[Ramblurr]

I don't have a dog in the double l topic, but I would love to see a better API for catching errors.

I like the isCancel(e) method over a constant, as it does away with the undefined check and extra === syntax.

However conceptually I prefer wasCanceled(e) as it helps reinforce the whole task-is-something-that-happens-over-time idea.

[kellyselden]

I've always spelled it canceled FWIW. I think it's a case of the most popular is not necessarily correct. But how about the color of that shed?

[machty]

Closing this since didCancel was added around 0.7.0. The plan is to continue to avoid any public API where L vs LL can show up.