Allow accessing encapsulated task state

[chancancode]

From discord:

@andreyfel : Hi @chancancode! I'm so excited to try out the new typings for e-c! Could you please advise how to deal with encapsulated task and the inner properties? In this example http://ember-concurrency.com/docs/encapsulated-task/ you can access uploadFile.last.progress. If I try to do in from TS, the progress field doesn't exist in the uploadFile.last, because it is just a TaskInstance which doesn't have such field.

Had I known that this is a thing, I would probably have written the types a bit differently, but this should work and is non-breaking.

I didn't make EncapsulatedTaskInstance public because I had this lingering suspicious that we'll need to add a second generic parameter to TaskInstance for the args. I don't want to lockdown that possibility, or deal with strangely mismatched ordering of parameters between TaskInstance vs EncapsulatedTaskInstance, so I kept that private.

For the time being, the public way to type it (as seen in the tests) is TaskInstance<T> & { ... }, which I think is plenty acceptable.

Got to update e-c-async and e-c-ts to account for this, then update the Octane tests.

cc @andreyfel

[andreyfel]

Hi @chancancode! Thank you for this PR! It seems that it is covering my use case! I've tested against that branch and it works!

The only thing I'm missing now is how to type this within the perform function of the encapsulated task. If I want to set a property on task instance inside perform what type does it have? EncapsulatedTaskInstance?

[chancancode]

I don't think we should block on making EncapsulatedTaskInstance public. It is very rare that you would have to type it (please show some examples – most cases I know of are either implicit or can be inferred, and I think it's rare that you would want to use this in a parameter or return position).

I do think people will copy the type alias, and I think that's not ideal but not so bad. If we a public one, it will be compatible with and won't break these existing usages.

I do think we should work towards making it public, we just have to address the other blocker first– whether we need to add the second generic to TaskInstance – and that's far from a hypothetical/theoretical issue. I don't think this PR is a good place to address that but we can also come to a decision on that fairy soon and fix both.

In the meantime, I think this adds enough values to those who need the feature and it's not obvious to me that they would be inconvenienced by this during normal usage.

[chancancode]

The only thing I'm missing now is how to type this within the perform function of the encapsulated task. If I want to set a property on task instance inside perform what type does it have? EncapsulatedTaskInstance?

Why did you have to type it? There are tests that confirms the this inference works correctly: https://github.com/chancancode/ember-concurrency/blob/encapsulated-task-v2/tests/types/ember-concurrency-test.ts#L2347-L2348

Is that not working for you for some reason?

[andreyfel]

@chancancode, yes, it works in a basic case. But I have a little bit more complex case. The idea is to have a task which exposes a property isRunningLong which should turn true in case if the task takes longer than loadingTimeout. I'm using an encapsulated task with an inner task for it:

export interface LongRunningTaskDescriptor<T, Args extends any[]> extends EncapsulatedTaskDescriptor<T, Args> {
  isRunningLong: boolean,
  timeoutTask: TaskProperty<void, []>,
}

export function longRunningTask<T, Args extends any[]>(fn: TaskFunction<T, Args>, loadingTimeout = 300): LongRunningTaskDescriptor<T, Args> {
  return {
    isRunningLong: false,

    timeoutTask: task(function * (this: LongRunningTaskDescriptor<T, Args>) {
      yield timeout(loadingTimeout);
      set(this, 'isRunningLong', true);
    }).drop(),

    * perform(...args): TaskGenerator<T> {
      try {
        this.timeoutTask.perform();
        return yield * fn.apply(this.context, args);
      } finally {
        this.timeoutTask.cancelAll();
        set(this, 'isRunningLong', false);
      }
    },
  };
}

It is used like this with new e-c and e-c-d:

@task({ restartable: true })
fetchSmth = longRunningTask(function * () {
    yield fetchSmth();
}, 600)

So, I have to specify type of this for the timeoutTask. And inside the perform function this.timeoutTask.perform doesn't work. And taskFor doesn't work here. Also typescript says that this.context doesn't exist. I'm not sure if it is a type issue or context is not a public filed.

[maxfierke]

context is sort of "intimate", undocumented API. It's useful for implementing extensions onto e-c, so perhaps its worth making public but its not something we'd expect people to use often (though with encapsulated tasks, it might make more sense.)

I think a composition like that to create an encapsulated task is a bit of an advanced use-case, so if it's working in the basic case, it may be worth deferring for later unless addressing it is fairly straightforward.