Skip to content

Handle numeric literal export names in declaration emit#3919

Merged
ahejlsberg merged 6 commits into
mainfrom
copilot/fix-tsgo-printer-panics
May 18, 2026
Merged

Handle numeric literal export names in declaration emit#3919
ahejlsberg merged 6 commits into
mainfrom
copilot/fix-tsgo-printer-panics

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 16, 2026

The printer panics when emitting declarations for CommonJS files with numeric export names (e.g. exports[1] = 2). emitModuleExportName only handled KindIdentifier and KindStringLiteral, missing KindNumericLiteral. Additionally, the declaration transformer passed numeric literals through unchanged, producing invalid syntax like export { _exported as 1 }.

  • Added numeric-to-string conversion in getNameExpressionPreferringIdentifier in the declaration transformer, since numeric property names in JS are string properties — now emits valid export { _exported as "1" } syntax
  • No printer changes needed since the transformer converts numeric literals to string literals before they reach emitModuleExportName
  • Added test case covering exports[1], module.exports[1], and Object.defineProperty(exports, 1, {})

Add KindNumericLiteral case to emitModuleExportName in the printer,
which was causing a panic when emitting declarations for CommonJS files
with numeric export names like `exports[1] = 2`.

Agent-Logs-Url: https://github.com/microsoft/typescript-go/sessions/9f3fe2c6-afe3-4d4b-9709-13e88c7271b6

Co-authored-by: jakebailey <5341706+jakebailey@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix tsgo printer panics when emitting declarations for CommonJS files Handle numeric literal in emitModuleExportName May 16, 2026
Copilot AI requested a review from jakebailey May 16, 2026 19:56
Comment on lines +20 to +31
//// [DtsFileErrors]


bug.d.ts(2,22): error TS2300: Duplicate identifier '(Missing)'.
bug.d.ts(2,23): error TS1003: Identifier expected.
bug.d.ts(2,25): error TS1128: Declaration or statement expected.
bug.d.ts(4,24): error TS2300: Duplicate identifier '(Missing)'.
bug.d.ts(4,25): error TS1003: Identifier expected.
bug.d.ts(4,27): error TS1128: Declaration or statement expected.
bug.d.ts(6,24): error TS2300: Duplicate identifier '(Missing)'.
bug.d.ts(6,25): error TS1003: Identifier expected.
bug.d.ts(6,27): error TS1128: Declaration or statement expected.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot Probably this indicates further brokenness, but compare to Strada.

I'd think we'd want to export as "1" etc since numeric props are just number props in disguise? Not sure. Look into it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right — numeric props are just string props in disguise. Fixed in 0019d09 by converting NumericLiteral to StringLiteral in getNameExpressionPreferringIdentifier in the declaration transformer. Now emits export { _exported as "1" } instead of the invalid export { _exported as 1 }, eliminating the parse errors from the baseline (down from 9 to just 3 duplicate identifier errors, which are expected since all 3 exports share the same name "1").

…ration emit

Numeric property names in JS are string properties; convert NumericLiteral
to StringLiteral in getNameExpressionPreferringIdentifier so declarations
emit valid syntax like `export { _exported as "1" }` instead of the
invalid `export { _exported as 1 }`.

Agent-Logs-Url: https://github.com/microsoft/typescript-go/sessions/0d9fa6e3-1d16-42fa-824c-c7baf31e67ee

Co-authored-by: jakebailey <5341706+jakebailey@users.noreply.github.com>
Copilot AI changed the title Handle numeric literal in emitModuleExportName Handle numeric literal export names in declaration emit May 16, 2026
Copilot AI requested a review from jakebailey May 16, 2026 20:45
Copy link
Copy Markdown
Member

@jakebailey jakebailey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems plausible

