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

[Symbol.enter]() Follow-on proposal #195

Open
rbuckton opened this issue Jul 19, 2023 · 18 comments
Open

[Symbol.enter]() Follow-on proposal #195

rbuckton opened this issue Jul 19, 2023 · 18 comments

Comments

@rbuckton
Copy link
Collaborator

There has been some interest in changing the behavior of using and await using slightly to guide users towards using over const. In #49 there is a more comprehensive discussion about the possibility of implementing full Python-like Context Manager semantics, but an alternative to this would be the addition of a very lightweight [Symbol.enter]() method to the Disposable protocol.

If adopted, a [Symbol.enter]() method would be used to produce the actual resource you intend for a user to interact with, i.e.:

class File {
  #fd;
  constructor(filename) {
    this.#fd = openSync(filename);
  }
  [Symbol.enter]() {
    if (this.#fd) return new FileMutator(this.#fd);
    throw new TypeError();
  }
  [Symbol.dispose]() {
    if (this.#fd) {
      const fd = this.#fd;
      this.#fd = undefined;
      closeSync(fd);
    }
  }
}
class FileMutator {
  constructor(fd) { ... }
  read(...) { ... }
  write(...) { ... }
}

For a statement like

using x = new File("foo.txt");

the resulting File object would be tracked along with its [Symbol.dispose]() method in the environment, but the result of its [Symbol.enter]() method would be what is actually stored in x.

This would guide API consumers towards using since the only way to interact with the resource via const would be to explicitly call [Symbol.enter](), which provides a way to opt-out of using should you need it.

However, most existing APIs provided by hosts are not designed for such a mechanism, and I imagine there is little appetite amongst implementers to draft an entirely parallel set of APIs to support this construct. Existing APIs could be adapted by implementing a [Symbol.enter]() { return this; }, which would result in behavior that is comparable to the current proposal. In fact, this is the default behavior of Python Context Managers as well.

Given that the default behavior would be [Symbol.enter]() { return this; }, we are considering [Symbol.enter] itself to be a purely optional mechanism that an API producer could implement to opt-in to more pedantic semantics around using and await using, without burdening all future disposables with such an API, much like how an Iterator today only mandates the presence of a next() method, with return() and throw() being purely optional.

Given that we intend for [Symbol.enter]() to be optional, we are hoping to pursue its addition as a follow-on proposal that we could potentially add after Stage 4, giving us time to see how the ecosystem responds to using and await using and whether the addition of [Symbol.enter]() is strictly necessary.

Please note that we are not currently considering a [Symbol.asyncEnter]() method as part of this discussion. If the stated goal is to "guide users towards using and await using", then the presence of [Symbol.enter] and [Symbol.asyncDispose] are sufficient indicators of intent. A Promise-returning [Symbol.asyncEnter]() method would introduce far too much complexity and introduce additional implicit awaits into your code, which we do not think is merited given the narrow scope of this change.


Related discussions

@Jamesernator
Copy link

Jamesernator commented Jul 20, 2023

I'm not a huge fan of the name Symbol.enter, like in my suggestion in #159 I chose Symbol.create specifically because all it does it make a wrapper rather than suggesting any additional behaviour.

However, most existing APIs provided by hosts are not designed for such a mechanism, and I imagine there is little appetite amongst implementers to draft an entirely parallel set of APIs to support this construct.

The thing is essentially all host APIs also auto cleanup on GC of the object (even when flaky). For userland though it's actually fairly annoying to add this behaviour as it's both tends to result in a lot of extra boilerplate and it's way too easy to hold a reference to the wrapper object.

Honestly the feature would be a lot less useful if this were possible to write:

@autoDisposeOnGC
class SomeDisposable {
    [Symbol.dispose]() {
        this.#diposeSomehow();
    }
}

but at present it isn't possible as autoDisposeOnGC would neccessarily need to strongly hold the instance for the method to work.

I do wonder though if SemiWeakRef/Orphanage could be re-proposed at some point, if it was implemented it would be possible to write autoDisposeOnOrphaned as:

const disposalOrphanage = new Orphanage(semiWeakRef => {
    const disposable = semiWeakRef.deref();
    semiWeakRef.release();
    disposalOrphanage.unregister(disposable);
    disposable[Symbol.dispose]();
});

function autoDisposeOnOrphaned(klass, ctx) {
    return class extends klass {
        constructor(...args) {
            super(...args);
            const semiWeakRef = new SemiWeakRef(this);
            disposalOrphanage.register(this, semiWeakRef, this);
        }
    }
}

@autoDisposeOnOrphaned
class SomeDisposable {
    [Symbol.dispose]() {
       this.#disposeSomehow();
    }
}

@mhofman
Copy link
Member

mhofman commented Jul 20, 2023

Interesting, I hadn't thought of using SemiWeakRef for this purpose. The cost of tracing and bookkeeping might make it a little prohibitive for this use case however, which is actually a concern since the author might not understand that cost.

I do still intend to propose a mechanism along the lines of the SemiWeakRef / Orphanage idea someday, as it's the only way to enable cooperative distributed gc, which is something we need in our case.

That said I'm skeptical of using it as a simple pre-finalization "disposer", especially when a simple refactor of the disposable can effectively do the same. I actually think you might be able to achieve almost the same auto-dispose decorator convenience with a Proxy and normal FinalizationRegistry.

@rbuckton
Copy link
Collaborator Author

I'm not a huge fan of the name Symbol.enter, like in my suggestion in #159 I chose Symbol.create specifically because all it does it make a wrapper rather than suggesting any additional behaviour.

I'm not much of a fan of Symbol.create, myself. It's too generic, as many things create things, and the wrong metaphor, as the method would often be used to expose a part of the resource that has already been created.

Honestly the feature would be a lot less useful if this were possible to write:

@autoDisposeOnGC
class SomeDisposable {
    [Symbol.dispose]() {
        this.#diposeSomehow();
    }
}

A base class is probably more feasible, since you can pass down a resource handle and cleanup method to a superclass constructor, but it could still be achievable with decorators, i.e.

class SomeDisposable {
    @CleanupOnGC(handle => doCleanup(handle))
    #handle;

