Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Support
using
andawait using
declarations #54505Support
using
andawait using
declarations #54505Changes from 1 commit
0cb73b3
0647856
8f082fc
688771f
80d0a84
a17aead
25a2fd3
40f546b
e0b9c73
08f41bd
7360338
587a1a4
3be9cf0
0b77050
bba71b1
2d4e506
ae6cbaf
afa64ca
5b6adea
7964b5f
0435db0
246a9a3
21b3e03
4a5ca67
b763f82
08a1434
2fc9bcf
1e8f045
3dc4011
8ad153a
51870e1
df5e591
558a502
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Large diffs are not rendered by default.
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.
In Babel we use
Symbol.for("Symbol.dispose")
andSymbol.for("Symbol.asyncDispose")
as fallbacks, so that users can use polyfills that do not modify the global scope. This is very useful for libraries, so that they can do somethingand they will be usable both natively and in older environments.
It might be great to align on this.
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.
TypeScript helpers generally don't polyfill/shim Symbols in this way. We usually depend on the developer to introduce any necessary global shim instead. I'm curious if @DanielRosenwasser has any thoughts on this approach, however.
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.
We discussed this in design meeting and don't believe TypeScript should support
Symbol.for()
as a fallback. For one,Symbol.for("Symbol.dispose")
is only marginally an improvement over just using a string like"@@dispose"
, and we don't treat the result ofSymbol.for()
as a meaningfully unique symbol, which is a requirement for us to properly type check it as a property name. As a result, a property name of[Symbol.dispose || Symbol.for("Symbol.dispose")]
produces a compilation error, so you wouldn't be able to produce that code without turning off error checking. Even if we did allowSymbol.for()
to produce a meaningfully unique symbol that we can type check, we would essentially have to bifurcateDisposable
to support each method name, and deal with the complexity of that only working in older compilation targets, since theSymbol.for()
fallback wouldn't be supported natively in--target esnext
.Babel supporting for
Symbol.for()
is actually a bit of a hazard for TypeScript. If someone were to publish a package that leveraged Babel's support for aSymbol.for()
fallback, they might in turn indicate that their objects are "disposable" in their documentation. That could lead to someone else putting together types for that package on DefinitelyTyped that indicate a class has aSymbol.dispose
method when it does not. This in turn could lead to errors during type check if the package's consumer isn't running with--lib esnext.disposable
, which could then result in them changing their--lib
to make the error go away without ensuring that their runtime environment actually supports a globalSymbol.dispose
.While I think it is commendable to support users' efforts to avoid global scope modifications, I don't think this is an approach TypeScript can take here. I'd even go so far as to recommend that Babel not provide such support, but given that Babel's support for
Symbol.for("Symbol.dispose")
only has an exteremly remote possibility of causing problems for TypeScript projects, I'll leave that up to you.I'll also note that, since the native
DisposableStack
has a built-in mechanism to adopt non-disposable resources viastack.adopt(value, v => v.close())
andstack.defer(() => { value.close(); })
, a workaround likeSymbol.for()
may not even be warranted.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.
Unrelated: at what point will it be OK for us to use es6 features like object shorthands in our esnext downlevel helpers?
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.
If our helpers were an AST instead of a string, then we could arguably downlevel them on demand. Unfortunately, that wouldn't work for
tslib
. Since we aren't downleveling the helpers, I'd say we can use new syntax only if we retire--target es5
and--target es3
.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 could copy your idea to optimize away the
next()
tail recursion that Babel uses 👀