Improve CommonJS support

[ahejlsberg]

In this PR:

• Permit multiple module.exports assignments in a file and union the assigned types.
• Make JSDoc @typedef declarations available on imports of CJS modules that use module.exports.
• Exclude module.exports assignments from stricter checking of TypeScript export = declarations.

These changes align Corsa more closely with the current behavior in Strada and eliminate a large number of baseline differences.

[Copilot]

Pull request overview

This PR improves CommonJS support in the TypeScript compiler port by making three key changes:

1. Permitting multiple module.exports assignments and unioning their types
2. Making JSDoc @typedef declarations available on imports of CJS modules with module.exports
3. Excluding module.exports assignments from stricter TypeScript export = checking

These changes align Corsa more closely with Strada's behavior and eliminate numerous baseline differences.

Changes:

• Added mergedExportEquals field to ModuleSymbolLinks to track merged export types for CJS modules
• Updated test baselines to reflect removal of TS1203 errors for module.exports assignments
• Improved type resolution for CJS modules with multiple exports and typedefs

Reviewed changes

Copilot reviewed 295 out of 295 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
internal/checker/types.go	Added mergedExportEquals field to support merging multiple CJS exports
testdata/baselines/reference/submodule/**/*.diff	Removed TS1203 "Export assignment cannot be used when targeting ECMAScript modules" errors for module.exports
testdata/baselines/reference/submodule/**/*.types	Updated type information to show proper CJS module export types
testdata/baselines/reference/submodule/**/*.symbols	Updated symbol resolution for CJS exports
testdata/baselines/reference/conformance/**/*.errors.txt	Removed erroneous TS1203 errors from conformance tests

[weswigham]

It's funny, because this was, hands down, the biggest thing we wanted to avoid by using the reparsing approach for JSDoc. At least in my opinion.

Supporting this in the checker, without a doubt, requires some thought on how to support it in the declaration transformer, which seems absent here thus far. Naively, we'll produce something like

export type Foo = number | string
declare const _exports: number
export = _exports

which, uhhh, not valid TS. You can see that in a bunch of the .js diffs. In strada, what we'd do, symbolically, was see if the export= target was namespace-y, and, if so, make a namespace to merge with that namespace that contained all these merged-in typedefs. The declaration transformer needs logic like that now that this is in place, so it can produce

declare namespace _exports {
    export { Foo };
}
declare const _exports: number;
export = _exports;
type Foo = string | number;

[weswigham]

Do note, the later can still fail if whatever the export= points at doesn't support normal namespace merges! But that's more rare, at least.

[jakebailey]

I wish we would just make that first example legal TS already...

[ahejlsberg]

the biggest thing we wanted to avoid by using the reparsing approach for JSDoc

Here's the scenario that concerns me. Say you have a CommonJS module like this:

/** @typedef {string[]} Foo */
module.exports.a = 1
module.exports.b = "hello"

and a client that imports it:

const m = require("./mod");
/** @type {m.Foo} */
let x;

This works fine and you get to reference m as a namespace alias in the @type annotation. But if you instead use a module.exports assignment in the module:

/** @typedef {string[]} Foo */
module.exports = { a: 1, b: "hello" }

then the client breaks because you can't reference m.Foo. You instead have to write @type {import("./mod").Foo} which does work (because, inconsistently, we haven't made that an error). And if we don't make it an error, then we still have to support the namespace pattern you mention in declaration emit.

Do note, the later can still fail if whatever the export= points at doesn't support normal namespace merges! But that's more rare, at least.

I don't think there are cases where it fails. The export= symbol generated by a module.exports assignment will never have a namespace meaning, so it's always happy to merge with an uninstantiated namespace containing the @typedef types.

[ahejlsberg]

So, I think there are two options. Either have module.exports assignments merge with an uninstantiated namespace that contains the @typedef types and support that in declaration emit, or completely eliminate exporting of @typedef types from modules with module.exports assignments. Right now we're sorta sitting in between two chairs because type annotations can use import to get at the types.

[weswigham]

This is another interesting input:

const a = require("./mod")

/**
 * @typedef {string | number} Foo
 */

module.exports = a

if you merge Foo down into a's target, Foo appears in mod, which seems wrong (but iirc is what strada does), however there's afaik no uniformly valid declaration emit. Another one is

const a = require("./mod")

/**
 * @typedef {string | number} Foo
 */

module.exports = a
module.exports[a.fieldName] = 12

ie, late-bound commonjs module exports (which maybe we don't support any more, but we used to) - those are a mire with this merge in place, and strada's behavior for both is... questionable. Especially in the context of isolated or syntactic declaration emit.

Now, a lot of this can be resolved if we do the work to allow merges like those in .d.ts files, too; that way we can just emit

export type Foo = number | string
declare const _exports: number
export = _exports

or

import * as mod from "./mod"
export type Foo = number | string
export = mod

and not error on read-back; but the issue I had when I tried to implement that is that it exposes all the badness of this merge to all TS users. If we can support it without implementation jankiness, it would definitely be preferable, though.

[sandersn]

I don't have much to add to what Wesley said, except:

1. In JS, we need typedefs to export alongside module.exports assignments; that's the real feedback we got from an internal user, and webpack still relies on it all over the place. We can't abandon the feature entirely.
2. If we only support JS, we don't have to support every case. I scanned my JS corpus and found no occurrences of the re-export+typedef pattern, for example. That doesn't apply if we also want to support TS.

Overall I'm with Wesley and Jake--I think we should go for the least-janky implementation that covers TS and JS completely.

I'm not current on the state of reparsing anymore, but wouldn't a JS-only implementation shrink reparsing even further. Maybe all statement-level reparsing would get dropped?

[weswigham]

A JS-only implementation struggles to be represented as a declaration file for incremental or publishing purposes, which is strada's issue. It's OK insofar as the analysis works, but definitely not ideal for the whole stack. We can take that approach again, but we need to introduce a lot of errors banning many forms of it when declaration emit is on because we can't roundtrip it, which... maybe makes it much less appealing.

It'd definitely be best to get something working across languages (ideally requiring only syntactic analysis to produce declarations, for isolatedDeclaration's sake), for sure. The issue I had is that "an alias with stuff attached" does not behave well in our current checking pipelines - I was hacking stuff up left and right and it just felt suuuuuper janky - even moreso than this second-order merge - making things that look like aliases to constant values have type exports - but only if you're still at the alias!

Honestly, I think we probably do need a dedicated non-alias thing for the module to look like when you merge an export= into a module with type exports (since it's no longer "effectively replaced by" the target of the alias, as it's augmenting it), the trouble is maintaining that and making it "look like" the target of the alias at the same time in all the places that matters without silently dropping parts of the symbol's meaning along the way. It probably does just touch a lot, since it basically requires all the handling you'd associate with a new root symbol kind flag, I think.

[ahejlsberg]

Ok, I'm going to come up with an implementation that allows both TS and JS to export types from modules with export assignments. I think I will also move the merging logic to the binder so it is generally available. I don't think we need a new kind of symbol. We should be able to add an ast.SymbolFlagsNamespaceModule flag to the export= symbol and then add the exported type symbols to the export= symbol's Exports map (which is otherwise empty). This means that the exported types will take precedence over any exported types in the export= target, but seeing as this whole construct is currently an error, it is not like it will break anything. At least in theory.