Add support for async disposable

[joshamaju]

Motivation

Cleanup effection task at application/effection boundary to avoid hanging operations.

issue

Approach

Add support for native Javascript async disposables.

Alternate Designs

Possible Drawbacks or Risks

TODOs and Open Questions

Learning

Screenshots

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/thefrontside/effection@1073

commit: 10a2f26

[joshamaju]

How do we move this forward?

[cowboyd]

I agree, let's push this over the finish line.

1. Formatting changes: don't worry about it, I'll fix them up.
2. Question about the types
3. We should also add asyncDispose to Scope (along with a test)

[joshamaju]

Please review the scope feature

[cowboyd]

@joshamaju Do we need to document this? I'm not sure what the best practice is on documenting whether something is an explicitly manageable resource.

[cowboyd]

@joshamaju I removed the formatting changes, and moved the scope implementation into ScopeInternal.

The only remaining issue is documentation. I'm not sure how much is required for asyncDispose, and perhaps not at all.

Issues from initial review have been addressed

[taras]

We should add it to resource page to say that they're supported out of the box with this version.

[joshamaju]

I've added a section to the upgrade guide documentation

[cowboyd]

@taras I'm not sure the resources page isn't really the right place because resources are implicitly cleaned up. Only tasks and scopes implement AsyncDisposable, but not sure where the place to explain that is. The section on typescript maybe? Rosetta Stone?

[cowboyd]

@joshamaju What do you think about making the Array returned by createScope() actually implement the AsyncDisposable. This would allow us to say:

using [scope] = createScope();

[joshamaju]

Is that even possible? I don't think it's possible

[joshamaju]

So I just checked. Can only be used like this using scope = createScope();, and cannot be async, only Symbol.dispose.

[cowboyd]

Yeah, it's not possible to use destructing assignment with explicitly managed resources.... another reason I don't like them. I had an AI tell me it was though, and so I got my hopes up. 🤣🤣🤣

I just think it will be awkward to use without being able to do it with a one-liner. Maybe we need a new method?

using scope = disposableScope();

using scope = createDisposableScope();

something like those or another alternative? ☝🏻

[joshamaju]

Maybe createScope should return a object instead of a tuple?

[cowboyd]

I don't think that would work either, would it? We could make the object Iterable for backwards compatibility if it did.

[joshamaju]

Yep, it works

[joshamaju]

Should it be a separate function i.e createDisposableScope?

[cowboyd]

So we could define a scope property on the tuple?

[joshamaju]

How do you mean?

[cowboyd]

export function createScope(
  parent: Scope = global,
): [Scope, () => Future<void>] {
  let [scope, destroy] = createScopeInternal(parent);
  let tuple = [scope, () => parent.run(destroy)];
  let disposable = Object.defineProperty(scope, Symbol.asyncDispose, { value: () => parent.run(destroy) });
  Object.defineProperty(tuple, 'scope', { value: disposable });
}

This would then let us say:

using { scope } = createScope();

[cowboyd]

Yeah, it's like I thought. Binding expressions are not supported inside using declarations microsoft/TypeScript#55527

It's yet another example of how explicit resource management was railroaded through without concern over how much incongruence with existing patterns it would introduce.

[cowboyd]

I think we can still do it, but it means we really have to abuse the runtime and the type system to make it work. I think what we can do is define Symbol.iterator and Symbol.asyncDispose on the actual returned scope so that it can be used as a tuple and as an explicitly managed resource:

// 🤣 🤣 
declare function createScope(parent?: Scope): Scope & AsyncDisposable & [Scope, () => Promise<void>];

[cowboyd]

This is horrendous, but it works:

export function createScope(
  parent: Scope = global,
): Scope & AsyncDisposable & [Scope, () => Future<void>] {
  let [scope, destroy] = createScopeInternal(parent);
  let dispose = () => parent.run(destroy);
  let tuple = [scope, dispose];
  Object.defineProperty(scope, Symbol.iterator, {
    enumerable: false,
    value: tuple[Symbol.iterator].bind(tuple),
  });
  Object.defineProperty(scope, Symbol.asyncDispose, {
    enumerable: false,
    value: dispose,
  })
  return scope as unknown as Scope & AsyncDisposable & [Scope, () => Future<void>];
}

[joshamaju]

Is the goal that a user can use it in one of the following ways?

await using scope = createScope();

or like this

const [scope] = createScope();

[cowboyd]

@joshamaju yes, that's the idea, although the second way is a no-no. You should always capture the destroy() function if you use it the second way.

It is a bit disgusting, but it let's you consume it either way.