Skip to content

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
Member

@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
Member

  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
Member

@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!

@daxpedda daxpedda merged commit 9b81f4b into wasm-bindgen:main Sep 28, 2024
41 checks passed
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

2 participants