-
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
match leading slash imports with path mappings - fixes #13730 #27980
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -730,6 +730,9 @@ namespace ts { | |
function tryLoadModuleUsingOptionalResolutionSettings(extensions: Extensions, moduleName: string, containingDirectory: string, loader: ResolutionKindSpecificLoader, | ||
state: ModuleResolutionState): Resolved | undefined { | ||
|
||
const resolved = tryLoadModuleUsingPathsIfEligible(extensions, moduleName, loader, state); | ||
if (resolved) return resolved.value; | ||
|
||
if (!isExternalModuleNameRelative(moduleName)) { | ||
return tryLoadModuleUsingBaseUrl(extensions, moduleName, loader, state); | ||
} | ||
|
@@ -738,6 +741,17 @@ namespace ts { | |
} | ||
} | ||
|
||
function tryLoadModuleUsingPathsIfEligible(extensions: Extensions, moduleName: string, loader: ResolutionKindSpecificLoader, state: ModuleResolutionState) { | ||
const { baseUrl, paths } = state.compilerOptions; | ||
if (baseUrl && paths && !pathIsRelative(moduleName)) { | ||
if (state.traceEnabled) { | ||
trace(state.host, Diagnostics.baseUrl_option_is_set_to_0_using_this_value_to_resolve_non_relative_module_name_1, baseUrl, moduleName); | ||
trace(state.host, Diagnostics.paths_option_is_specified_looking_for_a_pattern_to_match_module_name_0, moduleName); | ||
} | ||
return tryLoadModuleUsingPaths(extensions, moduleName, baseUrl, paths, loader, /*onlyRecordFailures*/ false, state); | ||
} | ||
} | ||
|
||
function tryLoadModuleUsingRootDirs(extensions: Extensions, moduleName: string, containingDirectory: string, loader: ResolutionKindSpecificLoader, | ||
state: ModuleResolutionState): Resolved | undefined { | ||
|
||
|
@@ -816,22 +830,13 @@ namespace ts { | |
} | ||
|
||
function tryLoadModuleUsingBaseUrl(extensions: Extensions, moduleName: string, loader: ResolutionKindSpecificLoader, state: ModuleResolutionState): Resolved | undefined { | ||
const { baseUrl, paths } = state.compilerOptions; | ||
const { baseUrl } = state.compilerOptions; | ||
if (!baseUrl) { | ||
return undefined; | ||
} | ||
if (state.traceEnabled) { | ||
trace(state.host, Diagnostics.baseUrl_option_is_set_to_0_using_this_value_to_resolve_non_relative_module_name_1, baseUrl, moduleName); | ||
} | ||
if (paths) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this should be pulled out into separate function instead of inlining. Its easier to read. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I thought about that. I however couldn't think of a name. The name
The trace There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes please add that back in there since its used in there anyways. |
||
if (state.traceEnabled) { | ||
trace(state.host, Diagnostics.paths_option_is_specified_looking_for_a_pattern_to_match_module_name_0, moduleName); | ||
} | ||
const resolved = tryLoadModuleUsingPaths(extensions, moduleName, baseUrl, paths, loader, /*onlyRecordFailures*/ false, state); | ||
if (resolved) { | ||
return resolved.value; | ||
} | ||
} | ||
const candidate = normalizePath(combinePaths(baseUrl, moduleName)); | ||
if (state.traceEnabled) { | ||
trace(state.host, Diagnostics.Resolving_module_name_0_relative_to_base_url_1_2, moduleName, baseUrl, candidate); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
//// [tests/cases/compiler/pathMappingBasedModuleResolution_rootImport_aliasWithRoot.ts] //// | ||
|
||
//// [foo.ts] | ||
export function foo() {} | ||
|
||
//// [bar.js] | ||
export function bar() {} | ||
|
||
//// [a.ts] | ||
import { foo } from "/foo"; | ||
import { bar } from "/bar"; | ||
|
||
|
||
//// [foo.js] | ||
"use strict"; | ||
exports.__esModule = true; | ||
function foo() { } | ||
exports.foo = foo; | ||
//// [bar.js] | ||
"use strict"; | ||
exports.__esModule = true; | ||
function bar() { } | ||
exports.bar = bar; | ||
//// [a.js] | ||
"use strict"; | ||
exports.__esModule = true; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
=== /root/a.ts === | ||
import { foo } from "/foo"; | ||
>foo : Symbol(foo, Decl(a.ts, 0, 8)) | ||
|
||
import { bar } from "/bar"; | ||
>bar : Symbol(bar, Decl(a.ts, 1, 8)) | ||
|
||
=== /root/src/foo.ts === | ||
export function foo() {} | ||
>foo : Symbol(foo, Decl(foo.ts, 0, 0)) | ||
|
||
=== /root/src/bar.js === | ||
export function bar() {} | ||
>bar : Symbol(bar, Decl(bar.js, 0, 0)) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
[ | ||
"======== Resolving module '/foo' from '/root/a.ts'. ========", | ||
"Module resolution kind is not specified, using 'NodeJs'.", | ||
"'baseUrl' option is set to '/root', using this value to resolve non-relative module name '/foo'.", | ||
"'paths' option is specified, looking for a pattern to match module name '/foo'.", | ||
"Module name '/foo', matched pattern '/*'.", | ||
"Trying substitution './src/*', candidate module location: './src/foo'.", | ||
"Loading module as file / folder, candidate module location '/root/src/foo', target file type 'TypeScript'.", | ||
"File '/root/src/foo.ts' exist - use it as a name resolution result.", | ||
"======== Module name '/foo' was successfully resolved to '/root/src/foo.ts'. ========", | ||
"======== Resolving module '/bar' from '/root/a.ts'. ========", | ||
"Module resolution kind is not specified, using 'NodeJs'.", | ||
"'baseUrl' option is set to '/root', using this value to resolve non-relative module name '/bar'.", | ||
"'paths' option is specified, looking for a pattern to match module name '/bar'.", | ||
"Module name '/bar', matched pattern '/*'.", | ||
"Trying substitution './src/*', candidate module location: './src/bar'.", | ||
"Loading module as file / folder, candidate module location '/root/src/bar', target file type 'TypeScript'.", | ||
"File '/root/src/bar.ts' does not exist.", | ||
"File '/root/src/bar.tsx' does not exist.", | ||
"File '/root/src/bar.d.ts' does not exist.", | ||
"Directory '/root/src/bar' does not exist, skipping all lookups in it.", | ||
"Loading module as file / folder, candidate module location '/bar', target file type 'TypeScript'.", | ||
"File '/bar.ts' does not exist.", | ||
"File '/bar.tsx' does not exist.", | ||
"File '/bar.d.ts' does not exist.", | ||
"Directory '/bar' does not exist, skipping all lookups in it.", | ||
"'baseUrl' option is set to '/root', using this value to resolve non-relative module name '/bar'.", | ||
"'paths' option is specified, looking for a pattern to match module name '/bar'.", | ||
"Module name '/bar', matched pattern '/*'.", | ||
"Trying substitution './src/*', candidate module location: './src/bar'.", | ||
"Loading module as file / folder, candidate module location '/root/src/bar', target file type 'JavaScript'.", | ||
"File '/root/src/bar.js' exist - use it as a name resolution result.", | ||
"======== Module name '/bar' was successfully resolved to '/root/src/bar.js'. ========" | ||
] |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
=== /root/a.ts === | ||
import { foo } from "/foo"; | ||
>foo : () => void | ||
|
||
import { bar } from "/bar"; | ||
>bar : () => void | ||
|
||
=== /root/src/foo.ts === | ||
export function foo() {} | ||
>foo : () => void | ||
|
||
=== /root/src/bar.js === | ||
export function bar() {} | ||
>bar : () => void | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
//// [tests/cases/compiler/pathMappingBasedModuleResolution_rootImport_aliasWithRoot_differentRootTypes.ts] //// | ||
|
||
//// [foo.ts] | ||
export function foo() {} | ||
|
||
//// [bar.js] | ||
export function bar() {} | ||
|
||
//// [a.ts] | ||
import { foo as foo1 } from "/foo"; | ||
import { bar as bar1 } from "/bar"; | ||
import { foo as foo2 } from "c:/foo"; | ||
import { bar as bar2 } from "c:/bar"; | ||
import { foo as foo3 } from "c:\\foo"; | ||
import { bar as bar3 } from "c:\\bar"; | ||
import { foo as foo4 } from "//server/foo"; | ||
import { bar as bar4 } from "//server/bar"; | ||
import { foo as foo5 } from "\\\\server\\foo"; | ||
import { bar as bar5 } from "\\\\server\\bar"; | ||
import { foo as foo6 } from "file:///foo"; | ||
import { bar as bar6 } from "file:///bar"; | ||
import { foo as foo7 } from "file://c:/foo"; | ||
import { bar as bar7 } from "file://c:/bar"; | ||
import { foo as foo8 } from "file://server/foo"; | ||
import { bar as bar8 } from "file://server/bar"; | ||
import { foo as foo9 } from "http://server/foo"; | ||
import { bar as bar9 } from "http://server/bar"; | ||
|
||
|
||
//// [foo.js] | ||
"use strict"; | ||
exports.__esModule = true; | ||
function foo() { } | ||
exports.foo = foo; | ||
//// [bar.js] | ||
"use strict"; | ||
exports.__esModule = true; | ||
function bar() { } | ||
exports.bar = bar; | ||
//// [a.js] | ||
"use strict"; | ||
exports.__esModule = true; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,81 @@ | ||
=== /root/a.ts === | ||
import { foo as foo1 } from "/foo"; | ||
>foo : Symbol(foo1, Decl(a.ts, 0, 8)) | ||
>foo1 : Symbol(foo1, Decl(a.ts, 0, 8)) | ||
|
||
import { bar as bar1 } from "/bar"; | ||
>bar : Symbol(bar1, Decl(a.ts, 1, 8)) | ||
>bar1 : Symbol(bar1, Decl(a.ts, 1, 8)) | ||
|
||
import { foo as foo2 } from "c:/foo"; | ||
>foo : Symbol(foo2, Decl(a.ts, 2, 8)) | ||
>foo2 : Symbol(foo2, Decl(a.ts, 2, 8)) | ||
|
||
import { bar as bar2 } from "c:/bar"; | ||
>bar : Symbol(bar2, Decl(a.ts, 3, 8)) | ||
>bar2 : Symbol(bar2, Decl(a.ts, 3, 8)) | ||
|
||
import { foo as foo3 } from "c:\\foo"; | ||
>foo : Symbol(foo3, Decl(a.ts, 4, 8)) | ||
>foo3 : Symbol(foo3, Decl(a.ts, 4, 8)) | ||
|
||
import { bar as bar3 } from "c:\\bar"; | ||
>bar : Symbol(bar3, Decl(a.ts, 5, 8)) | ||
>bar3 : Symbol(bar3, Decl(a.ts, 5, 8)) | ||
|
||
import { foo as foo4 } from "//server/foo"; | ||
>foo : Symbol(foo4, Decl(a.ts, 6, 8)) | ||
>foo4 : Symbol(foo4, Decl(a.ts, 6, 8)) | ||
|
||
import { bar as bar4 } from "//server/bar"; | ||
>bar : Symbol(bar4, Decl(a.ts, 7, 8)) | ||
>bar4 : Symbol(bar4, Decl(a.ts, 7, 8)) | ||
|
||
import { foo as foo5 } from "\\\\server\\foo"; | ||
>foo : Symbol(foo5, Decl(a.ts, 8, 8)) | ||
>foo5 : Symbol(foo5, Decl(a.ts, 8, 8)) | ||
|
||
import { bar as bar5 } from "\\\\server\\bar"; | ||
>bar : Symbol(bar5, Decl(a.ts, 9, 8)) | ||
>bar5 : Symbol(bar5, Decl(a.ts, 9, 8)) | ||
|
||
import { foo as foo6 } from "file:///foo"; | ||
>foo : Symbol(foo6, Decl(a.ts, 10, 8)) | ||
>foo6 : Symbol(foo6, Decl(a.ts, 10, 8)) | ||
|
||
import { bar as bar6 } from "file:///bar"; | ||
>bar : Symbol(bar6, Decl(a.ts, 11, 8)) | ||
>bar6 : Symbol(bar6, Decl(a.ts, 11, 8)) | ||
|
||
import { foo as foo7 } from "file://c:/foo"; | ||
>foo : Symbol(foo7, Decl(a.ts, 12, 8)) | ||
>foo7 : Symbol(foo7, Decl(a.ts, 12, 8)) | ||
|
||
import { bar as bar7 } from "file://c:/bar"; | ||
>bar : Symbol(bar7, Decl(a.ts, 13, 8)) | ||
>bar7 : Symbol(bar7, Decl(a.ts, 13, 8)) | ||
|
||
import { foo as foo8 } from "file://server/foo"; | ||
>foo : Symbol(foo8, Decl(a.ts, 14, 8)) | ||
>foo8 : Symbol(foo8, Decl(a.ts, 14, 8)) | ||
|
||
import { bar as bar8 } from "file://server/bar"; | ||
>bar : Symbol(bar8, Decl(a.ts, 15, 8)) | ||
>bar8 : Symbol(bar8, Decl(a.ts, 15, 8)) | ||
|
||
import { foo as foo9 } from "http://server/foo"; | ||
>foo : Symbol(foo9, Decl(a.ts, 16, 8)) | ||
>foo9 : Symbol(foo9, Decl(a.ts, 16, 8)) | ||
|
||
import { bar as bar9 } from "http://server/bar"; | ||
>bar : Symbol(bar9, Decl(a.ts, 17, 8)) | ||
>bar9 : Symbol(bar9, Decl(a.ts, 17, 8)) | ||
|
||
=== /root/src/foo.ts === | ||
export function foo() {} | ||
>foo : Symbol(foo, Decl(foo.ts, 0, 0)) | ||
|
||
=== /root/src/bar.js === | ||
export function bar() {} | ||
>bar : Symbol(bar, Decl(bar.js, 0, 0)) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also needs some tests to ensure that caching is working as intended (and hasn't changed)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you point me in the right direction? I am unsure what would be needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are tests in
src/testRunner/moduleResolution.ts
that verify caching is working as expected.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sheetalkamat Can you please explain how I can run the unit tests? Thank you
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sheetalkamat I figured it out, they are run by default