Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reintroduce bound dispose/disposeAsync getters #232

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

rbuckton
Copy link
Collaborator

@rbuckton rbuckton commented Jun 15, 2024

As discussed in #231 and #229, this explores the possibility of reintroducing bound getters for DisposableStack.prototype.dispose/AsyncDisposableStack.prototype.disposeAsync. By making these methods getters, a DisposableStack/AsyncDisposableStack subclass that wishes to override disposal needs only to override the [Symbol.dispose]() or [Symbol.asyncDispose]() methods. By making them into bound method getters, an instance of either class can be more readily used as a field on an object literal:

Subclassing:

// current
class MyDisposableStack extends DisposableStack {
  [Symbol.dispose]() {
    // custom dispose logic
    super[Symbol.dispose]();
    // more custom dispose logic
  }

  static {
    // copy the new @@dispose to dispose
    this.prototype.dispose = this.prototype[Symbol.dispose];
  }
}

// proposed
class MyDisposableStack extends DisposableStack {
  [Symbol.dispose]() {
    // custom dispose logic
    super[Symbol.dispose]();
    // more custom dispose logic
  }
}

Object Literals:

// current
function makeBuffers() {
  using stack = new DisposableStack();
  const myResource = stack.use(new MyResource());

  doSomeInit(myResource);

  const moved = stack.move();
  return {
    myResource,
    [Symbol.dispose]() {
      moved.dispose();
    },
  };
}

// proposed
function makeBuffers() {
  using stack = new DisposableStack();
  const myResource = stack.use(new MyResource());

  doSomeInit(myResource);

  return {
    myResource,
    [Symbol.dispose]: stack.move().dispose,
  };
}

Fixes #229
Fixes #231

@rbuckton rbuckton added needs-consensus A pull request that needs consensus at TC39 plenary normative Indicates a normative change to the specification labels Jun 15, 2024
Copy link

A preview of this PR can be found at https://tc39.es/proposal-explicit-resource-management/pr/232.

@rbuckton
Copy link
Collaborator Author

Ideally this can be pursued as a needs-consensus PR and not a follow-on to avoid making potentially breaking changes after implementations have shipped. I will bring this to the July/August 2024 TC39 plenary for discussion.

@rbuckton rbuckton self-assigned this Jun 15, 2024
@bakkot
Copy link

bakkot commented Jun 15, 2024

In the (extensive) discussions we had around extending built-ins in 2022 (focused on, but not exclusively about, Set methods), we concluded that we weren't going to make future built-in methods defer to other built-in methods on this. Subclasses will need to override all relevant parts of the public interface.

I don't want to revisit that discussion, but to summarize, making this a bound getter rather than an alias will make non-subclass consumers slower, for the benefit of very slightly simpler subclassing. I think that's the wrong tradeoff and I believe the committee has agreed.

@rictic
Copy link

rictic commented Jun 22, 2024

Hm, fair point. What was the motivation for having the dispose / disposeAsync methods? What if we dropped them?

Besides the foot gun in #231, it seems somewhat odd to me for the spec to define and give semantics for [Symbol.dispose] and [Symbol.asyncDispose], and then seeming not to prefer those methods in DisposableStack/AsyncDisposableStack, instead giving them string-named methods to call.

Should user space implementations of [Async]Disposable follow suit? Other symbol-named methods with semantics carved out by the spec generally don't do this.

@bakkot
Copy link

bakkot commented Jun 22, 2024

Should user space implementations of [Async]Disposable follow suit? Other symbol-named methods with semantics carved out by the spec generally don't do this.

They sometimes do: Array.prototype.values is an alias for Array.prototype[Symbol.iterator] (or rather, technically, the other way around); ditto for Map.prototype.values and Set.prototype.values.

String-named methods are nicer for consumers, but aren't a good basis for a protocol. So having both the symbol-named and string-named method is good practice, with one being an alias for the other.

As to the name, dispose is a good name for a method when the action is generic disposal, but sometimes it's going to be something like close or whatever, depending on the actual thing being implemented.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-consensus A pull request that needs consensus at TC39 plenary normative Indicates a normative change to the specification
Projects
None yet
3 participants