fix(checker): don't emit typeof for private-named static methods - #64170
Conversation
Declaration emit produced `typeof C.<mangled name>` for a static `#name` method, because shouldWriteTypeOfFunctionSymbol only checked that the symbol was a static method and that it was accessible from the class body. A private-named symbol carries its internal mangled name, which is not valid syntax, so the resulting .d.ts failed to parse. A `#name` member has no entity name it can be referenced by, so fall back to a structural type, matching what instance `#name` methods already do. Fixes microsoft#61545
@microsoft-github-policy-service agree |
There was a problem hiding this comment.
🟢 Approval recommended
The focused fix correctly avoids unrepresentable private-name type queries and has comprehensive regression coverage.
Pull request overview
Prevents declaration emit from generating invalid typeof references to static #private methods.
Changes:
- Excludes private-identifier symbols from static method type queries.
- Adds coverage for basic, generic, recursive, property, and instance cases.
File summaries
| File | Description |
|---|---|
tsc/internal/checker/nodebuilderimpl.go |
Uses structural fallback for static private methods. |
tsc/testdata/tests/cases/conformance/classes/members/privateNames/privateNameStaticMethodDeclarationEmit.ts |
Adds regression cases. |
tsc/testdata/baselines/reference/conformance/privateNameStaticMethodDeclarationEmit.js |
Verifies valid declaration output. |
tsc/testdata/baselines/reference/conformance/privateNameStaticMethodDeclarationEmit.types |
Records inferred types. |
tsc/testdata/baselines/reference/conformance/privateNameStaticMethodDeclarationEmit.symbols |
Records symbol resolution. |
Review details
- Files reviewed: 5/5 changed files
- Comments generated: 0
- Review effort level: Balanced
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
Jake Bailey (jakebailey)
left a comment
There was a problem hiding this comment.
I found more examples that break in similar ways:
static "quoted-name"() {}
static getQuotedName() {
return Foo["quoted-name"];
}
static ["computed-name"]() {}
static getComputedName() {
return Foo["computed-name"];
}
static 1() {}
static getNumericName() {
return Foo[1];
}I think IsIdentifierText would work to fix all of these. Probably the test should be renamed too.
`IsIdentifierText` covers quoted, computed and numeric static method
names in addition to `#name`. All of them produce an entity name that
cannot be written, so declaration emit generated unparseable output:
static getQuoted(): typeof Foo.quoted-name;
static getNumeric(): typeof Foo.1;
Late-bound names are unaffected, `isLateBindableIndexSignature` already
excludes them from the check.
Rename the test accordingly and move it next to the other declaration
emit tests.
|
Fixes #61545
Declaration emit produces unparseable output when an inferred type refers to a static
#namemethod:shouldWriteTypeOfFunctionSymbolonly checked that the symbol is a static method and that it's accessible from the enclosing declaration, and a#namemember is accessible inside its own class body. So the node builder wrotetypeofusing the symbol's mangled internal name. Spelled correctly it still wouldn't resolve, since#members are erased to#private;in declarations.A
#namemember has no entity name it could be referenced by, so this excludes private-named symbols and lets static methods take the structural fallback that instance#namemethods already take:Same bug in Strada, where the name surfaces as
typeof Foo.__#1@#bar.