-
Notifications
You must be signed in to change notification settings - Fork 13.1k
Fix completions at type-only export specifiers
#62651
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
base: main
Are you sure you want to change the base?
Conversation
|
This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise. |
| //// | ||
| //// export { M/*6*/ } from "./m1" | ||
| //// export type { M/*7*/ } from "./m1" | ||
| //// export { type M/*8*/ } from "./m1" |
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.
Fixing this is the main goal of this PR. At the moment, on the main branch this errors with:
Error: At marker '8': No completions at position '232'
| // import { type foo| } | ||
| return containingNodeKind !== SyntaxKind.ImportSpecifier; | ||
| // export { type foo| } | ||
| return !isImportOrExportSpecifier(parent); |
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.
This is the only change needed to accomplish the goal of this PR. I threw in the rest of the changes to fix some redundant type keyword completions (all of that is reflected by the tests I added)
| case SyntaxKind.ExportSpecifier: | ||
| return (node as ExportSpecifier).isTypeOnly || (node as ExportSpecifier).parent.parent.isTypeOnly; | ||
| case SyntaxKind.ExportDeclaration: | ||
| return (node as ExportDeclaration).isTypeOnly && !!(node as ExportDeclaration).moduleSpecifier && !(node as ExportDeclaration).exportClause; |
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.
Those extra requirements were quite odd here (and got in a way of the tests I was adding for type keyword inclusion/exclusion). An export declaration should be treated as type-only regardless of the existence of other fields on that export declaration AST node. The introduced change also matches the import-oriented counterpart of the containing function (isTypeOnlyImportDeclaration) where there are no extra conditions in the SyntaxKind.ImportSpecifier case.
see https://github.com/microsoft/TypeScript/pull/62651/files#r2449778554