Triple-slash reference type directives can override the import mode used for their resolution by weswigham · Pull Request #47732 · microsoft/TypeScript · GitHub

They now use the file's default mode by default, rather than always using commonjs. The new arguments to the
reference directive look like:

///<reference types="pkg" resolution-mode="require" />

or

///<reference types="pkg" resolution-mode="import" />

We consider it an error to see these kinds of references outside of node12 or nodenext resolution. This change is separate from the change adding similar configurability for type-only imports (and import types), as the plumbing work to support multi-modal type reference directives is much more extensive (since the API changes to support modality for them was not yet in place), so is probably best reviewed on its own.

Fixes #47795
Fixes #47806

…sed for their resolution They now use the file's default mode by default, rather than always using commonjs. The new arguments to the reference directive look like: ```ts ///<reference types="pkg" resolution-mode="require" /> ``` or ```ts ///<reference types="pkg" resolution-mode="import" /> ```

Re emit: I’m in favor of what you have now, not changing what the user wrote in the directive. Looks good to me at a glance, will take a closer look tomorrow.

Hm, is this going to be problematic (though silenceable with --skipLibCheck) for library code? I guess all library code that’s written with nodenext can potentially present problems for consumers in a different resolution mode, so maybe this is nothing new.

Re emit: I’m in favor of what you have now, not changing what the user wrote in the directive.

As-is, this always emits the appropriate resolution-mode in the directive, weather it was originally explicitly written or not.

Oh, I misunderstood. Hm, I’ll have to think about that. I think it complicates the library code problem more. If you happen to compile your library in nodenext but resolution would work for consumers under commonjs, it would be a shame to have errors on those references just for including superfluous syntax.

I think on principle I lean toward leaving code the user wrote as-is unless there’s a good reason it needs to be changed.

I now omit the resolution-mode override if it's unneeded in the resulting file, so they're not usually present. While preserving the input formatting where possible sounds nice, to do so needs context about where the original comment came from preserved (so we know when we have to change that formatting), which, since it's derived from a comment, is not the case right now (unlike nodes, which retain pointers back to their original source, FileReferences contain no such data and get copied around willy-nilly - we happily pull type references from random other files in your build!). It's probably doable, but'd complicate FileReferences a bit, since they'd need to somehow inherit their original file's applied resolution mode.

I've mentioned it in some linked issues, but the cache changes in this PR also happen to fix some language service issues to do with format detection caching, and as such this now includes a test to that effect. ♥

Would be good to get another set of eyes since it’s a big changeset, but looks good to me (for merging after 4.6 RC, I assume). Do you know where the bug in the caching code was that you accidentally fixed?

In the resolutionCache, the mode wasn't consistently handed in by all callers before, which wasn't obvious prior to this PR because triple-slash references intentionally didn't pass a mode.

Specifically, it wasn't being propagated here.

Some comments from looking over the code. I'll look at the tests next.

resolveTypeReferenceDirectiveNamesWorker -> actualResolveTypeReferenceDirectiveNamesWorker

I think our layers of indirection have gotten out of control

I think I basically get what's going on, but I don't think I can predict many of the ramifications. I'd vote that we merge this early enough for people to try it out.

…deOverride3.ts Co-authored-by: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com>

Fixes microsoft/TypeScript#47732

Wesley Wigham (weswigham) requested review from Andrew Casey (amcasey) and Andrew Branch (andrewbranch) February 3, 2022 23:29

TypeScript Bot (typescript-bot) assigned Wesley Wigham (weswigham) Feb 3, 2022

TypeScript Bot (typescript-bot) added Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Feb 3, 2022

Andrew Branch (andrewbranch) reviewed Feb 4, 2022

View reviewed changes

Omit redundant import modes in emitter

7997a62

This was referenced Feb 8, 2022

Add import assertions to type only imports and import types to force the resolution mode of the specifier #47807

Merged

Node 12 ESM reports different errors between editor and CLI #47795

Closed

TypeScript Bot (typescript-bot) added For Milestone Bug PRs that fix a bug with a specific milestone and removed For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Feb 9, 2022

Add test for microsoft#47806

f615b78

Wesley Wigham (weswigham) mentioned this pull request Feb 9, 2022

VS Code/language server doesn't properly detect NodeNext module kind based on package.json #47806

Closed

Andrew Branch (andrewbranch) approved these changes Feb 9, 2022

View reviewed changes

Add server test for triple-slash reference mode overrides

a8519fe

Nathan Shively-Sanders (sandersn) reviewed Feb 12, 2022

View reviewed changes

Move FileReference mode into helper

ffe16a4

