fix: Corsa differences in export= module augmentation#4446
fix: Corsa differences in export= module augmentation#4446pratheeknathani wants to merge 2 commits into
export= module augmentation#4446Conversation
There was a problem hiding this comment.
Pull request overview
Fixes a declaration-emit visibility edge case where an export = target (and its nested exported members) wasn’t treated as externally visible during .d.ts emit, causing a spurious TS4060 and preventing declaration output for a module augmentation scenario.
Changes:
- Treat declarations that are the target of an
export =in their containing external module as visible during declaration emit visibility checks. - Update submodule baselines to remove the erroneous TS4060 diagnostic and restore
a.d.tsemission for the affected test. - Remove now-unneeded triaged diff baselines for this case.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| internal/checker/emitresolver.go | Adds an export =-target visibility fast-path in determineIfDeclarationIsVisible. |
| testdata/baselines/reference/submoduleTriaged/compiler/exportAssignmentMembersVisibleInAugmentation.errors.txt.diff | Removes the triaged diff that captured the now-fixed TS4060 discrepancy. |
| testdata/baselines/reference/submodule/compiler/exportAssignmentMembersVisibleInAugmentation.errors.txt | Removes the error baseline now that TS4060 no longer occurs. |
| testdata/baselines/reference/submodule/compiler/exportAssignmentMembersVisibleInAugmentation.js | Updates baseline to include the restored a.d.ts output. |
| testdata/baselines/reference/submodule/compiler/exportAssignmentMembersVisibleInAugmentation.js.diff | Removes the diff baseline since the output now matches the reference expectation. |
| export function f(): T; // OK | ||
| ~ | ||
| !!! error TS4060: Return type of exported function has or is using private name 'T'. |
There was a problem hiding this comment.
Odd that this begins working
There was a problem hiding this comment.
Yeah, this lines up with what upstream TypeScript already does here (which is why there was a submoduleTriaged diff recording Corsa diverging from it).
TS marks foo/T visible via collectLinkedAliases(setVisibility=true) on the export = foo in index.d.ts, but Corsa moved that into PrecalculateDeclarationEmitVisibility, which only walks the file being emitted (a.ts), so the export = in the never-emitted index.d.ts was missed and the target fell through to isGlobalSourceFile and returned false. The fix just treats a declaration that's the export = target of its own module as visible, so it matches TS again.
There was a problem hiding this comment.
This sort of cross-file analysis is very spooky... It means emit can differ based on whether or not we use the checker for emit or not. Probably that is inevitable for this particular location, however
|
@typescript-bot perf test this faster |
|
@jakebailey Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
The PR deleted submoduleTriaged/compiler/exportAssignmentMembersVisibleInAugmentation.errors.txt.diff (the diff is now resolved) but left its entry in testdata/submoduleTriaged.txt. TestSubmoduleTriagedFilesExist in internal/testutil/baseline asserts every listed file exists, so it failed across every go test ./... job (no-submodules, noembed, race, ubuntu, windows, concurrent, generate). Remove the stale line; the issue-3481 group keeps augmentExportEquals2.errors.txt.diff, which is still a tracked diff.
|
Pushed The earlier commit correctly deleted the now-resolved triaged baseline The follow-up removes that single stale line. The issue-3481 group still keeps |
Summary
Test setup: // /node_modules/foo/index.d.ts export = foo; declare namespace foo { export type T = number; } // /a.ts import * as foo from "foo"; declare module "foo" { export function f(): T; // OK in TS } Corsa wrongly reported
TS4060: Return type of exported function has or is using private name 'T'when emitting...What changed
Test setup:
Corsa wrongly reported
TS4060: Return type of exported function has or is using private name 'T'when emittinga.d.ts. As a resulta.d.tswas never emitted.During declaration emit,
Tis resolved correctly, but its visibility is checked through its container, the namespacefoo.foois not markedexport; it is exported only throughexport = fooinindex.d.ts. TypeScript makes such anexport =target visible by callingcollectLinkedAliases(id, /*setVisibility*/ true)insidecheckExportAssignment, which runs while checking every file, including non-emitted ones likeindex.d.ts.Corsa moved that alias-visibility marking into the emit resolver (
PrecalculateDeclarationEmitVisibility), which only walks the file currently being emitted (a.ts/b.ts). Theexport = foolives inindex.d.ts, which is never emitted, sofoo(and thereforeT) was never marked visible.determineIfDeclarationIsVisiblethen fell through toisGlobalSourceFile(parent)and returned false, producing the spurious TS4060.Issue
Fixes #3481
Issue: #3481
Diffstat
Testing
Ran the relevant tests and linter for the changed files while developing.
Kept the change minimal and focused on this one issue.
AI assistance
I used GitHub Copilot to help write parts of this change. I've reviewed and tested it myself, I understand what it does, and I'll follow up on any review feedback.