Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Prev Previous commit
Next Next commit
Respond to PR comments
  • Loading branch information
Andy Hanson committed May 31, 2016
commit f4d6b67cb0973ed3366190e10f61c0cf3a4e4229
7 changes: 7 additions & 0 deletions src/compiler/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -861,6 +861,13 @@ namespace ts {
return fileExtensionIs(path, extension) ? path.substring(0, path.length - extension.length) : undefined;
}

export function getFileExtension(path: string): string {
Copy link
Member

@DanielRosenwasser DanielRosenwasser May 31, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider making the return type annotation string | undefined

const dot = path.lastIndexOf(".");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if the dot is somewhere else in the path, e.g. /foo/some.dir/filename ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Turns out we don't need this function, so removing it.

if (dot !== -1) {
return path.substring(dot);
}
}

export interface ObjectAllocator {
getNodeConstructor(): new (kind: SyntaxKind, pos?: number, end?: number) => Node;
getSourceFileConstructor(): new (kind: SyntaxKind, pos?: number, end?: number) => SourceFile;
Expand Down
4 changes: 4 additions & 0 deletions src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -2768,6 +2768,10 @@
"category": "Error",
"code": 6131
},
"File name '{0}' has a '{1}' extension - stripping it": {
"category": "Message",
"code": 6132
},

