Add AST helpers used by api-extractor - #64439
Conversation
typescript-automation
Bot
added
Author: Team
For Uncommitted Bug
| export function isExternalModule(file: SourceFile): boolean { | ||
| return file.externalModuleIndicator !== undefined; |
There was a problem hiding this comment.
We're never going to be able to rid ourselves of that property are we
| return flags; | ||
| } | ||
|
|
||
| function skipParentheses(node: Expression): Expression { |
There was a problem hiding this comment.
Do we have instructions or any sort of way to remember / validate that we ported these right into the future?
There was a problem hiding this comment.
Do you mean if we change the implementation in Go?
There was a problem hiding this comment.
Yes, right (I chose a random place to comment, sorry)
There was a problem hiding this comment.
We should probably add something in the general Copilot instructions, honestly. That will probably help turn a lot of stuff up during automated reviews. The API is so broad that I don't know if there's a good way to get more specific.
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
getNameOfDeclaration incorrectly returns undefined for supported identifier and element-access expressions.
Get a fresh assessment by requesting another Copilot review.
Review effort: Balanced
Findings: 1
What changed in this PR
Adds AST compatibility helpers required by API Extractor.
Changes:
- Adds module, modifier, and declaration-name helpers.
- Exposes supporting AST types and generated enum data.
- Adds synchronous API tests.
| File | Description |
|---|---|
tsc/internal/ast/ast_generated.go |
Extends declaration names. |
tsc/internal/api/enum_values_generated.go |
Exposes JS declaration enum values. |
tools/scripts/tsc/generate-enums.ts |
Supports custom enum filenames. |
tools/scripts/tsc/ast.json |
Updates the AST schema. |
packages/typescript/test/sync/ast.test.ts |
Tests the new helpers. |
packages/typescript/src/enums/jsDeclarationKind.ts |
Adds the runtime enum. |
packages/typescript/src/enums/jsDeclarationKind.enum.ts |
Adds the enum type. |
packages/typescript/src/ast/utils.ts |
Implements the AST helpers. |
packages/typescript/src/ast/modifiers.ts |
Centralizes modifier conversion. |
packages/typescript/src/ast/is.generated.ts |
Updates the declaration-name guard. |
packages/typescript/src/ast/ast.generated.ts |
Updates the declaration-name union. |
packages/typescript/src/api/node/node.infrastructure.ts |
Reuses modifier conversion. |
Files not reviewed (2)
- tsc/internal/api/enum_values_generated.go: Generated file
- tsc/internal/ast/ast_generated.go: Generated file
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
| if (declaration.kind === SyntaxKind.ExportAssignment) { | ||
| const expression = (declaration as ExportAssignment).expression; | ||
| return isIdentifier(expression) ? expression : undefined; | ||
| } | ||
| return getDeclaredName(declaration); |
There was a problem hiding this comment.
The new behavior is accurate to Go's ast.GetNameOfDeclaration.
Jake Bailey (jakebailey)
left a comment
Jake Bailey (jakebailey)
left a comment
There was a problem hiding this comment.
Anyway, I'll mark this LGTM if it works for you, and we can figure out parity later
| return flags; | ||
| } | ||
|
|
||
| export function getCombinedModifierFlags(node: Declaration): ModifierFlags { |
There was a problem hiding this comment.
Was excited to use this, but it should be callable for ClassElements too. (From what I can tell all ClassElementBase are also Declaration but the two are not related.
isExternalModule(sourceFile)getCombinedModifierFlags(declaration)getNameOfDeclaration(declaration)