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

Generate Symbol.dispose on classes #4118

Merged
merged 4 commits into from
Sep 28, 2024
Merged

Generate Symbol.dispose on classes #4118

merged 4 commits into from
Sep 28, 2024

Conversation

H-Plus-Time
Copy link
Contributor

Implements #4117 (Explicit Resource Management support).

The main things I can think of for additional tests here are:

  1. A basic Deno-only test involving using.
  2. The polyfill mechanism (not worth it given the simplicity)

Copy link
Collaborator

@daxpedda daxpedda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't fully review this yet, but considering its not standard yet, this should be behind an experimental flag.

Otherwise I'm happy to include this.

@daxpedda daxpedda added the waiting for author Waiting for author to respond label Sep 19, 2024
@H-Plus-Time
Copy link
Contributor Author

Gating behind an experimental flag I think would be more trouble than it's worth, for a few reasons:

  1. At worst, it is a no-op (Symbol.dispose exists uniquely, or not at all).
  2. Integrating tools (wasm-pack, basically) would need to account for said flag.
  3. While the overall proposal is at stage 3, Symbol.dispose is on the far side (in the sense that the symbol name and meaning have been settled for several years).

@daxpedda
Copy link
Collaborator

  1. At worst, it is a no-op (Symbol.dispose exists uniquely, or not at all).

My worry is that if something changes, for whatever reason, we would need a breaking change.

  1. Integrating tools (wasm-pack, basically) would need to account for said flag.

Indeed. We could do it via an environment variable, like many other features.

  1. While the overall proposal is at stage 3, Symbol.dispose is on the far side (in the sense that the symbol name and meaning have been settled for several years).

Unfortunately I'm not well versed in the JS ecosystem at all, my experience so far has been that things change a lot. From the looks of it I agree that this is very unlikely to change, but I don't want to bet on it.

@H-Plus-Time
Copy link
Contributor Author

Oh, right, environment variables - yep, I'm fine with that, provided it's discoverable.

Copy link
Collaborator

@daxpedda daxpedda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I explicitly added EXPERIMENTAL to the name and added a changelog entry.

Thank you!

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

Successfully merging this pull request may close these issues.

2 participants