"Variable '{0}' implicitly has an '{1}' type.": {
"category": "Error",
Expand Down
34 changes: 17 additions & 17 deletions src/compiler/program.ts
Original file line number Diff line number Diff line change
Expand Up @@ -615,11 +615,26 @@ namespace ts {
}

/**
* @param extensions - Either supportedTypeScriptExtensions or if --allowJs, allSupportedExtensions
* @param {boolean} onlyRecordFailures - if true then function won't try to actually load files but instead record all attempts as failures. This flag is necessary
* in cases when we know upfront that all load attempts will fail (because containing folder does not exists) however we still need to record all failed lookup locations.
*/
function loadModuleFromFile(candidate: string, extensions: string[], failedLookupLocation: string[], onlyRecordFailures: boolean, state: ModuleResolutionState): string {
// First try to keep/add an extension: importing "./foo.ts" can be matched by a file "./foo.ts", and "./foo" by "./foo.d.ts"
const keepOrAddExtension = loadModuleFromFileWorker(candidate, extensions, failedLookupLocation, onlyRecordFailures, state);
if (keepOrAddExtension) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: It says keepOrAddExtension, but that name implies that this is a boolean used for making a decision rather than the module itself. Consider unstrippedResolvedName or something like that.

return keepOrAddExtension;
}
// Then try stripping a ".js" or ".jsx" extension and replacing it with a TypeScript one, e.g. "./foo.js" can be matched by "./foo.ts" or "./foo.d.ts"
if (hasJavaScriptFileExtension(candidate)) {
const extensionless = removeFileExtension(candidate);
if (state.traceEnabled) {
const extension = candidate.substring(extensionless.length);
trace(state.host, Diagnostics.File_name_0_has_a_1_extension_stripping_it, candidate, extension);
}
return loadModuleFromFileWorker(extensionless, extensions, failedLookupLocation, onlyRecordFailures, state);
}
}
function loadModuleFromFileWorker(candidate: string, extensions: string[], failedLookupLocation: string[], onlyRecordFailures: boolean, state: ModuleResolutionState): string {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Newline above.

if (!onlyRecordFailures) {
// check if containig folder exists - if it doesn't then just record failures for all supported extensions without disk probing
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

containing

const directory = getDirectoryPath(candidate);
Expand All @@ -628,27 +643,12 @@ namespace ts {
}
}

// First try to keep/add an extension: importing "./foo.ts" can be matched by a file "./foo.ts", and "./foo" by "./foo.d.ts"
const keepOrAddExtension = forEach(extensions, ext => {
return forEach(extensions, ext => {
if (state.skipTsx && (ext === ".jsx" || ext === ".tsx")) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there isn't one, it would be useful to have a function to determine if the file extension is explicitly a JSX extension.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean function isJsxLike(extension: string) { return extension === ".jsx" || extension === ".tsx"; }?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do not think you need the extra lambda. just revert to the old tryLoad implementation.

return;
}
return tryLoad(fileExtensionIs(candidate, ext) ? candidate : candidate + ext);
});
if (keepOrAddExtension) {
return keepOrAddExtension;
}

// Then try stripping a ".js" or ".jsx" extension and replacing it with a TypeScript one, e.g. "./foo.js" can be matched by "./foo.ts" or "./foo.d.ts"
return forEach(supportedJavascriptExtensions, jsExt => {
if (state.skipTsx && jsExt === ".jsx") {
return;
}
const extensionless = tryRemoveExtension(candidate, jsExt);
if (extensionless !== undefined) {
return forEach(supportedTypeScriptExtensions, tsExt => tryLoad(extensionless + tsExt));
}
});

function tryLoad(fileName: string): string {
if (!onlyRecordFailures && state.host.fileExists(fileName)) {
Expand Down
32 changes: 32 additions & 0 deletions tests/baselines/reference/moduleResolution.trace.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
[
"======== Resolving module './a' from 'C:/Users/anhans/TypeScript/tests/cases/compiler/b.ts'. ========",
"Module resolution kind is not specified, using 'NodeJs'.",
"Loading module as file / folder, candidate module location 'C:/Users/anhans/TypeScript/tests/cases/compiler/a'.",
"File 'C:/Users/anhans/TypeScript/tests/cases/compiler/a.ts' exist - use it as a name resolution result.",
"======== Module name './a' was successfully resolved to 'C:/Users/anhans/TypeScript/tests/cases/compiler/a.ts'. ========",
"======== Resolving module './a.ts' from 'C:/Users/anhans/TypeScript/tests/cases/compiler/c.ts'. ========",
"Module resolution kind is not specified, using 'NodeJs'.",
"Loading module as file / folder, candidate module location 'C:/Users/anhans/TypeScript/tests/cases/compiler/a.ts'.",
"File 'C:/Users/anhans/TypeScript/tests/cases/compiler/a.ts' exist - use it as a name resolution result.",
"======== Module name './a.ts' was successfully resolved to 'C:/Users/anhans/TypeScript/tests/cases/compiler/a.ts'. ========",
"======== Resolving module './a.js' from 'C:/Users/anhans/TypeScript/tests/cases/compiler/d.ts'. ========",
"Module resolution kind is not specified, using 'NodeJs'.",
"Loading module as file / folder, candidate module location 'C:/Users/anhans/TypeScript/tests/cases/compiler/a.js'.",
"File 'C:/Users/anhans/TypeScript/tests/cases/compiler/a.js.ts' does not exist.",
"File 'C:/Users/anhans/TypeScript/tests/cases/compiler/a.js.tsx' does not exist.",
"File 'C:/Users/anhans/TypeScript/tests/cases/compiler/a.js.d.ts' does not exist.",
"File name 'C:/Users/anhans/TypeScript/tests/cases/compiler/a.js' has a '.js' extension - stripping it",
"File 'C:/Users/anhans/TypeScript/tests/cases/compiler/a.ts' exist - use it as a name resolution result.",
"======== Module name './a.js' was successfully resolved to 'C:/Users/anhans/TypeScript/tests/cases/compiler/a.ts'. ========",
"======== Resolving module './jquery.js' from 'C:/Users/anhans/TypeScript/tests/cases/compiler/jquery_user_1.ts'. ========",
"Module resolution kind is not specified, using 'NodeJs'.",
"Loading module as file / folder, candidate module location 'C:/Users/anhans/TypeScript/tests/cases/compiler/jquery.js'.",
"File 'C:/Users/anhans/TypeScript/tests/cases/compiler/jquery.js.ts' does not exist.",
"File 'C:/Users/anhans/TypeScript/tests/cases/compiler/jquery.js.tsx' does not exist.",
"File 'C:/Users/anhans/TypeScript/tests/cases/compiler/jquery.js.d.ts' does not exist.",
"File name 'C:/Users/anhans/TypeScript/tests/cases/compiler/jquery.js' has a '.js' extension - stripping it",
"File 'C:/Users/anhans/TypeScript/tests/cases/compiler/jquery.ts' does not exist.",
"File 'C:/Users/anhans/TypeScript/tests/cases/compiler/jquery.tsx' does not exist.",
"File 'C:/Users/anhans/TypeScript/tests/cases/compiler/jquery.d.ts' exist - use it as a name resolution result.",
"======== Module name './jquery.js' was successfully resolved to 'C:/Users/anhans/TypeScript/tests/cases/compiler/jquery.d.ts'. ========"
]
1 change: 1 addition & 0 deletions tests/cases/compiler/moduleResolution.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// @traceResolution: true
// @Filename: a.ts
export default 0;

Expand Down