    @PreventCleanupOnGC()
    [Symbol.dispose]() {
        doCleanup(this.#handle);
    }
}

or even without decorators:

class SomeDisposable {
    #handle;
    constructor() {
        ...
        this.#handle = new SafeHandle(handle, handle => doCleanup(handle));
    }
    [Symbol.dispose]() {
        this.#handle.dispose();
    }
}

@rixtox
Copy link

rixtox commented Jul 20, 2023

I'm not a huge fan of the name Symbol.enter, like in my suggestion in #159 I chose Symbol.create specifically because all it does it make a wrapper rather than suggesting any additional behaviour.

I'm not much of a fan of Symbol.create, myself. It's too generic, as many things create things, and the wrong metaphor, as the method would often be used to expose a part of the resource that has already been created.

Maybe Symbol.getDisposable is more semantically accurate?

@Jamesernator
Copy link

Jamesernator commented Jul 21, 2023

I do still intend to propose a mechanism along the lines of the SemiWeakRef / Orphanage idea someday, as it's the only way to enable cooperative distributed gc, which is something we need in our case.

Yeah I've been wanting it for a while for basically the same purpose, so it's been on my mind.

Thinking about it more though autoDisposeOnOrphaned wouldn't really compose properly with such a thing, as if you have separately some object that has autoDisposeOnOrphaned behaviour and simultaenously have such an object as part of a remote handle it will wind up being disposed even if it's still remotely held strongly.

(Well it could work if there were some kind've ordered-orphanage where ordered-orphanages created earlier only observe that an object is orphaned if all ordered-semiweakrefs created after the ordered-orphanage are also released. I think engines adding this unlikely.)

I actually think you might be able to achieve almost the same auto-dispose decorator convenience with a Proxy and normal FinalizationRegistry.

To an extent, but I don't think there's any way to do so without changing some other observable behaviour. e.g. In the example:

@autoDiscardOnGC
class SomeDisposable {
    method(someValue) {
        // (1)
        this.#whatever();
        // (2)
        someValue.method(this);
        // (3)
        if (someValue === someSpecificValue) {
            
        }
    }
}

const d = new SomeDisposable();

d.someMethod(someSpecificValue);

regardless of the implementation of autoDiscardOnGC one of the following must be true within the method:

  • this is the GC proxy in which case this.#whatever fails (basically makes the whole thing useless)
  • this is not the GC proxy in which case someValue.method(this) observes the unproxied value
  • someValue !== someSpecificValue if arguments were membrane wrapped to fix (2)

in all cases observable differences might happen (beyond the desired auto-dispose-on-GC) which makes the thing honestly more risky to use than just dealing with finalization themselves.

@webstrand
Copy link

I'd be in favour of a [Symbol.acquire] being called. Otherwise if you're using this for reference counting, database transactions, locks-over-https, etc you will still have to rely on the finalization registry to release the resources that you've acquired. Because if a user calls const resource = acquireLock() you have to be able to clean up that spurious lock.

@ggoodman
Copy link

ggoodman commented Nov 21, 2023

In #203, there's the question of detecting whether a disposable resource was correctly instantiated via using.

In this discussion, we're talking about the relationship between the disposable resource and the object received by the caller of using. But in this discussion, there seems to be this idea that the disposable object can produce a resource for the caller.

Has any thought been given to the idea of a resource instead producing a disposable?

For example, let's imagine a couple new protocols Symbol.disposer and Symbol.asyncDisposer:

class File {
  static #FileDisposer = class {
    #file;
    constructor(file) {
      #file = file;
    }
    [Symbol.dispose]() {
      if (this.#file.#fd) {
        const fd = this.#file.#fd;
        this.#file.fd = undefined;
        closeSync(fd);
      }
    }
  }

  #fd;
  constructor(filename) {
    this.#fd = openSync(filename);
  }
  [Symbol.disposer]() {
    if (this.#fd) return new File.#FileDisposer(this);
    throw new TypeError();
  }
  read(...) { ... }
  write(...) { ... }
}

In this way a resource can detect that it has been bound using the using or await using. That being said, it seems the caveat remains that it can't detect (synchronously) that it hasn't been instantiated via using or await using. I guess it could be done in a hacky way by deferring the check to the next micro tick. But that means that the feedback on incorrect use is unfortunately offset in time from the source.

I think an interesting side-benefit of a model like this is that the author of a resource can opt into building their own reference counting since they could observe the number of different referenced handles through calls to Symbol.disposer. That might also help address some of the concerns in #160.

@yw662
Copy link

yw662 commented Jan 25, 2024

I would prefer using.target as in new.target here. using.target would be true inside the functions called with using.

@ljharb
Copy link
Member

ljharb commented Jan 25, 2024

@yw662 since marking an item as disposable can be done with DisposableStack and not just using syntax, it would make no sense, nor be possible, to have syntax to indicate when a dispose method were "properly" called.

@rbuckton
Copy link
Collaborator Author

I've been considering this for some time, and I'm still not convinced a [Symbol.enter] is necessary at this time and is better left as a follow on. Even if [Symbol.enter] existed, it could not be backported to all of the existing runtime APIs that can be used with using and await using across the DOM, NodeJS, Electron, and other platforms today, so the limited benefit it would gain would only be for new code and not the wealth of existing code that already exists.

In addition, there exists two remediations for the lack of Symbol.enter:

  1. You could choose to utilize a FinalizationRegistry to either perform cleanup on your behalf, or log warnings to the developer console indicating incorrect usage.
  2. You could utilize a getter for [Symbol.dispose] or [Symbol.asyncDispose] that tracks its usage and either log a warning or throw an error if the object is used without having been registered.

I do believe that [Symbol.enter]() is worth further investigation as a follow on, but for now I am convinced that it represents a niche use case compared to the potential broad adoption across the existing APIs in the web platform.

@webstrand
Copy link

Does any browser actually implement using yet? I could not find any evidence that one does. If we are only considering transpiled "platforms" I would still request this change. However, if browsers have been released with support for this feature, then I agree, adding this feature at this point is useless. No code could be safely written to expect [Symbol.enter] to be called, you would always have to write your code for backwards compatibility.

I disagree that it's niche use, though. The lack of [Symbol.enter] in the ecosystem means that for every use case that I can imagine, I will always have to involve FinalizationRegistry to be sure that my resources are properly freed. The only advantage to using is as an opt-in to a mildly more accurate freeing of un-referenced resources. Disappointing for a whole new syntax.

@rbuckton To be clear, by your second remediation, you mean to use get [Symbol.dispose]() { this[Symbol.enter](); return this.realDisposeMethod }? And I suppose I could probably write a typescript lint that complains if an object with [Symbol.dispose] is inappropriately assigned.

@rbuckton
Copy link
Collaborator Author

Does any browser actually implement using yet? I could not find any evidence that one does. If we are only considering transpiled "platforms" I would still request this change. However, if browsers have been released with support for this feature, then I agree, adding this feature at this point is useless. No code could be safely written to expect [Symbol.enter] to be called, you would always have to write your code for backwards compatibility.

V8 is currently working on an implementation, and Symbol.dispose is shipping as a polyfill in NodeJS, but that's besides the point. Browsers have existing APIs that can be upgraded to support using via Symbol.dispose, but cannot be upgraded to support Symbol.enter in a way that would provide the guarantees you are seeking without breaking all existing callers. Browsers are also unlikely to reimplement these APIs just to support Symbol.enter, since those APIs would be redundant and would not benefit existing callers.

I disagree that it's niche use, though. The lack of [Symbol.enter] in the ecosystem means that for every use case that I can imagine, I will always have to involve FinalizationRegistry to be sure that my resources are properly freed.

The majority of APIs that are concerned with native handles are usually exposed by the host and often already have FinalizationRegistry-like semantics (i.e., NodeJS's FileHandle already does this). The majority of user-defined disposables will either be consuming/wrapping these existing host APIs, or be built around non-native handles, and thus generally won't need this capability. If those objects are dropped on the floor for GC to clean up, those wrapped, host-provided natives will be GC'd as well. For the subset of cases that do, FinalizationRegistry is a readily available option for post-hoc cleanup or notification.

The only advantage to using is as an opt-in to a mildly more accurate freeing of un-referenced resources.

I strongly disagree with this sentiment. As-is, using simplifies the complexity of accurate and timely freeing of resources (and other cleanup actions) in a well-defined and well-ordered mechanism that is far more difficult and cumbersome to do correctly without.

To be clear, I'm not saying that Symbol.enter isn't a valuable direction to consider, I am saying that it isn't necessary for the MVP for this proposal given that there would be no uptake amongst existing host APIs. It is certainly something that can be added on in a follow-on proposal whose need can be determined based on observation of using and Symbol.dispose in the wild.

To be clear, by your second remediation, you mean to use get [Symbol.dispose]() { this[Symbol.enter](); return this.realDisposeMethod }?

I mean something like the following:

class SomeResource {
  #nativeHandle;
  #disposeState = "unregistered";
  
  ...
  
  // user code that would use the handle if the resource is in a valid state
  doSomethingWithResource() {
    this.#throwIfInvalid();
    ...
  }
  
  // treat reading the Symbol.dispose method as registration
  get [Symbol.dispose]() {
    if (this.#disposeState === "unregistered") {
      this.#disposeState = "registered";
    }
    return SomeResource.#dispose;
  }
  
  // actual `Symbol.dispose` method implementation
  static #dispose = function() {
    if (this.#disposeState === "disposed") {
      return;
    }
    const nativeHandle = this.#nativeHandle;
    this.#disposeState = "disposed";
    this.#nativeHandle = undefined;
    cleanupNativeHandle(nativeHandle);
  };
  
  // verify resource is in the correct state
  #throwIfInvalid() {
    switch (this.#disposeState) {
    case "unregistered":
      throw new TypeError("This resource must either be registered via 'using' or attached to a 'DisposableStack'");
    case "disposed":
      throw new ReferenceError("Object is disposed");
    }
  }
}

In general it's a best practice to verify the state of a wrapped resource before acting on it, and while that would usually mean just checking whether the resource is disposed, it can be expanded as I have here to check whether the resource is registered.

And I suppose I could probably write a typescript lint that complains if an object with [Symbol.dispose] is inappropriately assigned.

This is feasible to do in a linter, to an extent. Receiving a Disposable as a return value (except from, say, a DisposableStack.prototype.use() method) or constructing a new Disposable and not immediately assigning it to a using or passing it as an argument (again, to something like DisposableStack.prototype.use()) could be considered a lint violation. We've considered doing this in TypeScript, but it would likely require additional type hints at various sites (parameters, fields, return values) to proliferate this information end-to-end. That's a fairly complex mechanism to implement, so it's important to get this capability in the hands of users to best determine whether the impact it would have would offset the additional development cost and checker complexity.

@rixtox
Copy link

rixtox commented Mar 27, 2024

Does any browser actually implement using yet? I could not find any evidence that one does. If we are only considering transpiled "platforms" I would still request this change. However, if browsers have been released with support for this feature, then I agree, adding this feature at this point is useless. No code could be safely written to expect [Symbol.enter] to be called, you would always have to write your code for backwards compatibility.

V8 is currently working on an implementation, and Symbol.dispose is shipping as a polyfill in NodeJS, but that's besides the point. Browsers have existing APIs that can be upgraded to support using via Symbol.dispose, but cannot be upgraded to support Symbol.enter in a way that would provide the guarantees you are seeking without breaking all existing callers. Browsers are also unlikely to reimplement these APIs just to support Symbol.enter, since those APIs would be redundant and would not benefit existing callers.

There's always an option to have a new set of APIs instead of upgrading existing ones. Even if you upgrade existing APIs, none of the existing code using them gets any kind of benefits you introduced. It's only new code or refactored code can benefit from your upgrade. If that's the case, new code may be better off using new set of APIs that are both easier to dispose and safer to use, meaning enforcing [Symbol.enter].

I disagree that it's niche use, though. The lack of [Symbol.enter] in the ecosystem means that for every use case that I can imagine, I will always have to involve FinalizationRegistry to be sure that my resources are properly freed.

The majority of APIs that are concerned with native handles are usually exposed by the host and often already have FinalizationRegistry-like semantics (i.e., NodeJS's FileHandle already does this). The majority of user-defined disposables will either be consuming/wrapping these existing host APIs, or be built around non-native handles, and thus generally won't need this capability. If those objects are dropped on the floor for GC to clean up, those wrapped, host-provided natives will be GC'd as well. For the subset of cases that do, FinalizationRegistry is a readily available option for post-hoc cleanup or notification.

I strongly disagree. You overlooked the importance of making a safe disposal mechanism simple and accessible. There are so many common use cases that doesn't involve native resources and still need to clean up or there can be resource leak. For example event listeners, HTTP requests, long running Promises just to name a few. FinalizationRegistry is both heavy and very difficult to write.

The only advantage to using is as an opt-in to a mildly more accurate freeing of un-referenced resources.

I strongly disagree with this sentiment. As-is, using simplifies the complexity of accurate and timely freeing of resources (and other cleanup actions) in a well-defined and well-ordered mechanism that is far more difficult and cumbersome to do correctly without.

using is merely a syntactic sugar to this:

const stack = new DisposableStack();
try {
    // lines before using x
    const x = stack.use(resource());
    // lines after using x
    const y = stack.use(resource());
    // lines after using y
} finally {
    stack.dispose();
}

Tell me what are the benefits using syntax can provide, other than a little bit of syntax convenience? I honestly would prefer directly using DisposableStack rather than adopting the using syntax. Because the using syntax introduces implicit control flow change to the semantic meaning of lexical blocks {}. I wrote a blog post about it.

To be clear, I'm not saying that Symbol.enter isn't a valuable direction to consider, I am saying that it isn't necessary for the MVP for this proposal given that there would be no uptake amongst existing host APIs. It is certainly something that can be added on in a follow-on proposal whose need can be determined based on observation of using and Symbol.dispose in the wild.

I think we only have one chance to get it right. It's now or never.

And I suppose I could probably write a typescript lint that complains if an object with [Symbol.dispose] is inappropriately assigned.

This is feasible to do in a linter, to an extent. Receiving a Disposable as a return value (except from, say, a DisposableStack.prototype.use() method) or constructing a new Disposable and not immediately assigning it to a using or passing it as an argument (again, to something like DisposableStack.prototype.use()) could be considered a lint violation. We've considered doing this in TypeScript, but it would likely require additional type hints at various sites (parameters, fields, return values) to proliferate this information end-to-end. That's a fairly complex mechanism to implement, so it's important to get this capability in the hands of users to best determine whether the impact it would have would offset the additional development cost and checker complexity.

If there's a working linter that can catch most of the using misuses, I would be okay with pushing forward the proposal without Symbol.enter.

@rbuckton
Copy link
Collaborator Author

There's always an option to have a new set of APIs instead of upgrading existing ones. Even if you upgrade existing APIs, none of the existing code using them gets any kind of benefits you introduced. It's only new code or refactored code can benefit from your upgrade. If that's the case, new code may be better off using new set of APIs that are both easier to dispose and safer to use, meaning enforcing [Symbol.enter].

It is a simple matter to refactor existing code to using when an existing API adopts it. It's quite another matter to have every host bifurcate their APIs merely to produce the same result. This is a tradeoff between what we chose to enforce in the language, and what implementers might be willing to accept. It's far easier to slot new capabilities on top of an existing API, much like Symbol.iterator did for NodeList.

Because the using syntax introduces implicit control flow change to the semantic meaning of lexical blocks {}.

That is quite literally the point. DisposableStack is a convenience mechanism primarily built for two specific cases:

  • As an adapter for non-disposable resources (via .adopt() and .defer())
  • As a container mechanism for composing disposables in a class constructor.

On its own, it's not much safer than existing mechanisms. It's too easy to incorrectly call stack.use() inside of a different block than the one that declared it and have a resource's lifetime extend past its reachability:

const stack1 = new DisposableStack();
try {
  const res1 = stack1.use(getResource1());
  const stack2 = new DisposableStack();
  try {
    const res2 = stack1.use(getResource2()); // oops, wrong stack
  }
  finally {
    stack2.dispose();
  }
  // continue to use res1
}
finally {
  stack1.dispose();
}

However, when composing disposables, DisposableStack+using is immensely useful for ensuring proper cleanup:

class C {
  #disposables;
  #res1;
  #res2;
  constructor(options) {
    using stack = new DisposableStack();
    this.#res1 = stack.use(getResource1(options));
    this.#res2 = stack.use(getResource2(options)); // if this throws, need to clean up
    this.#disposables = stack.move(); // ok, everything succeeded, track things we need to clean up later
  }
  [Symbol.dispose]() {
    this.#disposables.dispose();
  }
}

This is significantly harder to get right without DisposableStack, but for almost every other use case using is far more reliable.

I think we only have one chance to get it right. It's now or never.

I disagree. Implementations can slot in Symbol.dispose into existing APIs, prompting users to adopt this more readily as it requires far fewer changes to existing code without needing to relearn a different, parallel API. Symbol.enter can be added to using and DisposableStack without breaking that flow, and new APIs can leverage that as it becomes available. The Symbol.enter mechanics I've considered for a follow on are expressly designed to slot in as an opt-in capability of the API producer.

If there's a working linter that can catch most of the using misuses, I would be okay with pushing forward the proposal without Symbol.enter.

I'll look into adding a rule for typescript-eslint.

@rixtox
Copy link

rixtox commented Mar 27, 2024

It is a simple matter to refactor existing code to using when an existing API adopts it. It's quite another matter to have every host bifurcate their APIs merely to produce the same result. This is a tradeoff between what we chose to enforce in the language, and what implementers might be willing to accept. It's far easier to slot new capabilities on top of an existing API, much like Symbol.iterator did for NodeList.

It's 2024 already. There are many successful resource management prior arts out there for us to learn from. I don't understand why we have to stick with the C# flavor out of all alternatives. Developers nowadays deserves safer guarantees when we design new language features. Even Joe Biden understand the importance of accessible memory/resource safety guarantees. I'm just not convinced that having a seamless upgrade for existing APIs can be more important than making the language feature to be safer.

On its own, it's not much safer than existing mechanisms. It's too easy to incorrectly call stack.use() inside of a different block than the one that declared it and have a resource's lifetime extend past its reachability:

const stack1 = new DisposableStack();
try {
  const res1 = stack1.use(getResource1());
  const stack2 = new DisposableStack();
  try {
    const res2 = stack1.use(getResource2()); // oops, wrong stack
  }
  finally {
    stack2.dispose();
  }
  // continue to use res1
}
finally {
  stack1.dispose();
}

I won't call that a mistake at all. Whether res2 should be bound to stack1 or stack2 depends very much on the semantics and logics requirements. What you wrote above can be totally reasonable and valid for some real use cases. Changing it to using doesn't make it "safer" in anyway, it just limits you to bind resource to the enclosing lexical block, and you don't get to choose the actual intended lifetime of your resource, unless you use DisposableStack and move(), which again, shows the flexibility and power of simply using DisposableStack.

Writing stack.dispose() explicitly also makes control flow analysis easier. You don't get unexpected hidden control flow from merely surrounding some code with lexical blocks {}. Just think of the mental burden you added upon developers to think about the control flow implications of surrounding some code with merely lexical block {}. It's crazy to realize that the following code can behave differently with only difference being an extra lexical block.

const getResource1 = () => { [Symbol.dispose]() {} };
const getResource2 = () => { [Symbol.dispose]() { throw new Error(); } };

{
    using res1 = getResource1();
    using res2 = getResource2();
    console.log("done"); // this will print
}

{
    using res1 = getResource1();
    {
        using res2 = getResource2();
    }
    console.log("done"); // this won't print
}

Just imagine how much mental burden you are imposing on code reviewers by adopting the using syntax. I definitely don't want to review code utilizing this behavior everyday.

However, when composing disposables, DisposableStack+using is immensely useful for ensuring proper cleanup:

class C {
  #disposables;
  #res1;
  #res2;
  constructor(options) {
    using stack = new DisposableStack();
    this.#res1 = stack.use(getResource1(options));
    this.#res2 = stack.use(getResource2(options)); // if this throws, need to clean up
    this.#disposables = stack.move(); // ok, everything succeeded, track things we need to clean up later
  }
  [Symbol.dispose]() {
    this.#disposables.dispose();
  }
}

This is significantly harder to get right without DisposableStack, but for almost every other use case using is far more reliable.

There are so many use cases other than just class constructors where we want to extend the lifetime of resource beyond the enclosing lexical block.

I think we only have one chance to get it right. It's now or never.

I disagree. Implementations can slot in Symbol.dispose into existing APIs, prompting users to adopt this more readily as it requires far fewer changes to existing code without needing to relearn a different, parallel API. Symbol.enter can be added to using and DisposableStack without breaking that flow, and new APIs can leverage that as it becomes available. The Symbol.enter mechanics I've considered for a follow on are expressly designed to slot in as an opt-in capability of the API producer.

A seamless upgrade for this feature is really not necessary, and probably can cause more confusion for developers, because there would be two difference ways of calling the same API. FWIW, we had experienced "bifurcate" changes when we migrated from callback based APIs to Promise based APIs. Although it's not the smoothest transition in the world, but nowadays people can pick it up quite easily without any problem. And fortunate for us, unlike the migration from callbacks to Promises, there are not that many APIs that manages resource. It won't be as overwhelming as the Promise transition.

What if we just keep it a TypeScript feature instead of pushing it to JavaScript? Or just move forward without the using syntax, just add DisposableStack only.

@rixtox
Copy link

rixtox commented Mar 28, 2024

To clarify what I meant by:

A seamless upgrade for this feature is really not necessary, and probably can cause more confusion for developers

Imagine an API got upgraded, how can a linter help users to adopt it?

// From some library that upgraded their API:
interface Resource extends Disposable {
	close(): void;
}
function createResource(): Resource;

// In some user's code:
{
	const res = createResource();
	//    ^-- eslint: Consider change it to `using res = createResource();`
	try {
		// ...
	} finally {
		res.close();
	}
}

Now, how likely the user would just make the easiest change and do:

{
	using res = createResource();
	try {
		// ...
	} finally {
		res.close();
	}
}

Now the resource would be closed twice. Which may or may not cause problems but it's generally not a good idea, and often will cause issues.

@rbuckton
Copy link
Collaborator Author

What if we just keep it a TypeScript feature instead of pushing it to JavaScript? Or just move forward without the using syntax, just add DisposableStack only.

I did not propose this as a TypeScript feature that we want in JavaScript. I proposed this as a JavaScript feature that supports other JavaScript proposals, such as the Shared Structs proposal which needs a convenient mechanism for handling Mutex lock lifetime.

Imagine an API got upgraded, how can a linter help users to adopt it?
[...]
Now the resource would be closed twice. Which may or may not cause problems but it's generally not a good idea, and often will cause issues.

A more robust description of the lint failure can go a long way to informing users how to correctly address the issue. If the API is a recognized host API, the linter can be even more specific and recognize such cases.

A well defined Disposable should ideally handle double-dispose correctly, with DisposableStack and AsyncDisposableStack as built-in examples of this behavior.

It's 2024 already. There are many successful resource management prior arts out there for us to learn from. I don't understand why we have to stick with the C# flavor out of all alternatives.

This is already a blend of multiple similar capabilities across several languages, though it is heavily influenced by both C# and Python as prior art.

@rbuckton
Copy link
Collaborator Author

I have added https://github.com/rbuckton/proposal-using-enforcement to the agenda for the upcoming TC39 plenary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants