Add --module preserve, support require in --moduleResolution bundler

[andrewbranch]

This PR allows users targeting Bun and bundlers to write fully-typed import statements and require calls in the same file.

Background and motivation

When we first introduced --moduleResolution bundler, we asked for feedback on how often people found themselves needing to write require calls in bundler code. We heard that it was very rare, so we chose not to perform any module resolution for require in that moduleResolution mode. This simplified resolution of package.json conditional "exports" and "imports"—if we were performing module resolution at all, it must have been from an import statement (or dynamic import), therefore we could unconditionally match the import condition. This resolution strategy would only be safe and accurate if we could ensure that these imports would not be transformed into require calls before a bundler saw them—and this is exactly what ts-loader, for example, would do if it used a tsconfig.json with "module": "commonjs". To avoid this problem, we required "module": "esnext" to be used in conjunction with "moduleResolution": "bundler". This restriction also prevented usage of import x = require("dep") syntax entirely.

The motivation for lifting this restriction is that we want a better default module resolution setting for VS Code than --moduleResolution node10, which lacks support for package.json "exports" but is still the default today¹. nodenext is too restrictive to be a good default, since many unconfigured projects might be targeting Bun or a bundler. We tried to switch to --moduleResolution bundler, but had to revert because JavaScript users complained that they stopped getting IntelliSense on things they required.

Adding support for require() resolution to --moduleResolution bundler solves this. Now, when module resolution is initiated from an import statement, it matches the import package.json "exports" condition, and when it’s initiated from a require call, it matches require.

--module preserve

In TypeScript files, though, require calls look like:

import x = require("dep");

which are not allowed in --module esnext. To represent the fact that Bun and most bundlers allow import and require in the same file, we need a --module mode that does the same, without assuming that imports will be transformed into require calls.

In --module preserve, ESM syntax is emitted as-written², while TypeScript-specific CommonJS syntax (import x = require("dep") and export = {}) is allowed and emitted as its standard CommonJS equivalent output (require variable statement and module.exports assignment, respectively). (Although we expect most users to use this mode with --noEmit for use in Bun and bundlers, the way code emits, or would emit if enabled, is the basis for how TypeScript performs type checking and module resolution on imports/requires.)

--module preserve implies these defaults:

• --moduleResolution bundler
• --esModuleInterop
• --resolveJsonModule

I also considered, and am open to, implying --moduleDetection force.

Footnotes

1. Recent confusion due to bad default module resolution: Intellisense not resolving modules when using exports in package.json #56412 ↩

2. --module preserve does not change the default behavior of import elision. Imports will never be transformed into require calls, but imports that are never used in JS-emitting positions will still be dropped, as in any other mode. To prevent unused imports from being elided, use --verbatimModuleSyntax. ↩

[typescript-bot]

Looks like you're introducing a change to the public API surface area. If this includes breaking changes, please document them on our wiki's API Breaking Changes page.

Also, please make sure @DanielRosenwasser and @RyanCavanaugh are aware of the changes, just as a heads up.

[andrewbranch]

@typescript-bot test top200

[typescript-bot]

Heya @andrewbranch, I've started to run the diff-based top-repos suite on this PR at 4142f99. You can monitor the build here.

Update: The results are in!

[typescript-bot]

@andrewbranch Here are the results of running the top-repos suite comparing main and refs/pull/56785/merge:

Everything looks good!

[jakebailey]

Seems good but I am wondering about the breaking API changes; the two I noted are used by some important libraries.

[jakebailey]

Similarly here, and also used externally: https://github.com/search?q=%2F%5C.getModeForUsageLocation%2F&type=code

[jakebailey]

I think this one is still breaking after the commit.

[jakebailey]

Oh, the new stuff is still "breaking" at a type level, but with hidden overloads?

[andrewbranch]

Yep. The thought is that I want tool authors to notice immediately, but I don’t want it to crash when users bring a newer TS to an existing tool.

[jakebailey]

Sounds good; hopefully nobody accidentally removes that backwards compat code in a refactor but that sounds doubtful.

[andrewbranch]

@jakebailey there’s no way to get a correct resolution mode answer in --module preserve without supplying compilerOptions. What do you think about making type-level breaking changes, but guarding against an undefined compilerOptions internally?

[jakebailey]

I think making them assert with a good message would be a good idea, at the least. But, I don't know how I feel about breaking ts-node, tsimp, rollup, svelte, rush, lit-analyzer, dt-tools, etc...

If undefined, can we just assume some options, keeping those from breaking? Presumably before, we were able to answer without the compiler option, right?

[andrewbranch]

Yes, the implementation will only be incorrect when --module is preserve. (By “guarding against an undefined compilerOptions” I meant “don’t throw, assume module is not preserve.”)

[jakebailey]

Yes, the implementation will only be incorrect when --module is preserve. (By “guarding against an undefined compilerOptions” I meant “don’t throw, assume module is not preserve.”)

Even better!

[jakebailey]

Looks good to my eye, thanks for adding the back compat code.

[mrazauskas]

Perhaps this is the right place to ask. I was wondering why new ModuleKinds and ModuleResolutionKinds are getting listed only in ts.ModuleKind and ts.ModuleResolutionKind, but not in ts.server.protocol.ModuleKind and ts.server.protocol.ModuleResolutionKind enums?

For instance, "node16" or "bundler" are missing here:

TypeScript/src/server/protocol.ts

Lines 3756 to 3759 in 55153b0

	export const enum ModuleResolutionKind {
	Classic = "Classic",
	Node = "Node",
	}

Similarly "node16" is missing and this PR is not adding "preserve" here:

TypeScript/src/server/protocol.ts

Lines 3745 to 3754 in 55153b0

	export const enum ModuleKind {
	None = "None",
	CommonJS = "CommonJS",
	AMD = "AMD",
	UMD = "UMD",
	System = "System",
	ES6 = "ES6",
	ES2015 = "ES2015",
	ESNext = "ESNext",
	}

[andrewbranch]

I honestly had no idea those were there. I think the only time they’d be used is for clients to send the CompilerOptionsForInferredProjects command, which today in VS Code still uses module: "CommonJS", moduleResolution: "Node", so we didn’t notice the need to add anything else. I’ll update them, thanks 👍

[typescript-bot]

Thanks for the PR! It looks like you've changed the TSServer protocol in some way. Please ensure that any changes here don't break consumers of the current TSServer API. For some extra review, we'll ping @sheetalkamat, @mjbvz, @zkat, and @joj for you. Feel free to loop in other consumers/maintainers if necessary.

[andrewbranch]

Thanks for the reviews @sheetalkamat and @jakebailey!