Nathan Shively-Sanders (sandersn) approved these changes Feb 15, 2022

View reviewed changes

Comment thread tests/cases/fourslash/server/nodeNextModuleKindCaching1.ts

Comment thread tests/baselines/reference/nodeModulesTripleSlashReferenceModeDeclarationEmit5(module=node12).js

Comment thread tests/cases/conformance/node/nodeModulesTripleSlashReferenceModeOverride3.ts Outdated

Update tests/cases/conformance/node/nodeModulesTripleSlashReferenceMo…

f919c2a

…deOverride3.ts Co-authored-by: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com>

Wesley Wigham (weswigham) force-pushed the assertions-for-resolution branch from 079515f to f919c2a Compare February 15, 2022 21:18

Wesley Wigham (weswigham) merged commit 67172e4 into microsoft:main Feb 15, 2022

Ryan Cavanaugh (RyanCavanaugh) mentioned this pull request Feb 17, 2022

TypeError: path.indexOf is not a function in github actions #47921

Closed

Ryan Cavanaugh (RyanCavanaugh) mentioned this pull request Feb 17, 2022

API break notification: crash in resolveTypeReferenceDirective TypeStrong/ts-node#1647

Closed

Daniel Rosenwasser (DanielRosenwasser) mentioned this pull request Feb 23, 2022

Design Meeting Notes, 2/9/2022 #48016

Closed

Daniel Rosenwasser (DanielRosenwasser) added Breaking Change Would introduce errors in existing code Domain: API Relates to the public API for TypeScript labels Apr 5, 2022

Sheetal Nandi (sheetalkamat) added a commit to microsoft/TypeScript-TmLanguage that referenced this pull request May 24, 2022

Resolution mode support

0dfae8c

Fixes microsoft/TypeScript#47732

Andrew Branch (andrewbranch) mentioned this pull request Aug 18, 2022

Different behavior for type reference directive in nodenext based on whether types are in @types or not #47444

Open

Microsoft (microsoft) locked as resolved and limited conversation to collaborators Oct 22, 2025

-                              processTypeReferenceDirective(fileName, resolvedTypeReferenceDirective, { kind: FileIncludeKind.TypeReferenceDirective, file: file.path, index, });
+                              const mode = ref.resolutionMode || file.impliedNodeFormat;
+                              if (mode && getEmitModuleResolutionKind(options) !== ModuleResolutionKind.Node12 && getEmitModuleResolutionKind(options) !== ModuleResolutionKind.NodeNext) {
+                                  programDiagnostics.add(createDiagnosticForRange(file, ref, Diagnostics.Resolution_modes_are_only_supported_when_moduleResolution_is_node12_or_nodenext));

                           tracing?.push(tracing.Phase.Program, "resolveTypeReferenceDirectiveNamesWorker", { containingFileName });
                           performance.mark("beforeResolveTypeReference");
-                          const result = actualResolveTypeReferenceDirectiveNamesWorker(typeDirectiveNames, containingFileName, redirectedReference);
+                          const result = actualResolveTypeReferenceDirectiveNamesWorker(typeDirectiveNames, containingFileName, redirectedReference, containingFileMode);

Triple-slash reference type directives can override the import mode used for their resolution by weswigham · Pull Request #47732 · microsoft/TypeScript · GitHub

Uh oh!

Triple-slash reference type directives can override the import mode used for their resolution - #47732

Conversation

Wesley Wigham (weswigham) commented Feb 3, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Andrew Branch (andrewbranch) left a comment

Choose a reason for hiding this comment

Uh oh!

Andrew Branch (andrewbranch) Feb 4, 2022

Choose a reason for hiding this comment

Uh oh!

Wesley Wigham (weswigham) commented Feb 4, 2022

Uh oh!

Andrew Branch (andrewbranch) commented Feb 4, 2022

Uh oh!

Andrew Branch (andrewbranch) commented Feb 4, 2022

Uh oh!

Wesley Wigham (weswigham) commented Feb 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Wesley Wigham (weswigham) commented Feb 9, 2022

Uh oh!

Andrew Branch (andrewbranch) left a comment

Choose a reason for hiding this comment

Uh oh!

Wesley Wigham (weswigham) commented Feb 9, 2022

Uh oh!

Wesley Wigham (weswigham) commented Feb 9, 2022

Uh oh!

Nathan Shively-Sanders (sandersn) left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Nathan Shively-Sanders (sandersn) Feb 11, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Nathan Shively-Sanders (sandersn) left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Wesley Wigham (weswigham) commented Feb 3, 2022 •
edited
Loading

Wesley Wigham (weswigham) commented Feb 9, 2022 •
edited
Loading