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

Fix #9542 (allow files under node_modules to be included in the compilation). #9607

Merged
merged 7 commits into from
Jul 11, 2016
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
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -51,3 +51,4 @@ internal/
!**/.vscode/tasks.json
!tests/cases/projects/projectOption/**/node_modules
!tests/cases/projects/NodeModulesSearch/**/*
!tests/baselines/reference/project/nodeModules*/**/*
32 changes: 21 additions & 11 deletions src/compiler/program.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1095,13 +1095,13 @@ namespace ts {
// As all these operations happen - and are nested - within the createProgram call, they close over the below variables.
// The current resolution depth is tracked by incrementing/decrementing as the depth first search progresses.
const maxNodeModulesJsDepth = typeof options.maxNodeModuleJsDepth === "number" ? options.maxNodeModuleJsDepth : 2;
let currentNodeModulesJsDepth = 0;
let currentNodeModulesDepth = 0;

// If a module has some of its imports skipped due to being at the depth limit under node_modules, then track
// this, as it may be imported at a shallower depth later, and then it will need its skipped imports processed.
const modulesWithElidedImports: Map<boolean> = {};

// Track source files that are JavaScript files found by searching under node_modules, as these shouldn't be compiled.
// Track source files that are source files found by searching under node_modules, as these shouldn't be compiled.
const sourceFilesFoundSearchingNodeModules: Map<boolean> = {};

const start = new Date().getTime();
Expand Down Expand Up @@ -1918,9 +1918,21 @@ namespace ts {
reportFileNamesDifferOnlyInCasingError(fileName, file.fileName, refFile, refPos, refEnd);
}

// If the file was previously found via a node_modules search, but is now being processed as a root file,
// then everything it sucks in may also be marked incorrectly, and needs to be checked again.
if (file && lookUp(sourceFilesFoundSearchingNodeModules, file.path) && currentNodeModulesDepth == 0) {
sourceFilesFoundSearchingNodeModules[file.path] = false;
if (!options.noResolve) {
processReferencedFiles(file, getDirectoryPath(fileName), isDefaultLib);
processTypeReferenceDirectives(file);
}

modulesWithElidedImports[file.path] = false;
processImportedModules(file, getDirectoryPath(fileName));
}
// See if we need to reprocess the imports due to prior skipped imports
if (file && lookUp(modulesWithElidedImports, file.path)) {
if (currentNodeModulesJsDepth < maxNodeModulesJsDepth) {
else if (file && lookUp(modulesWithElidedImports, file.path)) {
if (currentNodeModulesDepth < maxNodeModulesJsDepth) {
modulesWithElidedImports[file.path] = false;
processImportedModules(file, getDirectoryPath(fileName));
}
Expand All @@ -1942,6 +1954,7 @@ namespace ts {

filesByName.set(path, file);
if (file) {
sourceFilesFoundSearchingNodeModules[path] = (currentNodeModulesDepth > 0);
file.path = path;

if (host.useCaseSensitiveFileNames()) {
Expand Down Expand Up @@ -2075,13 +2088,10 @@ namespace ts {
const isJsFileFromNodeModules = isFromNodeModulesSearch && hasJavaScriptFileExtension(resolution.resolvedFileName);

if (isFromNodeModulesSearch) {
sourceFilesFoundSearchingNodeModules[resolvedPath] = true;
}
if (isJsFileFromNodeModules) {
currentNodeModulesJsDepth++;
currentNodeModulesDepth++;
}

const elideImport = isJsFileFromNodeModules && currentNodeModulesJsDepth > maxNodeModulesJsDepth;
const elideImport = isJsFileFromNodeModules && currentNodeModulesDepth > maxNodeModulesJsDepth;
const shouldAddFile = resolution && !options.noResolve && i < file.imports.length && !elideImport;

if (elideImport) {
Expand All @@ -2096,8 +2106,8 @@ namespace ts {
file.imports[i].end);
}

if (isJsFileFromNodeModules) {
currentNodeModulesJsDepth--;
if (isFromNodeModulesSearch) {
currentNodeModulesDepth--;
}
}
}
Expand Down
28 changes: 19 additions & 9 deletions src/harness/projectsRunner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -479,17 +479,27 @@ class ProjectRunner extends RunnerBase {

it("Baseline of emitted result (" + moduleNameToString(moduleKind) + "): " + testCaseFileName, () => {
if (testCase.baselineCheck) {
const errs: Error[] = [];
ts.forEach(compilerResult.outputFiles, outputFile => {

Harness.Baseline.runBaseline("Baseline of emitted result (" + moduleNameToString(compilerResult.moduleKind) + "): " + testCaseFileName, getBaselineFolder(compilerResult.moduleKind) + outputFile.fileName, () => {
try {
return Harness.IO.readFile(getProjectOutputFolder(outputFile.fileName, compilerResult.moduleKind));
}
catch (e) {
return undefined;
}
});
// There may be multiple files with different baselines. Run all and report at the end, else
// it stops copying the remaining emitted files from 'local/projectOutput' to 'local/project'.
try {
Copy link
Contributor

Choose a reason for hiding this comment

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

sorry about that. i had this fixed a while back in the transforms branch, but never ported it back.

Harness.Baseline.runBaseline("Baseline of emitted result (" + moduleNameToString(compilerResult.moduleKind) + "): " + testCaseFileName, getBaselineFolder(compilerResult.moduleKind) + outputFile.fileName, () => {
try {
return Harness.IO.readFile(getProjectOutputFolder(outputFile.fileName, compilerResult.moduleKind));
}
catch (e) {
return undefined;
}
});
}
catch (e) {
errs.push(e);
}
});
if (errs.length) {
throw Error(errs.join("\n "));
}
}
});

Expand Down
2 changes: 2 additions & 0 deletions tests/baselines/reference/moduleAugmentationInDependency2.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ export {};
//// [app.ts]
import "A"

//// [index.js]
"use strict";
//// [app.js]
"use strict";
require("A");
2 changes: 0 additions & 2 deletions tests/baselines/reference/nodeResolution6.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,5 @@ export declare var y;
import y = require("a");


//// [ref.js]
var x = 1;
//// [b.js]
"use strict";
2 changes: 0 additions & 2 deletions tests/baselines/reference/nodeResolution8.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,5 @@ export declare var y;
//// [b.ts]
import y = require("a");

//// [ref.js]
var x = 1;
//// [b.js]
"use strict";
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ exports.x = 1;
//// [file2.js]
"use strict";
exports.y = 1;
//// [file4.js]
"use strict";
exports.z1 = 1;
//// [file1.js]
"use strict";
var file1_1 = require("folder2/file1");
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,6 @@ define(["require", "exports", "m1"], function (require, exports, m1) {
"use strict";
m1.f1("test");
m1.f2.a = "10"; // Error: Should be number
m1.rel = 42; // Error: Should be boolean
m1.f2.person.age = "10"; // OK if stopped at 2 modules: person will be "any".
});
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
maxDepthExceeded/root.ts(3,1): error TS2322: Type 'string' is not assignable to type 'number'.
maxDepthExceeded/root.ts(4,1): error TS2322: Type 'number' is not assignable to type 'boolean'.


==== entry.js (0 errors) ====
Expand All @@ -10,8 +11,12 @@ maxDepthExceeded/root.ts(3,1): error TS2322: Type 'string' is not assignable to
"person": m3.person
};

==== index.js (0 errors) ====
==== relative.js (0 errors) ====
exports.relativeProp = true;

==== maxDepthExceeded/node_modules/m1/index.js (0 errors) ====
var m2 = require('m2');
var rel = require('./relative');

/**
* @param {string} p1 The first param
Expand All @@ -22,11 +27,17 @@ maxDepthExceeded/root.ts(3,1): error TS2322: Type 'string' is not assignable to

exports.f2 = m2;

==== maxDepthExceeded/root.ts (1 errors) ====
exports.rel = rel.relativeProp;

==== maxDepthExceeded/root.ts (2 errors) ====
import * as m1 from "m1";
m1.f1("test");
m1.f2.a = "10"; // Error: Should be number
~~~~~~~
!!! error TS2322: Type 'string' is not assignable to type 'number'.
m1.rel = 42; // Error: Should be boolean
~~~~~~
!!! error TS2322: Type 'number' is not assignable to type 'boolean'.

m1.f2.person.age = "10"; // OK if stopped at 2 modules: person will be "any".

Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,13 @@
"resolvedInputFiles": [
"lib.d.ts",
"maxDepthExceeded/node_modules/m2/entry.js",
"maxDepthExceeded/node_modules/m1/relative.js",
"maxDepthExceeded/node_modules/m1/index.js",
"maxDepthExceeded/root.ts"
],
"emittedFiles": [
"maxDepthExceeded/root.js"
"maxDepthExceeded/built/node_modules/m1/relative.js",
"maxDepthExceeded/built/node_modules/m1/index.js",
"maxDepthExceeded/built/root.js"
]
}

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,5 @@
var m1 = require("m1");
m1.f1("test");
m1.f2.a = "10"; // Error: Should be number
m1.rel = 42; // Error: Should be boolean
m1.f2.person.age = "10"; // OK if stopped at 2 modules: person will be "any".
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
maxDepthExceeded/root.ts(3,1): error TS2322: Type 'string' is not assignable to type 'number'.
maxDepthExceeded/root.ts(4,1): error TS2322: Type 'number' is not assignable to type 'boolean'.


==== entry.js (0 errors) ====
Expand All @@ -10,8 +11,12 @@ maxDepthExceeded/root.ts(3,1): error TS2322: Type 'string' is not assignable to
"person": m3.person
};

==== index.js (0 errors) ====
==== relative.js (0 errors) ====
exports.relativeProp = true;

==== maxDepthExceeded/node_modules/m1/index.js (0 errors) ====
var m2 = require('m2');
var rel = require('./relative');

/**
* @param {string} p1 The first param
Expand All @@ -22,11 +27,17 @@ maxDepthExceeded/root.ts(3,1): error TS2322: Type 'string' is not assignable to

exports.f2 = m2;

==== maxDepthExceeded/root.ts (1 errors) ====
exports.rel = rel.relativeProp;

==== maxDepthExceeded/root.ts (2 errors) ====
import * as m1 from "m1";
m1.f1("test");
m1.f2.a = "10"; // Error: Should be number
~~~~~~~
!!! error TS2322: Type 'string' is not assignable to type 'number'.
m1.rel = 42; // Error: Should be boolean
~~~~~~
!!! error TS2322: Type 'number' is not assignable to type 'boolean'.

m1.f2.person.age = "10"; // OK if stopped at 2 modules: person will be "any".

Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,13 @@
"resolvedInputFiles": [
"lib.d.ts",
"maxDepthExceeded/node_modules/m2/entry.js",
"maxDepthExceeded/node_modules/m1/relative.js",
"maxDepthExceeded/node_modules/m1/index.js",
"maxDepthExceeded/root.ts"
],
"emittedFiles": [
"maxDepthExceeded/root.js"
"maxDepthExceeded/built/node_modules/m1/relative.js",
"maxDepthExceeded/built/node_modules/m1/index.js",
"maxDepthExceeded/built/root.js"
]
}

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import * as m1 from "m1";
m1.f1("test");
m1.f2.a = "10"; // Error: Should be number
m1.rel = 42; // Error: Should be boolean

m1.f2.person.age = "10"; // OK if stopped at 2 modules: person will be "any".
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
{
"compilerOptions": {
"allowJs": true
}
"allowJs": true,
"maxNodeModuleJsDepth": 1, // Note: Module m1 is already included as a root file
"outDir": "built"
},
"include": ["**/*"],
"exclude": ["node_modules/m2/**/*"]
}