Split overloaded 'use' method into multiple methods.

[rbuckton]

Per the September 2022 plenary discussion, the DisposableStack.prototype.use method should not be overloaded and instead split into multiple methods:

• use(disposable) to add a disposable resource to be disposed when the stack is disposed.
• adopt(value, onDispose) to add a non-disposable resource (value), such that onDispose(value) is called when the stack is disposed.
• defer(onDispose) to add a callback to execute when the stack is disposed.

[ljharb]

so "adopt" is like "use and defer"?

[bakkot]

adopt doesn't call the Symbol.dispose method on its first argument, whereas use would, so it is not quite like "use and defer".

[ljharb]

hmm - so is adopt(x, y) just defer(y => x(y))?

[bakkot]

Not quite: adopt returns its first argument.

I had the same question on Wednesday, and if I understood @rbuckton correctly, it's intended for cases like this

let resource = stack.adopt(getResource(), handleResourceDisposal);

Rewriting that to use defer would require

let resource = getResource();
stack.defer(() => handleResourceDisposal(resource));

which has the problem that there is now a place you can insert code between where the resource is acquired and the place where it is registered for disposal (i.e. between the first and the second lines). Using adopt avoids that risk.

[ljharb]

defer(y => { x(y); return y; }) then? Maybe I'm still not understanding.

[bakkot]

The argument to defer isn't invoked until scope exit, so no, not like that.

[ljharb]

ahhh i see, because it allows "defer" but with eager evaluation of the expression. thanks.

[bakkot]

Yeah.

Also, to be clear, the function passed to defer isn't passed any arguments: in your defer(y => x(y)) , snippet, y is always going to be undefined.

[rbuckton]

• use(value) registers a disposable and returns value.
• adopt(value, onDispose) registers something like { [Symbol.dispose]() { onDispose(value); } }, and returns value.
• defer(onDispose) registers something like { [Symbol.dispose]() { onDispose(); } }, and returns undefined.

[ljharb]

Ok, so why isn’t defer and adopt the same method, with the value as a second optional argument?

[rbuckton]

Ok, so why isn’t defer and adopt the same method, with the value as a second optional argument?

For several reasons:

• defer(onDispose, value?) feels too reminiscent of filter(callback, thisArg?), and I'd rather not confuse users.
• Having the callback last is reminiscent of NodeJS's continuation-passing style.
• adopt(value, v => v.close()) works better for type inference purposes since the type of v can be determined from the type of value; the other way around requires you to write defer(v => |, value) first and then jump back to | to get completions.

[ljharb]

I don't agree with the first point at all - there's tons of APIs, array or not, that aren't filter that don't take an optional thisArg (reduce, split, join, etc), nor do I see a problem with the second - this isn't a continuation, it's basically an event handler. I'm also not persuaded by type inference arguments.