// Numeric property names are string properties in JS; convert to string literal
nameExpr = tx.Factory().NewStringLiteral(nameExpr.Text(), ast.TokenFlagsNone)
}
if ast.IsStringLiteralLike(nameExpr) && scanner.IsIdentifierText(nameExpr.Text(), core.LanguageVariantStandard) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if ast.IsStringLiteralLike(nameExpr) && scanner.IsIdentifierText(nameExpr.Text(), core.LanguageVariantStandard) {
if ast.IsStringOrNumericLiteralLike(nameExpr) && scanner.IsIdentifierText(nameExpr.Text(), core.LanguageVariantStandard) {

}

func (tx *DeclarationTransformer) getNameExpressionPreferringIdentifier(nameExpr *ast.Node) *ast.Node {
if ast.IsNumericLiteral(nameExpr) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change isn't correct or necessary. Just use isStringOrNumericLiteralLike in the check below.

Copy link
Copy Markdown
Member

@ahejlsberg ahejlsberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot Please make the requested changes.

@ahejlsberg
Copy link
Copy Markdown
Member

@copilot Please make the requested changes.

Copilot stopped work on behalf of ahejlsberg due to an error May 17, 2026 12:23
Copilot AI requested a review from ahejlsberg May 17, 2026 12:23
@ahejlsberg
Copy link
Copy Markdown
Member

Nevermind on the above comments. I agree that converting numeric literals to string literals is the best option here, though it is different from what Strada does. Strada converts the numeric literal 1 to the identifier _1. Similarly, it converts non-identifier strings by injecting underscores, e.g. "a b" becomes _a_b. I think it is better to just export using string literals.

@ahejlsberg ahejlsberg marked this pull request as ready for review May 17, 2026 12:48
Copilot AI review requested due to automatic review settings May 17, 2026 12:48
@ahejlsberg ahejlsberg enabled auto-merge May 17, 2026 12:48
@ahejlsberg ahejlsberg requested a review from jakebailey May 17, 2026 12:49
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes declaration emit for CommonJS numeric export names by converting numeric property names into string-literal export names and adding regression coverage.

Changes:

  • Converts numeric CommonJS export names to string literals during declaration transformation.
  • Updates module export-name printing behavior.
  • Adds a compiler test and baselines for numeric export declarations.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
internal/transformers/declarations/transform.go Converts numeric export property names to string literals before declaration emit.
internal/printer/printer.go Adds numeric literal handling in module export-name emission.
testdata/tests/cases/compiler/numericExportNameDeclaration.ts Adds regression input covering numeric CommonJS export patterns.
testdata/baselines/reference/compiler/numericExportNameDeclaration.js Adds emitted declaration baseline and dts diagnostics.
testdata/baselines/reference/compiler/numericExportNameDeclaration.symbols Adds symbol baseline for the new test.
testdata/baselines/reference/compiler/numericExportNameDeclaration.types Adds type baseline for the new test.

Comment thread internal/printer/printer.go Outdated
Comment on lines +1300 to +1301
case ast.KindNumericLiteral:
p.emitNumericLiteral(node.AsNumericLiteral())
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot double check and then make this change

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 4304c9f. The KindNumericLiteral case in emitModuleExportName now emits the numeric literal as a quoted string literal ("1" instead of bare 1), since ModuleExportName syntax only accepts identifiers or string literals.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot what? That's not what I asked, the comment was saying the code was dead and I asked you to confirm

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed — the KindNumericLiteral case was dead code. The declaration transformer's getNameExpressionPreferringIdentifier (line 2176) converts all numeric literals to string literals before they reach the printer. The parser's parseModuleExportName/canParseModuleExportName only produces identifiers or string literals, never numeric literals. Removed the dead case in 95b66cc.

@ahejlsberg ahejlsberg added this pull request to the merge queue May 18, 2026
Merged via the queue into main with commit 7119345 May 18, 2026
21 checks passed
@ahejlsberg ahejlsberg deleted the copilot/fix-tsgo-printer-panics branch May 18, 2026 17:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

tsgo printer panics when emitting declarations for CommonJS files with numeric export names

5 participants