-
Notifications
You must be signed in to change notification settings - Fork 32
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
Expand Disposable
/AsyncDisposable
to parallel Python's ExitStack
/AsyncExitStack
#80
Comments
Oh I'd love to have a stack in the standard library!
I disagree that
I see you took this from the original
Is this really a useful method? I can't currently imagine a good use case, but it seems like an easy way to shoot yourself in the foot with using const stack = DisposableStack.from([getResourceA(), getResourceB()]) where the second allocation might throw. Or when the first returns an object that is not disposable and the
💡 Oh I like that interface!
I don't think this is bad. Tbh in Python I've been confused when writing a context manager whether I should put the allocation in Instead of some unclear things happening when an object is used as a disposable
I much prefer being explicit {
using const file = openFile();
… // do something with file
}
const lock = new Lock();
{
using void lock.acquire();
… // do something while the lock is acquired
} (It's still up to the implementation of
I wonder if this should actually return a new class PluginHost {
#channel;
#socket;
[Symbol.dispose];
constructor() {
using const stack = new Disposable.Stack();
this.#channel = stack.use(new NodeProcessIpcChannelAdapter(process));
this.#socket = stack.use(new NodePluginHostIpcSocket(this.#channel));
// if we made it here, then there were no errors during construction and
// we can safely move the disposables out of `stack` and onto the plugin host itself:
this[Symbol.dispose] = stack.move();
}
} You still could do |
Just to clarify, if you are talking about a list-like stack, that's not what this is.
This aligns with
I'm debating this. I found
Using classes as namespaces is something the committee has so far been against, having recently been opposed to
One purpose of function* g() {
yield getResourceA();
yield getResourceB();
}
using const stack = DisposableStack.from(g());
The resource passed into using const stack = new DisposableStack();
const x = stack.use(getResourceX());
const y = stack.use(getResourceY()); There isn't a way to get them back from
My preference is for an approach where the
From my experimentation, having I don't imagine that subclassing |
Of course, I was referring to the
Yeah, it does. On the other hand, I'm not sure how you intended the
Ah. I guess then we'd either have to introduce 4 new global bindings, or wait for builtin modules :-/
Hm, I'm not convinced of that. I doubt there will be lots of resources produced by true iterators/generators. Why wouldn't you rather write using const stack = new DisposableStack();
stack.use(getResourceA());
stack.use(getResourceB()); or even just using void getResourceA();
using void getResourceB(); If it needs to be a function, I'd much rather have function g() {
using const stack = new DisposableStack();
stack.use(getResourceA());
stack.use(getResourceB());
return stack.move();
}
using const stack = g(); than using a generator function. Accepting any iterable in using const stack = new DisposableStack();
for (const res of g()) stack.use(res); which isn't that much longer. It seems like there is no way to get back the resources that you passed into stack.use() (or into from()) from the stack object.
Exactly my thoughts. We could make it iterable, but I'd prefer it not to be. It's not meant to be a
Yeah, you're right about the reassignment, but on the other hand I do see the primary (only?) use case of function makePluginHost() {
using const stack = new Disposable.Stack();
return {
channel: stack.use(new NodeProcessIpcChannelAdapter(process)),
socket: stack.use(new NodePluginHostIpcSocket(this.#channel)),
[Symbol.dispose]: stack.moveToFunction(),
};
} Or
I've never been a proponent of having |
I'm not sure I understand what you mean here.
I can see your point. One of the other motivations for
I agree.
The Python documentation shows examples of with ExitStack() as stack:
files = [stack.enter_context(open(fname)) for fname in filenames]
close_files = stack.pop_all().close We could add a bound function makePluginHost() {
using const stack = new DisposableStack();
return {
channel: stack.use(new NodeProcessIpcChannelAdapter(process)),
socket: stack.use(new NodePluginHostIpcSocket(this.#channel)),
[Symbol.dispose]: stack.move().dispose,
};
}
There seems to be a growing sentiment within a subset of TC39 that subclassing of builtins is problematic (specifically around |
Oh, that's a perfect solution as well. Python has it easy with its autobound methods :-)
That was wrt subclassing. If class DisposableStack
#emptied = false;
#entries = [];
use(value, context = undefined) {
if (this.#emptied) throw new Error("Already disposed or moved");
if (typeof value == "function") {
this.#entries.push({dispose: value, resource: context});
} else if (value != null) {
const dispose = value[Symbol.dispose];
if (typeof dispose != "function") throw new Error("not a Disposable")
this.#entries.push({dispose, resource: value});
}
}
#dispose(...args) {
if (!this.#entries.length) return;
try {
const { resource, dispose } = this.#entries.pop();
%call(dispose, resource, ...args);
} finally {
this.#dispose(...args);
}
}
[Symbol.dispose](...args) {
if (this.#emptied) return;
this.#emptied = true;
this.#dispose(...args);
}
dispose = this[Symbol.dispose].bind(this);
#empty() {
if (this.#emptied) throw new Error("Already disposed or moved");
this.#emptied = true;
}
// Variant move-A - constructor with argument
move() {
this.#empty();
const C = %GetSpeciesConstructor(this);
let called = false;
return new C((...args) => {
if (called) return;
called = true;
this.#dispose(...args);
});
}
// Variant move-B - constructor without argument, but .use()
move() {
this.#empty();
const C = %GetSpeciesConstructor(this);
const result = new C();
let called = false;
result.use((...args) => {
if (called) return;
called = true;
this.#dispose(...args);
});
return result;
}
// Variant move-C - no subclassing
move() {
this.#empty();
const result = new DisposableStack();
result.#entries.push(...this.#entries);
this.#entries.length = 0;
return result;
}
} |
I think it's more like this (based on 25.1.5.3 ArrayBuffer.prototype.slice (start, end)): // Variant move-D: private slot (or field in this example) that is expected to have been initialized on the instance
move() {
if (this.#disposed) throw new ReferenceError();
const C = %GetSpeciesConstructor(this, %DisposableStack%);
const newStack = new C();
if (!(#entries in newStack)) throw new TypeError(); // RequireInternalSlot
newStack.#entries = this.#entries;
this.#entries = []; // NOTE: a DisposableStack is still usable after a move
return newStack;
} |
The example in the first post seem to imply a use case where the disposable resources are initialized in a block (the constructor), but their lifetime should actually extend past that block because the resources are saved outside the block into a longer lived container. In my mind a block with |
I've been experimenting with a variation of Python's
ExitStack
that I mentioned here. I quite like the API it provides in tandem withusing const
when building up custom disposables, as it significantly helps with cleanup when something fails during construction:I'm considering renaming the
Disposable
andAsyncDisposable
globals in this proposal toDisposableStack
andAsyncDisposableStack
to more clearly denote the resource stack-like behavior and adding theuse
method (nee.enter
,push
, etc.) to allow new resources to be added, and themove
method to move resources out of the stack and into a new one. Its a very convenient API when combined withusing const
.Tentative API sketch (in TypeScript parlance):
In particular:
DisposableStack.use
parallelsExitStack.enter_context
/ExitStack.push
/ExitStack.callback
__enter__
DisposableStack.move
parallelsExitStack.pop_all
DisposableStack[Symbol.dispose]
parallelsExitStack.close
AsyncDisposableStack.use
parallelsAsyncExitStack.enter_async_context
/AsyncExitStack.push_async_exit
/AsyncExitStack.push_async_callback
__aenter__
AsyncDisposableStack.move
has no parallel inAsyncExitStack
, but serves a similar purpose asExitStack.pop_all
AsyncDisposableStack[Symbol.asyncDispose]
parallelsAsyncExitStack.aclose
cc: @mhofman
The text was updated successfully, but these errors were encountered: