-
Notifications
You must be signed in to change notification settings - Fork 12.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
--moduleResolution minimal
#50153
Closed
andrewbranch
wants to merge
16
commits into
microsoft:main
from
andrewbranch:module-resolution/minimal
Closed
--moduleResolution minimal
#50153
andrewbranch
wants to merge
16
commits into
microsoft:main
from
andrewbranch:module-resolution/minimal
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Sep 2, 2022
typescript-bot
added
Author: Team
For Uncommitted Bug
PR for untriaged, rejected, closed or missing bug
labels
Sep 26, 2022
andrewbranch
requested review from
sheetalkamat,
DanielRosenwasser and
jakebailey
September 27, 2022 17:25
5 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
--moduleResolution minimal
is a new module resolution mode that only allows relative paths with file extensions in module specifiers. This means nonode_modules
, no dropping extensions, no special meaning for files namedindex.js
. This mode is suitable (though incomplete, without support for HTTP imports and import maps) for browser-native ESM. It’s also the common denominator in resolution features between the browser, Node, Deno, and bundlers (they don’t agree on much, so not much is included here).Importing from
./node_modules
not allowedI’ve decided to disallow relative imports into node_modules at least for now. The problem is that if you have
it would be safe, correct, and useful for us to type that module by pulling types from
./node_modules/@types/three/src/Three.d.ts
(which is not handled by any existing behavior—only sibling.d.ts
files are looked up for relative imports). But it’s not clear that such a mapping is always correct or even possible. If a library usestypesVersions
orexports
, the types may be nested in a subfolder. If those options don’t include a wildcard mapping, then it’s impossible to import arbitrary files from the package in Node-style resolution, which makes any attempt to infer a mapping from an arbitrary relative import of an implementation file to its type definition in some other folder feel highly suspect and fragile. The more you think about it, the more it feels like looking at package.json files or trying@types
violates the expectations for relative imports and one of the core assumptions of this resolution mode: that node_modules isn’t special.We could decide to truly give node_modules no special behavior, perhaps with the one concession that (non-type-only) imports from
node_modules/@types
are not allowed. For now I’ve chosen not to block resolution, but to issue an error in the checker when a relative import includes/node_modules/
in its module specifier. This keeps options open for us in the future.Folks using this for the browser probably will need a way to specify typings for their vendored dependencies, which I assume they will drop in a folder not called node_modules. #50600 tracks this feature request (and it need not be specific to
--moduleResolution minimal
).Importing
"*.ts"
allowedAlso included is a new compiler option
allowImportingTsExtensions
, which can be enabled only in--moduleResolution minimal
(though the planned--moduleResolution hybrid
will support it as well) and when--noEmit
or--emitDeclarationOnly
is set. When these conditions are met, imports will be allowed to use the extension of the target input file (.ts
,.tsx
,.mts
) rather than the output extension (.js
,.mjs
). Additionally, type-only imports under these options are allowed to use a.d.ts
suffix when resolving to a.d.ts
file. To help with portability, declaration files themselves are not subject to the--allowImportingTsExtensions
and--noEmit
requirements.URL handling not implemented yet
One thing that’s missing here is URL decoding. However, this is also missing from
node16
andnodenext
in ESM mode, and I think it would be a lot of noise to introduce it here in the same PR. I’d like to do that in a follow-up PR.Part of #50152
Closes #11979