Skip to content
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

Merged
merged 1 commit into from
Nov 8, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 15 additions & 10 deletions src/compiler/moduleNameResolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Copy link
Member

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)

Copy link
Contributor Author

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.

Copy link
Member

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.

Copy link
Contributor Author

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

Copy link
Contributor Author

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

return tryLoadModuleUsingBaseUrl(extensions, moduleName, loader, state);
}
Expand All @@ -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 {

Expand Down Expand Up @@ -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) {
Copy link
Member

Choose a reason for hiding this comment

The 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.
We also need trace above this line about using baseUrl in that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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.

I thought about that. I however couldn't think of a name. The name tryLoadModuleUsingPaths is already taken. What do you suggest?

We also need trace above this line about using baseUrl in that.

The trace baseUrl_option_is_set_to_0_using_this_value_to_resolve_non_relative_module_name_1 is not exactly correct for what is happening. And since we have the trace paths_option_is_specified_looking_for_a_pattern_to_match_module_name_0 I thought that would be enough. But if you really want it there I could of course add it back in.

Copy link
Member

Choose a reason for hiding this comment

The 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);
Expand Down
27 changes: 14 additions & 13 deletions src/services/stringCompletions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -247,19 +247,9 @@ namespace ts.Completions.StringCompletions {
const scriptPath = sourceFile.path;
const scriptDirectory = getDirectoryPath(scriptPath);

const extensionOptions = getExtensionOptions(compilerOptions);
if (isPathRelativeToScript(literalValue) || isRootedDiskPath(literalValue)) {
if (compilerOptions.rootDirs) {
return getCompletionEntriesForDirectoryFragmentWithRootDirs(
compilerOptions.rootDirs, literalValue, scriptDirectory, extensionOptions, compilerOptions, host, scriptPath);
}
else {
return getCompletionEntriesForDirectoryFragment(literalValue, scriptDirectory, extensionOptions, host, scriptPath);
}
}
else {
return getCompletionEntriesForNonRelativeModules(literalValue, scriptDirectory, compilerOptions, host, typeChecker);
}
return isPathRelativeToScript(literalValue) || !compilerOptions.baseUrl && (isRootedDiskPath(literalValue) || isUrl(literalValue))
? getCompletionEntriesForRelativeModules(literalValue, scriptDirectory, compilerOptions, host, scriptPath)
: getCompletionEntriesForNonRelativeModules(literalValue, scriptDirectory, compilerOptions, host, typeChecker);
}

interface ExtensionOptions {
Expand All @@ -269,6 +259,17 @@ namespace ts.Completions.StringCompletions {
function getExtensionOptions(compilerOptions: CompilerOptions, includeExtensions = false): ExtensionOptions {
return { extensions: getSupportedExtensionsForModuleResolution(compilerOptions), includeExtensions };
}
function getCompletionEntriesForRelativeModules(literalValue: string, scriptDirectory: string, compilerOptions: CompilerOptions, host: LanguageServiceHost, scriptPath: Path) {
const extensionOptions = getExtensionOptions(compilerOptions);
if (compilerOptions.rootDirs) {
return getCompletionEntriesForDirectoryFragmentWithRootDirs(
compilerOptions.rootDirs, literalValue, scriptDirectory, extensionOptions, compilerOptions, host, scriptPath);
}
else {
return getCompletionEntriesForDirectoryFragment(literalValue, scriptDirectory, extensionOptions, host, scriptPath);
}
}

function getSupportedExtensionsForModuleResolution(compilerOptions: CompilerOptions): ReadonlyArray<Extension> {
const extensions = getSupportedExtensions(compilerOptions);
return compilerOptions.resolveJsonModule && getEmitModuleResolutionKind(compilerOptions) === ModuleResolutionKind.NodeJs ?
Expand Down
8 changes: 8 additions & 0 deletions src/testRunner/unittests/moduleResolution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -757,6 +757,9 @@ import b = require("./moduleB");
],
"somefolder/*": [
"someanotherfolder/*"
],
"/rooted/*": [
"generated/*"
]
}
};
Expand All @@ -773,6 +776,7 @@ import b = require("./moduleB");
"/root/folder1/file2/index.d.ts",
// then first attempt on 'generated/*' was successful
]);
check("/rooted/folder1/file2", file2, []);
check("folder2/file3", file3, [
// first try '*'
"/root/folder2/file3.ts",
Expand Down Expand Up @@ -900,6 +904,9 @@ import b = require("./moduleB");
],
"somefolder/*": [
"someanotherfolder/*"
],
"/rooted/*": [
"generated/*"
]
}
};
Expand All @@ -911,6 +918,7 @@ import b = require("./moduleB");
"/root/folder1/file2.d.ts",
// success when using 'generated/*'
]);
check("/rooted/folder1/file2", file2, []);
check("folder1/file3", file3, [
// first try '*'
"/root/folder1/file3.ts",
Expand Down
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))

Loading