It seems silly to me to have two separate methods (even if they had names that actually seemed related, and that actually conveyed what they do, which "defer" kind of does but "adopt" doesn't at all) when the only difference is the value argument's presence.

[rbuckton]

It seems silly to me to have two separate methods (even if they had names that actually seemed related, and that actually conveyed what they do, which "defer" kind of does but "adopt" doesn't at all) when the only difference is the value argument's presence.

It seemed silly to me to have three, which is why they were overloaded. Regardless, I'm not in favor of (callback, value?) because of the mess it causes for inference when using an editor with type information (be that TS or JSDoc types). reduce is a perfect example of the issue with inference:

["a", "b", "c"].reduce((memo, value) => ¦

As you type and reach the ¦ a type system would infer memo to be a string. But if you're planning to aggregate it into a Map it won't infer that type until you write the rest of the call:

["a", "b", "c"].reduce((memo, value) => ¦, new Map())

split and join are poor examples as they don't accept callbacks. There are many built-ins that do take callbacks, and the majority take the callback last:

• new Proxy(obj, handlers) (the handlers are essentially a collection of callbacks)
• JSON.parse(text, reviver)
• JSON.stringify(value, replacer)
• Array.from(items, mapfn) (if you ignore the thisArg)

Aside from reduce/reduceRight, the only other functions in the standard library that take a value following the callback take a thisArg (map, filter, find, etc.).

[ljharb]

If you're making a consistency argument, then the thisArg pattern is exactly what you'd want to follow. I don't think a type inference argument should be valid since ecmascript doesn't have types in that sense - if that mattered, the order of import/from would have been reversed, and a precedent might have existed to follow. I understand the issue with inference, I'm saying that may be an unavoidable cost.

[bakkot]

I don't think a type inference argument should be valid since ecmascript doesn't have types in that sense

I think it's important to consider the effects on tooling, given that many people are using tools. We absolutely should not assume everyone is using a tool, but we also shouldn't give considerations about tooling literally zero weight.

[ljharb]

that's fair that we should give it nonzero weight. However, I think defer/adopt are confusing names, and it took a lot of questioning before i realized that they're actually the same method with an optional value argument squished into the front. I think it would be very, very confusing unless we combine them into one (callback, value?) signature.

[bakkot]

Honestly they don't really seem like the same method to me? One is "I wish to use this value, and here's the function used for disposing of the value's resources [because it doesn't implement @@dispose itself] once it goes out of scope"; the other is "here's a function to call at the end of this block, with no particular resource associated". They're not really that similar in intent, even if they do have similar underlying mechanics.

As to the naming question, defer is pretty clear, I think: "defer this function to be called at scope exit". I agree adopt is a bit weird, but I can't immediately see a better name for the operation I describe in the preceding paragraph.

(That said, I don't feel that strongly that those two operations need to be separate methods; I see the type inference concern but don't know if it warrants having an extra method. I am fine either way.)

[rbuckton]

@ljharb:

[...] I don't think a type inference argument should be valid since ecmascript doesn't have types in that sense - if that mattered, the order of import/from would have been reversed, [...]

That has consistently been a pain, and a proposal to do just that was created and put on the agenda at one point, though it was withdrawn and wasn't presented.

However, I think defer/adopt are confusing names [...]

I'm more than happy to consider alternatives. I chose defer since it is reminiscent of Go's defer, though Python's ExitStack just uses callback. I chose adopt since its purpose is interoperability, to "use something that isn't disposable as if it was disposable". I considered "adapt", but that doesn't imply a containment/ownership relationship. I was leaning towards a short single-word function name, but we could also consider something like useNonDisposable, etc.

I think it would be very, very confusing unless we combine them into one (callback, value?) signature.

But what would you call that? defer doesn't seem like the right term for something that takes a value, and if we were to use that signature I'd find it confusing unless it was (callback, thisArg?), necessitating the use of a function expression rather than an arrow function so that you can access this.

I personally like the idea that the subject of the operation is the first argument and the return value, hence why the signature for use is <T>(value: T) => T, and the signature for adopt is <T>(value: T, onDispose: (value: T) => void) => T.

@bakkot:

(That said, I don't feel that strongly that those two operations need to be separate methods; I see the type inference concern but don't know if it warrants having an extra method. I am fine either way.)

I still prefer the overloaded use I presented:

class DisposableStack {
  use<T extends Disposable | null | undefined>(value: T): T;
  use<T>(value: T, onDispose: (value: T) => void): T;
  use(onDispose: () => void): void;
}

But if we were to break this into two methods instead of three, I'd prefer this breakdown:

class DisposableStack {
  use<T extends Disposable | null | undefined>(value: T): T;
  use<T>(value: T, onDispose: (value: T) => void): T;
  
  defer(onDispose: () => void): void;
}

Where the use and adopt methods are merged, with the 2nd argument existing as a way to override any potential [Symbol.dispose]() method on value.

[bakkot]

I still prefer the overloaded use I presented:

There's two bad things about that overload (to my eyes):

1. f(x) branches on x being disposable vs callable
2. f(x, y) treats x as a different kind of thing than f(x)

These are both bad, but the first one is more serious.

The three-method form eliminates both of these problems.

Having use plus an overloaded defer which is either defer(() => void) or defer<T>(T => void, T), as Jordan suggests, also eliminates both of these problems (but has the type inference problem you point out).

Having use plus an overloaded defer which is either defer(() => void) or defer<T>(T, T => void) only eliminates the first problem.

Having defer plus an overloaded use which is either use<T extends Disposable>(T) or use<T>(T, T => void) only eliminates the first problem.

I would be happiest if we eliminate both of the problems with the originally proposed overload. I can live with only eliminating the first problem. I would be unhappy if we don't eliminate either problem.

[rbuckton]

@bakkot:

[...]

The three-method form eliminates both of these problems.

Having use plus an overloaded defer which is either defer(() => void) or defer<T>(T => void, T), as Jordan suggests, also eliminates both of these problems (but has the type inference problem you point out).

[...]

I would be happiest if we eliminate both of the problems with the originally proposed overload. I can live with only eliminating the first problem. I would be unhappy if we don't eliminate either problem.

If we are to break this down, then my preference would still be for the three-method form to better support type inference.

---

@ljharb:
Was my explanation of adopt and defer sufficient? I'd like to think that the names I chose are representative of their operations:

• use(disposable) - Use a disposable object, returning it. Allows null | undefined, just like using.

  • The term use is intended to align with using, but also distinctive enough so as not to be confused with the declaration itself.

• adopt(nonDisposable, onDisposeCallback) - Adopt a non-disposable object as if it were disposable, using the provided callback for cleanup, returning the non-disposable object. Allows null | undefined, but always registers the object (unlike using).

  • The term adopt is intended to align with similar terminology in other areas, such as:
    • Promises: adopting the state of a foreign promise, https://github.com/tc39/proposal-faster-promise-adoption, etc.
    • DOM Documents: adopting a node from another document, i.e. Document.adoptNode.
  • In other words, though nonDisposable isn't a disposable we will treat it like one (given the provided callback).

• defer(onDisposeCallback) - Defer an operation until the stack is disposed.

  • The term defer was chosen to align with Go's defer, which has analogous behavior.

If you have suggestions or alternatives, I'd be glad to hear them.

[shicks]

I haven't thought all the way through this yet, but I'm very interested in how incremental migrations would work here. Specifically, what does it look like when a pre-disposable API updates to become an actual disposable? Relatedly, what might it look like if anybody is polyfilling or ponyfilling disposable or DisposableStack?

For the former question, I imagine you'd start with code that looks like stack.adopt(resource, r => r.close()). If resource grew a Symbol.dispose method then you could continue using it with adopt, but could then switch to use(resource) or using resource at any time. I could imagine a possibility where adopt was actually useOrDefer or some such, so that it would switch to using the symbol method if it existed, and otherwise it would use the given callback - this would give it some distinction from both use and defer. But I can also imagine plenty of arguments against that design.

Aside about polyfills/ponyfills (tl;dr: I think it's not an issue)

I have a harder time reasoning about polyfills/ponyfills, since I'm not quite sure what they might look like. Presumably a polyfill would actually define Symbol.dispose, which downstream disposables would then depend on. A transpiler would probably change using into a for-of loop, and there would be an actual DisposableStack implementation that could be used as well. What happens when the latter is replaced with the real thing? I assume the symbol would drop in correctly and there's no problems?

[rbuckton]

[...] I could imagine a possibility where adopt was actually useOrDefer or some such, so that it would switch to using the symbol method if it existed, and otherwise it would use the given callback - this would give it some distinction from both use and defer. But I can also imagine plenty of arguments against that design.

Aside from just adopting non-disposables, adopt is also able to use custom dispose semantics for actual disposables:

using stack = new DisposableStack();
const res = stack.adopt(getResource(), res => {
  disposeCounter++; // some extra work we want to do.
  res[Symbol.dispose]();
});

As such, I wouldn't want adopt to conditionally use the callback based on the presence of Symbol.dispose, but to always use the callback. Even with the overloaded use version proposed currently, the presence of a callback would always outweigh whether the object itself was disposable.

[bakkot]

I could imagine a possibility where adopt was actually useOrDefer or some such, so that it would switch to using the symbol method if it existed, and otherwise it would use the given callback

I'm strongly opposed to doing this kind of switching. It's harder to read, and it makes it a lot more likely that adding the Symbol.dispose will be a breaking change (in particular if it's not just an alias for the thing the user was doing previously).

[rbuckton]

[...] A transpiler would probably change using into a for-of loop [...]

Actually, my plan for TypeScript is to implement with nested try..finally, per the example transposed representations in the explainer. Transpiling to a for..of would require a much more complex transform since you would need to also transform any containing iteration statements and nested continue or break statements to work around the introduction of an additional loop (something also mentioned in the explainer).

[ljharb]

Regarding naming, use(disposable) makes sense to me, altho i'd mildly prefer using(disposable) precisely because it mirrors the keyword.

defer(callback) also makes sense to me, not because i think any notable percentage JS programmers will have ever seen go or be familiar with it, but because it accurately describes what it's doing.

adopt(value, nonDisposable) is the one I'm not comfortable with. One because value is optional, and it should be adopt(nonDisposable[, value ]), but also the name - it's just a really weird pattern imo, and something like .defer((x => { x.close(); return x; }).bind(null, value)) or similar doesn't seem sufficiently unergonomic to me to warrant adding this method now (iow, it seems better for a followup proposal).

[bakkot]

something like .defer((x => { x.close(); return x; }).bind(null, value)) or similar doesn't seem sufficiently unergonomic to me to warrant adding this method now

The implementation of adopt is something like

class DisposableStack {
  // ...
  adopt(resource, callback) {
    this.defer(() => callback(resource));
    return resource;
  }
}

So your snippet doesn't do the same thing as adopt - in particular, it doesn't evaluate to value. Did you mean (x => { stack.defer(() => x.close()); return x; })(value)? Personally that absolutely seems sufficiently unergonomic to warrant adding this method now. No one is going to write that; they're just going to mention the resource twice and risk not cleaning it up.

[ljharb]

I'm not sure why it needs to be chained - stack.defer(() => callback(resource)); resource then?

[bakkot]

Because then you're mentioning resource twice, which is bad. Specifically, if you're doing something like getResource(), then writing

let resource = stack.adopt(getResource(), x => x.close());

makes acquiring the resource and registering it for cleanup a single operation, which is important. Without adopt, you have to write

let resource = getResource();
stack.defer(() => callback(resource));

which (as mentioned above) has the problem that there is now a place you can insert code between where the resource is acquired and the place where it is registered for disposal (i.e. between the first and the second lines).

[rbuckton]

adopt(value, nonDisposable) is the one I'm not comfortable with. One because value is optional, and it should be adopt(nonDisposable[, value ]), but also the name [...]

I'm open to suggestions for an alternative name, but I strongly disagree with having the value as the 2nd argument. It's not that it's optional, per se, but that it passes through the argument (including null and undefined). As I've said before, the pattern to emulate here is JSON.parse(text, reviver), JSON.stringify(obj, replacer), Array.from(obj, mapper), fs.readFile(filename, callback), not array.map(callback, thisArg). If it came down to it, I would rather ban null and undefined as the first argument to adopt than to change the argument order.

[ljharb]

That you can insert code doesn't mean you will; this seems like a very unnecessary guardrail/convenience to me compared to the cost of an entirely distinct API method.

[rbuckton]

That you can insert code doesn't mean you will; this seems like a very unnecessary guardrail/convenience to me compared to the cost of an entirely distinct API method.

I'm not sure I understand what you mean by "can insert code"? The point of adopt is that you want to both use a resource and provide custom disposal behavior (primarily to interop with a non-Disposable resource), meaning that you will want to run code that is associated with that resource. use is convenient for Disposables, defer is convenient for callbacks, neither are convenient for interop, hence the third method.

[ljharb]

That you can do it in two lines seems perfectly acceptable to me.

[bakkot]

I strongly disagree. There are other APIs for which "just do it in two steps" is totally fine, but not this one.

[rbuckton]

That you can do it in two lines seems perfectly acceptable to me.

It isn't to me. When this ships, there will be a long tail of APIs between the DOM, NodeJS, Electron, third-party packages, etc. that will not yet support Symbol.dispose, meaning that for several years its likely adopt would see more use than use. It is a matter of convenience that, to me, trumps having both .then and .catch on Promise (for example).

[rbuckton]

I'm going to go ahead and merge #103 and we can discuss this again in plenary.

[hax]

If you're making a consistency argument, then the thisArg pattern is exactly what you'd want to follow.

I really hope we won't repeat thisArg pattern in any other place except Array methods. thisArg pattern IMO have big ergonomics issues, not only type infer, but also error-prone. As I understand, the only reason we have thisArg pattern is because we didn't have arrow functions and creating new functions may still have big cost in some engines in JavaScript 1.7 era. Even that, the API could have been designed like arr.forEach([thisArg, method]) which avoid the issues.