A subclass that overrides DisposableStack#dispose must also override DisposableStack#[Symbol.dispose]

[rictic]

Consider code like:

class NotifyingStack extends DisposableStack {
  dispose() {
    const wasDisposed = this.disposed;
    super.dispose();
    if (!wasDisposed) {
      notify();
    }
  }
}

By my reading of https://tc39.es/proposal-explicit-resource-management/#sec-disposablestack.prototype-@@dispose this code is buggy, notify won't be called when NotifyingStack.prototype[Symbol.dispose] is called, only when its dispose method is called. Such a subclass would need to also update NotifyingStack.prototype[Symbol.dispose]. This could be a potential footgun in the API.

Likewise AsyncDisposableStack and its disposeAsync, and Symbol.asyncDispose fields.

[rictic]

If Symbol.dispose was instead a getter on the prototype, or was defined as a function that called this.dispose(), that would alleviate the issue. We'd want to document in that case that users should override dispose and not Symbol.dispose.

[rbuckton]

In an earlier draft of the proposal, DisposableStack.prototype.dispose was actually a getter that returned a bound function invoked this[Symbol.dispose]. If we were to make a change, it would likely be in that direction since the runtime is going to invoke [Symbol.dispose](), not dispose(). That would also be more in line with what has been discussed in #229 (comment)

[rbuckton]

In the meantime, a potential workaround that matches the current spec behavior would be to do the following:

class NotifyingStack extends DisposableStack {
  dispose() {
    const wasDisposed = this.disposed;
    super.dispose();
    if (!wasDisposed) {
      notify();
    }
  }
  static { this.prototype[Symbol.dispose] = this.prototype.dispose; }
}

[rbuckton]

Also, is there a reason you are not just using .defer() in this case?

class NotifyingStack extends DisposableStack {
  constructor(notify) {
    this.defer(notify);
  }
}

[rictic]

Ha, that's elegant!

The actual use case I was evaluating was goog.Disposable, which has many subclasses that override its defer method, and wondering if it could ever extend DisposableStack (answer: prooooobably not? there's a number of relatively minor differences that are likely to be issues at scale. the foremost one I've noticed is that it disposes in FIFO order). I'm gonna try a bit though, starting by narrowing the differences by giving it a [Symbol.dispose]. If that works, I'll try improving its error handling with SuppressedError, then going FIFO -> LIFO. This issue isn't a blocker at all, just a potential footgun I noticed while evaluating.