Skip to content

Refactor module resolution logic to have configurable goal extensions #9430

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.

Sign up for GitHub

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

Closed
weswigham wants to merge 9 commits into from
Closed

Refactor module resolution logic to have configurable goal extensions #9430

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
ed46d89
Alter lookup logic to look for js after exhausting @types
weswigham Jun 29, 2016
5a3c695
Add projects test with an @types and real module
weswigham Jun 29, 2016
5e40c8e
Add missing files from node_modules folder
weswigham Jun 29, 2016
7894136
Add another test
weswigham Jun 29, 2016
fa9bfc8
Pass through a resolution goal enum rather than extensions
weswigham Jun 29, 2016
6a257a4
Accept new baselines
weswigham Jun 29, 2016
ef102da
Add trace baselines to project tests
weswigham Jun 30, 2016
903ad65
Rename ResolutionGoal and merge with ExtensionPriority
weswigham Jun 30, 2016
b1b31d2
Merge branch 'master' into fix-lookup-priority
weswigham Jul 5, 2016
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
Add trace baselines to project tests
  • Loading branch information
@weswigham
weswigham committed Jun 30, 2016
commit ef102daa9bdaa46fd7a37717f287fb17af8d3d2b
54 changes: 37 additions & 17 deletions src/compiler/core.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1135,28 +1135,48 @@ namespace ts {
export const supportedTypeScriptExtensions = [".ts", ".tsx", ".d.ts"];
export const supportedJavascriptExtensions = [".js", ".jsx"];

export function getExtensionsForGoal(goal: ResolutionGoal): string[] {
const extensions: string[] = [];
if (goal & ResolutionGoal.TS) {
extensions.push(".ts");
}
if (goal & ResolutionGoal.TSX) {
extensions.push(".tsx");
}
if (goal & ResolutionGoal.DTS) {
extensions.push(".d.ts");
}
if (goal & ResolutionGoal.JS) {
extensions.push(".js");
export const extensionMap = {
[ResolutionGoal.TS]: ".ts",
[ResolutionGoal.TSX]: ".tsx",
[ResolutionGoal.DTS]: ".d.ts",
[ResolutionGoal.JS]: ".js",
[ResolutionGoal.JSX]: ".jsx",
};

export const reverseExtensionMap = {
".ts": ResolutionGoal.TS,
".tsx": ResolutionGoal.TSX,
".d.ts": ResolutionGoal.DTS,
".js": ResolutionGoal.JS,
".jsx": ResolutionGoal.JSX,
};

const extensionPowerset: { [index: number]: string[] } = {
[ResolutionGoal.None]: []
};

function createPowerset(base: ResolutionGoal, powerset: { [index: number]: string[] }): void {
const newBases: number[] = [];
for (const key in extensionMap) {
const newKey = base | parseInt(key);
if (!powerset[newKey]) {
newBases.push(newKey);
powerset[newKey] = concatenate(powerset[base], [extensionMap[key]]);
}
}
if (goal & ResolutionGoal.JSX) {
extensions.push(".jsx");
for (const newBase of newBases) {
createPowerset(newBase, powerset);
}
return extensions;
}

createPowerset(ResolutionGoal.None, extensionPowerset);

export function getExtensionsForGoal(goal: ResolutionGoal): string[] {
return extensionPowerset[goal];
}

export function getResolutionGoalForCompilerOptions(options?: CompilerOptions) {
return options && options.allowJs ? ResolutionGoal.Any : ResolutionGoal.TypeScript;
return (options && options.allowJs) ? ResolutionGoal.Any : ResolutionGoal.TypeScript;
}

export function getSupportedExtensions(options?: CompilerOptions): string[] {
Expand Down
116 changes: 56 additions & 60 deletions src/compiler/program.ts
View file
Open in desktop

Large diffs are not rendered by default.

15 changes: 15 additions & 0 deletions src/compiler/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2964,4 +2964,19 @@ namespace ts {
export interface SyntaxList extends Node {
_children: Node[];
}

/* @internal */
export const enum ResolutionGoal {
None = 0,
TS = 1 << 0,
TSX = 1 << 1,
DTS = 1 << 2,
JS = 1 << 3,
JSX = 1 << 4,

JSXLike = JSX | TSX,
TypeScript = TS | TSX | DTS,
JavaScript = JS | JSX,
Any = TypeScript | JavaScript,
}
}
21 changes: 19 additions & 2 deletions src/harness/projectsRunner.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ interface CompileProjectFilesResult {
compilerOptions?: ts.CompilerOptions;
errors: ts.Diagnostic[];
sourceMapData?: ts.SourceMapData[];
traces?: string[];
}

interface BatchCompileProjectTestCaseResult extends CompileProjectFilesResult {
Expand Down Expand Up @@ -130,6 +131,7 @@ class ProjectRunner extends RunnerBase {
writeFile: (fileName: string, data: string, writeByteOrderMark: boolean) => void,
compilerOptions: ts.CompilerOptions): CompileProjectFilesResult {

const traces: string[] = [];
const program = ts.createProgram(getInputFiles(), compilerOptions, createCompilerHost());
let errors = ts.getPreEmitDiagnostics(program);

Expand All @@ -152,7 +154,8 @@ class ProjectRunner extends RunnerBase {
moduleKind,
program,
errors,
sourceMapData
sourceMapData,
traces
};

function getSourceFileText(fileName: string): string {
Expand All @@ -175,6 +178,10 @@ class ProjectRunner extends RunnerBase {
return sourceFile;
}

function trace(s: string) {
traces.push(s);
}

function createCompilerHost(): ts.CompilerHost {
return {
getSourceFile,
Expand All @@ -186,7 +193,8 @@ class ProjectRunner extends RunnerBase {
getNewLine: () => Harness.IO.newLine(),
fileExists: fileName => fileName === Harness.Compiler.defaultLibFileName || getSourceFileText(fileName) !== undefined,
readFile: fileName => Harness.IO.readFile(fileName),
getDirectories: path => Harness.IO.getDirectories(path)
getDirectories: path => Harness.IO.getDirectories(path),
trace,
};
}
}
Expand Down Expand Up @@ -242,6 +250,7 @@ class ProjectRunner extends RunnerBase {
sourceMapData: projectCompilerResult.sourceMapData,
outputFiles,
errors: projectCompilerResult.errors,
traces: projectCompilerResult.traces
};

function createCompilerOptions() {
Expand Down Expand Up @@ -493,6 +502,14 @@ class ProjectRunner extends RunnerBase {
}
});

it("Trace of result (" + moduleNameToString(moduleKind) + "): " + testCaseFileName, () => {
if (testCase.traceResolution) {
Harness.Baseline.runBaseline("Baseline of traces (" + moduleNameToString(compilerResult.moduleKind) + "): " + testCaseFileName, getBaselineFolder(compilerResult.moduleKind) + testCaseJustName + ".trace.txt", () => {
return ts.map((compilerResult.traces || []), (s) => s.replace(/\'([^']*?(\/|\\)tests(\/|\\)cases(\/|\\)projects(\/|\\))/, "'")).join("\n");
});
}
});


it("SourceMapRecord for (" + moduleNameToString(moduleKind) + "): " + testCaseFileName, () => {
if (compilerResult.sourceMapData) {
Expand Down
2 changes: 2 additions & 0 deletions tests/baselines/reference/project/realisticNpmTypes/amd/realisticNpmTypes.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
],
"baselineCheck": true,
"moduleResolution": "node",
"allowJs": true,
"traceResolution": true,
"resolvedInputFiles": [
"lib.d.ts",
"node_modules/@types/m1/index.d.ts",
Copy link
Copy Markdown
Member

@DanielRosenwasser DanielRosenwasser Jun 29, 2016

Choose a reason for hiding this comment

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

So this file doesn't exist, but it's referenced?

Copy link
Copy Markdown
Member Author

@weswigham weswigham Jun 29, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Hah. It didn't get committed because it's in a node_modules folder. That's funny. And annoying. Let me fix that.

Copy link
Copy Markdown
Member

@billti billti Jun 29, 2016

Choose a reason for hiding this comment

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

Yeah, see the last line I added to the gitignore file on my prior merge to master. That nearly got me too.

Expand Down
18 changes: 18 additions & 0 deletions tests/baselines/reference/project/realisticNpmTypes/amd/realisticNpmTypes.trace.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
======== Resolving module 'm1' from 'realisticNpmTypes/index.ts'. ========
Explicitly specified module resolution kind: 'NodeJs'.
Loading module 'm1' from 'node_modules' folder.
File 'realisticNpmTypes/node_modules/m1.ts' does not exist.
File 'realisticNpmTypes/node_modules/m1.tsx' does not exist.
File 'realisticNpmTypes/node_modules/m1.d.ts' does not exist.
File 'realisticNpmTypes/node_modules/m1/package.json' does not exist.
File 'realisticNpmTypes/node_modules/m1/index.ts' does not exist.
File 'realisticNpmTypes/node_modules/m1/index.tsx' does not exist.
File 'realisticNpmTypes/node_modules/m1/index.d.ts' does not exist.
File 'realisticNpmTypes/node_modules/@types/m1.ts' does not exist.
File 'realisticNpmTypes/node_modules/@types/m1.tsx' does not exist.
File 'realisticNpmTypes/node_modules/@types/m1.d.ts' does not exist.
File 'realisticNpmTypes/node_modules/@types/m1/package.json' does not exist.
File 'realisticNpmTypes/node_modules/@types/m1/index.ts' does not exist.
File 'realisticNpmTypes/node_modules/@types/m1/index.tsx' does not exist.
File 'realisticNpmTypes/node_modules/@types/m1/index.d.ts' exist - use it as a name resolution result.
======== Module name 'm1' was successfully resolved to 'realisticNpmTypes/node_modules/@types/m1/index.d.ts'. ========
2 changes: 2 additions & 0 deletions tests/baselines/reference/project/realisticNpmTypes/node/realisticNpmTypes.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
],
"baselineCheck": true,
"moduleResolution": "node",
"allowJs": true,
"traceResolution": true,
"resolvedInputFiles": [
"lib.d.ts",
"node_modules/@types/m1/index.d.ts",
Expand Down
18 changes: 18 additions & 0 deletions tests/baselines/reference/project/realisticNpmTypes/node/realisticNpmTypes.trace.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
======== Resolving module 'm1' from 'realisticNpmTypes/index.ts'. ========
Explicitly specified module resolution kind: 'NodeJs'.
Loading module 'm1' from 'node_modules' folder.
File 'realisticNpmTypes/node_modules/m1.ts' does not exist.
File 'realisticNpmTypes/node_modules/m1.tsx' does not exist.
File 'realisticNpmTypes/node_modules/m1.d.ts' does not exist.
File 'realisticNpmTypes/node_modules/m1/package.json' does not exist.
File 'realisticNpmTypes/node_modules/m1/index.ts' does not exist.
File 'realisticNpmTypes/node_modules/m1/index.tsx' does not exist.
File 'realisticNpmTypes/node_modules/m1/index.d.ts' does not exist.
File 'realisticNpmTypes/node_modules/@types/m1.ts' does not exist.
File 'realisticNpmTypes/node_modules/@types/m1.tsx' does not exist.
File 'realisticNpmTypes/node_modules/@types/m1.d.ts' does not exist.
File 'realisticNpmTypes/node_modules/@types/m1/package.json' does not exist.
File 'realisticNpmTypes/node_modules/@types/m1/index.ts' does not exist.
File 'realisticNpmTypes/node_modules/@types/m1/index.tsx' does not exist.
File 'realisticNpmTypes/node_modules/@types/m1/index.d.ts' exist - use it as a name resolution result.
======== Module name 'm1' was successfully resolved to 'realisticNpmTypes/node_modules/@types/m1/index.d.ts'. ========
2 changes: 2 additions & 0 deletions tests/baselines/reference/project/realisticNpmTypes2/amd/realisticNpmTypes2.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
],
"baselineCheck": true,
"moduleResolution": "node",
"allowJs": true,
"traceResolution": true,
"resolvedInputFiles": [
"lib.d.ts",
"node_modules/@types/m1/m1.d.ts",
Expand Down
20 changes: 20 additions & 0 deletions tests/baselines/reference/project/realisticNpmTypes2/amd/realisticNpmTypes2.trace.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
======== Resolving module 'm1' from 'realisticNpmTypes2/index.ts'. ========
Explicitly specified module resolution kind: 'NodeJs'.
Loading module 'm1' from 'node_modules' folder.
File 'realisticNpmTypes2/node_modules/m1.ts' does not exist.
File 'realisticNpmTypes2/node_modules/m1.tsx' does not exist.
File 'realisticNpmTypes2/node_modules/m1.d.ts' does not exist.
Found 'package.json' at 'realisticNpmTypes2/node_modules/m1/package.json'.
Expected type of 'typings' field in 'package.json' to be 'string', got 'undefined'.
Expected type of 'types' field in 'package.json' to be 'string', got 'undefined'.
File 'realisticNpmTypes2/node_modules/m1/index.ts' does not exist.
File 'realisticNpmTypes2/node_modules/m1/index.tsx' does not exist.
File 'realisticNpmTypes2/node_modules/m1/index.d.ts' does not exist.
File 'realisticNpmTypes2/node_modules/@types/m1.ts' does not exist.
File 'realisticNpmTypes2/node_modules/@types/m1.tsx' does not exist.
File 'realisticNpmTypes2/node_modules/@types/m1.d.ts' does not exist.
Found 'package.json' at 'realisticNpmTypes2/node_modules/@types/m1/package.json'.
Expected type of 'typings' field in 'package.json' to be 'string', got 'undefined'.
'package.json' has 'types' field 'm1.d.ts' that references 'realisticNpmTypes2/node_modules/@types/m1/m1.d.ts'.
File 'realisticNpmTypes2/node_modules/@types/m1/m1.d.ts' exist - use it as a name resolution result.
======== Module name 'm1' was successfully resolved to 'realisticNpmTypes2/node_modules/@types/m1/m1.d.ts'. ========
2 changes: 2 additions & 0 deletions tests/baselines/reference/project/realisticNpmTypes2/node/realisticNpmTypes2.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
],
"baselineCheck": true,
"moduleResolution": "node",
"allowJs": true,
"traceResolution": true,
"resolvedInputFiles": [
"lib.d.ts",
"node_modules/@types/m1/m1.d.ts",
Expand Down
20 changes: 20 additions & 0 deletions tests/baselines/reference/project/realisticNpmTypes2/node/realisticNpmTypes2.trace.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
======== Resolving module 'm1' from 'realisticNpmTypes2/index.ts'. ========
Explicitly specified module resolution kind: 'NodeJs'.
Loading module 'm1' from 'node_modules' folder.
File 'realisticNpmTypes2/node_modules/m1.ts' does not exist.
File 'realisticNpmTypes2/node_modules/m1.tsx' does not exist.
File 'realisticNpmTypes2/node_modules/m1.d.ts' does not exist.
Found 'package.json' at 'realisticNpmTypes2/node_modules/m1/package.json'.
Expected type of 'typings' field in 'package.json' to be 'string', got 'undefined'.
Expected type of 'types' field in 'package.json' to be 'string', got 'undefined'.
File 'realisticNpmTypes2/node_modules/m1/index.ts' does not exist.
File 'realisticNpmTypes2/node_modules/m1/index.tsx' does not exist.
File 'realisticNpmTypes2/node_modules/m1/index.d.ts' does not exist.
File 'realisticNpmTypes2/node_modules/@types/m1.ts' does not exist.
File 'realisticNpmTypes2/node_modules/@types/m1.tsx' does not exist.
File 'realisticNpmTypes2/node_modules/@types/m1.d.ts' does not exist.
Found 'package.json' at 'realisticNpmTypes2/node_modules/@types/m1/package.json'.
Expected type of 'typings' field in 'package.json' to be 'string', got 'undefined'.
'package.json' has 'types' field 'm1.d.ts' that references 'realisticNpmTypes2/node_modules/@types/m1/m1.d.ts'.
File 'realisticNpmTypes2/node_modules/@types/m1/m1.d.ts' exist - use it as a name resolution result.
======== Module name 'm1' was successfully resolved to 'realisticNpmTypes2/node_modules/@types/m1/m1.d.ts'. ========
3 changes: 2 additions & 1 deletion tests/cases/project/realisticNpmTypes.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,6 @@
],
"baselineCheck": true,
"moduleResolution": "node",
"allowJs": true
"allowJs": true,
"traceResolution": true
}
Copy link
Copy Markdown
Member

@billti billti Jun 29, 2016

Choose a reason for hiding this comment

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

You are not specifying allowJs, that's why this is working even with the above bug, as you are never even checking for .js files.

Copy link
Copy Markdown
Member Author

@weswigham weswigham Jun 29, 2016

Choose a reason for hiding this comment

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

'doh. It's just added by default in so many places that I thought this may be one of them. Added and fixed.

3 changes: 2 additions & 1 deletion tests/cases/project/realisticNpmTypes2.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,6 @@
],
"baselineCheck": true,
"moduleResolution": "node",
"allowJs": true
"allowJs": true,
"traceResolution": true
}