Restore import defer = parsing#61837
Uh oh!
There was an error while loading. Please reload this page.
Conversation
typescript-bot added the For Uncommitted Bug 
typescript-bot commented Jun 9, 2025
This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise. |
1 similar comment
typescript-bot commented Jun 9, 2025
This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise. |
There was a problem hiding this comment.
Pull Request Overview
This PR restores the parsing of the "defer" identifier in import equals declarations and enhances the test coverage for various scenarios involving "defer" in import statements.
- Adds new test cases for valid and invalid "defer" usage in various module systems.
- Updates baseline outputs to reflect the corrections.
- Refines the parser logic in src/compiler/parser.ts to correctly distinguish "defer" as a phase modifier.
Reviewed Changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| tests/cases/conformance/importDefer/*.ts | Added tests for "defer" usage in import equals and import declarations. |
| tests/baselines/reference/* | Updated baselines reflecting the new parsing behavior. |
| src/compiler/parser.ts | Modified the condition for recognizing "defer" to improve parsing accuracy. |
Comments suppressed due to low confidence (2)
src/compiler/parser.ts:8396
- [nitpick] The compound condition for detecting a deferred import is complex; adding an inline comment or refactoring using intermediate variables could improve code clarity.
identifier?.escapedText === "defer" && (token() === SyntaxKind.FromKeyword ? !lookAhead(nextTokenIsStringLiteral) : token() !== SyntaxKind.CommaToken && token() !== SyntaxKind.EqualsToken) src/compiler/parser.ts:8396
- [nitpick] Consider caching the result of token() in a local variable before the condition to ensure consistency and potentially reduce function calls.
identifier?.escapedText === "defer" && (token() === SyntaxKind.FromKeyword ? !lookAhead(nextTokenIsStringLiteral) : token() !== SyntaxKind.CommaToken && token() !== SyntaxKind.EqualsToken) 
jakebailey commented Jun 9, 2025
@typescript-bot test it |
typescript-bot commented Jun 9, 2025
Hey @jakebailey, the results of running the DT tests are ready. Everything looks the same! |
typescript-bot commented Jun 9, 2025
@jakebailey Here are the results of running the user tests with tsc comparing Everything looks good! |
typescript-bot commented Jun 9, 2025
@jakebailey Here they are:tscComparison Report - baseline..pr
System info unknown Hosts
Scenarios
Developer Information: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| identifier = isIdentifier() ? parseIdentifier() : undefined; | ||
| } | ||
| else if (identifier?.escapedText === "defer" && token() !== SyntaxKind.FromKeyword){ | ||
| else if ( |
There was a problem hiding this comment.
Why isn't this just the same as what's done for type above? You can't mix type and defer, so I would have expected both to parse in the same fashion.
There was a problem hiding this comment.
The difference is that the type parsing considers type as a modifier in import type foo =, while the defer parsing doesn't in import defer foo =.
Do you prefer if I unify the path, but then add a "nice" error for import defer foo = rather than just complaining about the unexpected foo/=?
There was a problem hiding this comment.
Daniel pointed out there's a difference between them in other ways like import type foo,{bar } from "..." is not legal as it's ambiguous whether or not bar is type or not. So, all good.
There was a problem hiding this comment.
Well technically we could still unify parsing for them and report that type case as an "early error" while still building the proper AST.
(I'm happy to both keep the code as-is or to update it)
There was a problem hiding this comment.
After thinking more, technically neither allow named bindings or defaults. I think this is okay as-is. I'll make a second PR to add more tests.
typescript-bot commented Jun 9, 2025
@jakebailey Here are the results of running the top 400 repos with tsc comparing Everything looks good! |
DanielRosenwasser merged commit 0dfd0c2 into microsoft:mainUh oh!
There was an error while loading. Please reload this page.
4 participants
Fixes#60757 (comment)
I also added a bunch more tests for when using
deferas a binding identifier in ES import declarations. Note thatimport defer from from "foo"is invalid, but I added a test to verify that it reports the same error as forimport defer foo from "foo".cc @jakebailey