Reparse CJS export assignments only when not shadowed by locals by ahejlsberg · Pull Request #3345 · microsoft/typescript-go

ahejlsberg · 2026-04-05T15:16:10Z

This PR improves our detection of CJS export assignments (module.exports = ... and exports.xxx = ...) by tracking local shadowing declarations of module and exports identifiers in the parser and only reparsing CJS export assignments when no shadowing local declarations are present. Tracking in the parser allows us to preserve our reparsing strategy which we'd likely have to abandon to fully solve the issue in the binder. The parser tracking can't account for hoisted declarations and thus technically isn't 100% accurate, but it is far better than nothing.

In addition to #2656, this fixes an issue that exists in both Strada and Corsa where a simple file like the following

function ff(module) {
    module.exports = 42
}

is considered a CJS module even though it shouldn't be.

Fixes #2656.

Copilot

Pull request overview

This PR improves CommonJS export assignment detection in JavaScript parsing by tracking whether module / exports identifiers are shadowed by locals, and suppressing the CommonJS reparsing step when shadowing is present (addressing #2656).

Changes:

Add parser-level shadowFlags tracking for module / exports bindings and use it to gate CommonJS reparsing.
Extend binding/checking to better handle CommonJS export constructs (including a new checker path for CommonJSExport).
Add new compiler tests + baseline updates covering shadowed module/exports scenarios and the original regression.

Reviewed changes

Copilot reviewed 32 out of 32 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
`internal/parser/parser.go`	Introduces `ShadowFlags` state and updates parsing to track and scope shadowing.
`internal/parser/reparser.go`	Gates CommonJS reparsing (`module.exports =`, `exports.x =`) on `shadowFlags`.
`internal/checker/checker.go`	Adds `CommonJSExport` checking to emit TS2309-style diagnostics in ESM contexts.
`internal/binder/binder.go`	Treats additional JS assignment kinds as expando assignments for binding.
`internal/ast/utilities.go`	Extends declaration-name helpers to recognize `module.exports =` assignments.
`testdata/tests/cases/compiler/*.ts`	Adds new regression/coverage tests for shadowed `module`/`exports`.
`testdata/baselines/reference/**`	Updates expected baselines impacted by the new detection/checking behavior.

internal/parser/reparser.go

Copilot · 2026-04-05T15:24:48Z

internal/parser/parser.go

+		saveShadowFlags := p.shadowFlags
 		statements := p.parseList(PCBlockStatements, (*Parser).parseStatement)
+		p.shadowFlags = saveShadowFlags


Saving/restoring shadowFlags for every { ... } block treats shadowing as purely block-scoped. That’s incorrect for var (and function declarations in non-strict modes), which are function/global scoped even when declared inside a nested block.

Example: if (cond) { var module = {}; } module.exports = 1; should treat module as shadowed after the block, but restoring shadowFlags here will clear the shadow and may incorrectly reparse module.exports as a CommonJS export assignment.

Suggestion: track shadowing with a function-scope layer (for var/hoisted decls) separate from a block-scope layer (for let/const/catch), or otherwise propagate function-scoped shadowing out of the block before restoring.

Suggested change

saveShadowFlags := p.shadowFlags

statements := p.parseList(PCBlockStatements, (*Parser).parseStatement)

p.shadowFlags = saveShadowFlags

statements := p.parseList(PCBlockStatements, (*Parser).parseStatement)

This is a known limitation. Tracking in the parser can't fully handle hoisting because usages may precede a declaration.

Copilot · 2026-04-05T15:24:48Z

internal/parser/parser.go

@@ -1326,6 +1337,7 @@ func (p *Parser) parseForOrForInOrForOfStatement() *ast.Node {
 		p.parseExpected(ast.KindCloseParenToken)
 		result = p.factory.NewForStatement(initializer, condition, incrementor, p.parseStatement())
 	}
+	p.shadowFlags = saveShadowFlags
 	p.finishNode(result, pos)


Restoring shadowFlags at the end of parsing a for (...) ... statement will drop shadowing introduced by var in the initializer, even though var bindings are visible after the loop.

Example: for (var module of xs) {} module.exports = 1; should treat module as shadowed in the subsequent statement, but the restore here will clear it and may incorrectly reparse module.exports.

Suggestion: only unwind block-scoped shadowing here, while preserving function-scoped shadowing (e.g., var bindings) so it remains in effect after the loop.

Again, this is a known limitation.

Copilot · 2026-04-05T15:24:48Z

internal/parser/parser.go

+func (p *Parser) updateShadowFlags(name *ast.Node) {
+	if p.isJavaScript() && name != nil {
+		if ast.IsModuleIdentifier(name) {
+			p.shadowFlags |= ShadowFlagsModule
+		}
+		if ast.IsExportsIdentifier(name) {
+			p.shadowFlags |= ShadowFlagsExports
+		}
+	}


shadowFlags is parser-global state, so it needs to be saved/restored for all constructs that introduce a new binding scope. The current saves/restores cover blocks, for, catch, and some function/class forms, but arrow functions and switch case blocks don’t appear to reset shadowFlags when exiting their scope.

That can cause bindings like (module) => {} or switch { case 0: let exports; } to permanently set ShadowFlagsModule/Exports and suppress later CommonJS export reparsing at the outer scope.

Suggestion: apply the same save/restore pattern around arrow function parameter+body parsing and around parseCaseBlock (or otherwise centralize scope management for shadow tracking).

Arrow functions are now handled. Case blocks don't have their own scope so need no additional handing.

jakebailey · 2026-04-06T17:31:52Z

What's funny is that I think cjs-module-lexer (what node uses to load CJS from ESM statically) does actually want to see these nested declarations as exports; check out the examples...

That being said, that does not mean we have to do so for these, just that their ESM loader won't error if you bind them from ESM (they'll just be undefined I think).

ahejlsberg · 2026-04-06T19:54:55Z

What's funny is that I think cjs-module-lexer (what node uses to load CJS from ESM statically) does actually want to see these nested declarations as exports; check out the examples...

I played with it a bit, and sure enough, it doesn't do any scope analysis. So you could argue we should just leave it the way it is without this PR. However, in #2656 it is unfortunate that we detect CJS when there are ESM constructs that clearly make the example an ES module. Strada detects ESM at parse time and then ignores CJS constructs at bind time because the module is marked ESM. We can't really do that with reparsing because the ESM constructs may come later in the file (as is the case in #2656). So not clear what to do there.

jakebailey · 2026-04-06T20:28:35Z

internal/parser/parser.go

 	jsdocScannerInfoHasSeeOrLink
 )

+type ShadowFlags int32


The type doesn't need to be exported either, technically

jakebailey · 2026-04-06T20:30:07Z

We can't really do that with reparsing because the ESM constructs may come later in the file (as is the case in #2656). So not clear what to do there.

Yeah. #3167 was attempting to solve this in the binder for that reason.

All in all I'm getting less and less happy with reparsing; we are basically doing a bit of binder-esque work in the parser with this PR 😄

ahejlsberg · 2026-04-06T20:38:25Z

All in all I'm getting less and less happy with reparsing; we are basically doing a bit of binder-esque work in the parser with this PR 😄

Agreed. At this point I'm thinking the easiest way to solve this is to get rid of the JSExportAssignment and CommonJSExport reparsing in reparseCommonJS and just do binding and checking of those the way we did in Strada. That way, we can completely ignore CJS constructs when the parser marks a module as ESM.

ahejlsberg added 4 commits April 5, 2026 10:54

Reparse CJS export assignments only when not shadowed by locals

c04b6a1

Error on CJS exports.xxx assignments in ES modules

a3034a4

Accept new baselines

6415aaf

Add tests

ba1e5fd

ahejlsberg requested review from RyanCavanaugh, Copilot and jakebailey and removed request for Copilot April 5, 2026 15:16

Copilot started reviewing on behalf of ahejlsberg April 5, 2026 15:17 View session

ahejlsberg requested a review from Copilot April 5, 2026 15:17

Copilot started reviewing on behalf of ahejlsberg April 5, 2026 15:18 View session

Copilot AI reviewed Apr 5, 2026

View reviewed changes

ahejlsberg added 5 commits April 5, 2026 12:30

Check shadowing for both exports.xxx and module.exports.xxx

f7c5801

Save and restore shadowFlags in arrow function parsing

6b23368

Add more tests

a71867d

ParserState.shadowFlags shouldn't be public

a25f65e

Merge branch 'main' into fix-2656

e87090d

jakebailey reviewed Apr 6, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reparse CJS export assignments only when not shadowed by locals#3345

Reparse CJS export assignments only when not shadowed by locals#3345
ahejlsberg wants to merge 9 commits intomainfrom
fix-2656

ahejlsberg commented Apr 5, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI Apr 5, 2026

Uh oh!

ahejlsberg Apr 5, 2026 •

edited

Loading

Uh oh!

Copilot AI Apr 5, 2026

Uh oh!

ahejlsberg Apr 5, 2026

Uh oh!

Copilot AI Apr 5, 2026

Uh oh!

ahejlsberg Apr 5, 2026

Uh oh!

jakebailey commented Apr 6, 2026 •

edited

Loading

Uh oh!

ahejlsberg commented Apr 6, 2026

Uh oh!

jakebailey Apr 6, 2026

Uh oh!

jakebailey commented Apr 6, 2026

Uh oh!

ahejlsberg commented Apr 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

ahejlsberg commented Apr 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

Uh oh!

ahejlsberg Apr 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

Uh oh!

ahejlsberg Apr 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

Uh oh!

ahejlsberg Apr 5, 2026

Choose a reason for hiding this comment

Uh oh!

jakebailey commented Apr 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ahejlsberg commented Apr 6, 2026

Uh oh!

jakebailey Apr 6, 2026

Choose a reason for hiding this comment

Uh oh!

jakebailey commented Apr 6, 2026

Uh oh!

ahejlsberg commented Apr 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ahejlsberg commented Apr 5, 2026 •

edited

Loading

ahejlsberg Apr 5, 2026 •

edited

Loading

jakebailey commented Apr 6, 2026 •

edited

Loading