[api] Stop dropping dispose Promises in the async API - #64584
Andrew Branch (andrewbranch) wants to merge 3 commits into
Conversation
typescript-automation
Bot
added
Author: Team
For Uncommitted Bug
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The disposal and diagnostic changes are coherent and tested; only a non-blocking generated test-title mismatch remains.
Review effort: Balanced
Findings: 1
Open (1)
What changed in this PR
Updates async API resources to expose awaited disposal and improves compiler guidance for incorrect synchronous disposal.
Changes:
- Replaces dropped disposal promises with
Symbol.asyncDispose. - Updates API tests to use
await using. - Adds an
await usingdiagnostic and compiler baselines.
| File | Description |
|---|---|
tsc/testdata/tests/cases/compiler/usingAsyncDisposable.ts |
Adds diagnostic coverage. |
tsc/testdata/baselines/reference/compiler/usingAsyncDisposable.types |
Records inferred types. |
tsc/testdata/baselines/reference/compiler/usingAsyncDisposable.symbols |
Records symbols. |
tsc/testdata/baselines/reference/compiler/usingAsyncDisposable.errors.txt |
Records improved diagnostics. |
tsc/internal/diagnostics/diagnostics_generated.go |
Adds generated diagnostic metadata. |
tsc/internal/diagnostics/diagnosticMessages.json |
Defines the diagnostic. |
tsc/internal/diagnostics/diagnosticMessages.generated.json |
Updates generated messages. |
tsc/internal/checker/checker.go |
Appends the await using suggestion. |
packages/typescript/test/sync/api.test.ts |
Adds synchronous disposal scopes. |
packages/typescript/test/async/api.test.ts |
Awaits resource disposal in tests. |
packages/typescript/src/api/sync/api.ts |
Updates generated sync disposal calls. |
packages/typescript/src/api/async/api.ts |
Exposes asynchronous disposal promises. |
Files not reviewed (1)
- tsc/internal/diagnostics/diagnostics_generated.go: Generated file
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
| diagnostics.The_initializer_of_a_using_declaration_must_be_either_an_object_with_a_Symbol_dispose_method_or_be_null_or_undefined, &diags) { | ||
| globalAsyncDisposableType := c.getGlobalAsyncDisposableType() | ||
| optionalAsyncDisposableType := c.getUnionType([]*Type{globalAsyncDisposableType, c.nullType, c.undefinedType}) | ||
| if globalAsyncDisposableType != c.emptyObjectType && c.isTypeAssignableTo(widenedInitializerType, optionalAsyncDisposableType) { |
There was a problem hiding this comment.
Er, why is this not just looking up a Symbol.asyncDispose member?
There was a problem hiding this comment.
The check for actually being able to use await using is this assignability check, so that seemed like a reasonable alternative to picking out null/undefined, looking up the member, and checking that member’s type. That's what the assignability check is going to do anyway. Just looking up the member without checking its type would suggest await using on something that had an incorrect [Symbol.asyncDispose]: never signature or something weird.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
A few objects in the async API accidentally used
[Symbol.dispose]()instead of[Symbol.asyncDispose](), throwing their disposal Promise away. I think that should be up to the consumer whether to fire and forget.The second commit improves our own diagnostic message when you use a
usingon an AsyncDisposable, suggesting you useawait usinginstead.