Add [Symbol.dispose] alias to Disposable class

[DavidANeil]

With explicit resource management being added to the language in the near future (supported in TypeScript 5.2), it would be nice to be able to use the using keyword with closure-library Disposables.

I think this would be as simple as this change:

if (typeof Symbol === 'function' && typeof Symbol.dispose === 'symbol') {
  goog.Disposable.prototype[Symbol.dispose] = goog.Disposable.prototype.dispose;
}

I can make this change (with some actual comments) if no one objects.

[shicks]

While the general ideas are related, it's not clear to me that (in practice) this would actually help much. In particular, goog.Disposable objects tend to live for a medium-to-long amount of time, typically bounded by a "parent object"'s lifetime, which is a pretty different paradigm than a function scope. Given that, I don't see how linking these things would be useful.

In general, Disposable is a pretty archaic/crufty class that we're mostly trying to move away from, so I'm inclined to avoid making this sort of change to it. Can you provide some more concrete use cases in the context of an actual application that would benefit from this?

[DavidANeil]

Sure, and yeah we have a lot of Disposables that live a long time. The most obvious case is our unit tests:

it('should do stuff', () => {
    using injector = createInjector();
    const item = injector.get(UnitUnderTest);
   // test
});

vs.

it('should do stuff', () => {
    const injector = createInjector();
    const item = injector.get(UnitUnderTest);
    // test

    injector.dispose();
});

It would also be convenient in occasional functions that create short lived versions of classes that are disposable, just to do some processing, get a result, and return the result.

function f() {
    using someDisposable = new SomeDisposable();
    return someDisposable.someComputation();
}

vs.

function f() {
    const someDisposable = new SomeDisposable();
    const result = someDisposable.someComputation();
    someDisposable.dispose();
    return result;
}

Neither of these cases actually need the try {} finally {} behavior, as exceptions are rare and leaks aren't catastrophic, but having it certainly wouldn't harm anything.

[shicks]

Thanks for those use cases. Presumably Closure Compiler will want to transpile using syntax, so it will likely be polyfilling Symbol.dispose, which we'd want to somehow piggyback on. Given the coordination requirements, we need to do a bit of designing internally, but we'll be looking into it this next quarter or so.

[rictic]

While the general ideas are related, it's not clear to me that (in practice) this would actually help much. In particular, goog.Disposable objects tend to live for a medium-to-long amount of time, typically bounded by a "parent object"'s lifetime, which is a pretty different paradigm than a function scope. Given that, I don't see how linking these things would be useful.

I've been using standard disposables with using syntax lately, and they're quite useful for this case, but it took me a bit to wrap my head around this use case. DisposableStack, and its move method is key.

The first examples you see are pretty trivial, looking like:

async function getAnswer() {
  using eventSource = constructEventSource();
  using handler = new SourceHandler(eventSource);
  return handler.getAnswer();
} // `using` syntax means we can count on handler and eventSource to be disposed of, even if the function throws

But with DisposableStack, you can reliably make objects that compose other disposables:

class Answerer {
  constructor() {
    using localDisposables = new DisposableStack();
    const eventSource = localDisposables.use(constructEventSource());
    const handler = localDisposables.use(new SourceHandler(eventSource);
    this.handler = handler;
    // Here's the key point: we're moving the block-scoped disposables into
    // this.disposables. After the move, localDisposables is empty and already 
    // disposed, and the returned Answerer is now responsible for eventSource
    // and handler, and they'll be disposed when it is.
    //
    // But if the constructor throws an error, all the resources it created before
    // it threw will still be disposed of, because they're tracked by localDisposables
    // which owns them up until the end of the constructor.
    this.disposables = localDisposables.move();
  }

  [Symbol.dispose]() {
    this.disposables[Symbol.dispose]();
  }
}

[rictic]

I think the change should be something more like:

if (typeof Symbol?.dispose === 'symbol') {
  goog.Disposable.prototype[Symbol.dispose] = function() {
    this.dispose();
  };
}

That way, if a subclass overrides dispose, then Symbol.dispose will still behave the same as dispose. And I'm gonna see if we can get away without the wrapping if statement, since closure compiler now automatically polyfills Symbol.dispose.

[shicks]

This issue is being closed as a preparation step before archiving the repository. See the README for more detail.