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

BaseClient/Client: Add support for 'using' keyword (Explicit Resource Management) #10057

Closed
JoshuaKGoldberg opened this issue Dec 27, 2023 · 1 comment · Fixed by #10063
Closed

Comments

@JoshuaKGoldberg
Copy link

Which application or package is this feature request for?

discord.js

Feature

Right now, as I understand it, the documented way to close out a Client after it's done is destroy():

const client = new Client({ intents: [/* ... */] });

await client.login(/* ... */);

// ... (use the client) ...

await client.destroy();

One gotcha is that if the ( use the client ) bit throws, the await client.destroy() won't run - unless you try/catch or try/(catch/)finally). This is a common gotcha in async code.

Explicit Resource Management is a TC39 proposal to add explicit syntax for situations where a resource should have some method called upon disposal. It's stage 3 now and supported in TypeScript >=5.2.

Ideal solution or implementation

Proposal: can the BaseClient class be given a [Symbol.asyncDispose] method that calls this.destroy()?

class BaseClient<Ready extends boolean = boolean> {
  // ... (existing class fields) ...

  async [Symbol.asyncDispose]() {
    await this.destroy();
  }
}

That way we'd be able to use clients like:

await using client = new Client({ intents: [/* ... */] });

...and be more assured that the client's .destroy() will be called and awaited when done.

Alternative solutions or implementations

For now, we can somewhat shim this in with:

class DisposableClient<Ready extends boolean = boolean> extends Client<Ready> {
	async [Symbol.asyncDispose]() {
		await this.destroy();
	}
}
await using client = new DisposableClient({
    intents: [GatewayIntentBits.Guilds],
});

Other context

I made a small standalone package containing this suggested DisposableClient: https://github.com/JoshuaKGoldberg/disposable-discord-client -> https://www.npmjs.com/package/disposable-discord-client

@Renegade334
Copy link
Contributor

Renegade334 commented Dec 27, 2023

Given that explicit resource management is still at the proposal phase, adding language features that don't yet exist in the language doesn't seem like a good idea.

Yes, Typescript 5.2 has a polyfill for it, but a) it requires very specific project configuration; and b) it requires meddling with primitive globals (assigning properties to the Symbol constructor) to avoid causing errors when running on versions of Node older than about six months or so, which don't natively polyfill the well-known Symbols.

Once the TC39 proposal is ratified and implemented in V8, I don't see why not.

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

Successfully merging a pull request may close this issue.

4 participants