-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Conversation
There was a problem hiding this 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.
Gating behind an experimental flag I think would be more trouble than it's worth, for a few reasons:
|
My worry is that if something changes, for whatever reason, we would need a breaking change.
Indeed. We could do it via an environment variable, like many other features.
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. |
Oh, right, environment variables - yep, I'm fine with that, provided it's discoverable. |
…l.dispose if it doesn't exist)
… var, update tests to reflect lack of said var
There was a problem hiding this 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!
Implements #4117 (Explicit Resource Management support).
The main things I can think of for additional tests here are:
using
.