Skip to content

Commit

Permalink
Merge pull request #9459 from Microsoft/dontEmitNodeModules-2.0
Browse files Browse the repository at this point in the history
Port #9421 to not compile files under from node_modules
  • Loading branch information
billti authored Jul 1, 2016
2 parents bd58cf0 + 2dc84a8 commit f7c9a77
Show file tree
Hide file tree
Showing 15 changed files with 71 additions and 33 deletions.
37 changes: 22 additions & 15 deletions src/compiler/program.ts
Original file line number Diff line number Diff line change
Expand Up @@ -779,13 +779,18 @@ namespace ts {
while (true) {
const baseName = getBaseFileName(directory);
if (baseName !== "node_modules") {
const result =
// first: try to load module as-is
loadModuleFromNodeModulesFolder(moduleName, directory, failedLookupLocations, state) ||
// second: try to load module from the scope '@types'
loadModuleFromNodeModulesFolder(combinePaths("@types", moduleName), directory, failedLookupLocations, state);
if (result) {
return result;
// Try to load source from the package
const packageResult = loadModuleFromNodeModulesFolder(moduleName, directory, failedLookupLocations, state);
if (packageResult && hasTypeScriptFileExtension(packageResult)) {
// Always prefer a TypeScript (.ts, .tsx, .d.ts) file shipped with the package
return packageResult;
}
else {
// Else prefer a types package over non-TypeScript results (e.g. JavaScript files)
const typesResult = loadModuleFromNodeModulesFolder(combinePaths("@types", moduleName), directory, failedLookupLocations, state);
if (typesResult || packageResult) {
return typesResult || packageResult;
}
}
}

Expand Down Expand Up @@ -1097,7 +1102,7 @@ namespace ts {
const modulesWithElidedImports: Map<boolean> = {};

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

const start = new Date().getTime();

Expand Down Expand Up @@ -1378,7 +1383,7 @@ namespace ts {
getSourceFile: program.getSourceFile,
getSourceFileByPath: program.getSourceFileByPath,
getSourceFiles: program.getSourceFiles,
getFilesFromNodeModules: () => jsFilesFoundSearchingNodeModules,
isSourceFileFromExternalLibrary: (file: SourceFile) => !!lookUp(sourceFilesFoundSearchingNodeModules, file.path),
writeFile: writeFileCallback || (
(fileName, data, writeByteOrderMark, onError, sourceFiles) => host.writeFile(fileName, data, writeByteOrderMark, onError, sourceFiles)),
isEmitBlocked,
Expand Down Expand Up @@ -2066,15 +2071,17 @@ namespace ts {
// - noResolve is falsy
// - module name comes from the list of imports
// - it's not a top level JavaScript module that exceeded the search max
const isJsFileUnderNodeModules = resolution && resolution.isExternalLibraryImport &&
hasJavaScriptFileExtension(resolution.resolvedFileName);
const isFromNodeModulesSearch = resolution && resolution.isExternalLibraryImport;
const isJsFileFromNodeModules = isFromNodeModulesSearch && hasJavaScriptFileExtension(resolution.resolvedFileName);

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

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

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

if (isJsFileUnderNodeModules) {
if (isJsFileFromNodeModules) {
currentNodeModulesJsDepth--;
}
}
Expand Down
8 changes: 3 additions & 5 deletions src/compiler/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ namespace ts {
getSourceFiles(): SourceFile[];

/* @internal */
getFilesFromNodeModules(): Map<boolean>;
isSourceFileFromExternalLibrary(file: SourceFile): boolean;

getCommonSourceDirectory(): string;
getCanonicalFileName(fileName: string): string;
Expand Down Expand Up @@ -2277,10 +2277,9 @@ namespace ts {
}
else {
const sourceFiles = targetSourceFile === undefined ? host.getSourceFiles() : [targetSourceFile];
const nodeModulesFiles = host.getFilesFromNodeModules();
for (const sourceFile of sourceFiles) {
// Don't emit if source file is a declaration file, or was located under node_modules
if (!isDeclarationFile(sourceFile) && !lookUp(nodeModulesFiles, sourceFile.path)) {
if (!isDeclarationFile(sourceFile) && !host.isSourceFileFromExternalLibrary(sourceFile)) {
onSingleFileEmit(host, sourceFile);
}
}
Expand Down Expand Up @@ -2314,10 +2313,9 @@ namespace ts {
function onBundledEmit(host: EmitHost) {
// Can emit only sources that are not declaration file and are either non module code or module with
// --module or --target es6 specified. Files included by searching under node_modules are also not emitted.
const nodeModulesFiles = host.getFilesFromNodeModules();
const bundledSources = filter(host.getSourceFiles(),
sourceFile => !isDeclarationFile(sourceFile) &&
!lookUp(nodeModulesFiles, sourceFile.path) &&
!host.isSourceFileFromExternalLibrary(sourceFile) &&
(!isExternalModule(sourceFile) ||
!!getEmitModuleKind(options)));
if (bundledSources.length) {
Expand Down
2 changes: 0 additions & 2 deletions tests/baselines/reference/moduleAugmentationInDependency2.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@ export {};
//// [app.ts]
import "A"

//// [index.js]
"use strict";
//// [app.js]
"use strict";
require("A");
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,6 @@ 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
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
define(["require", "exports", "m1"], function (require, exports, m1) {
define(["require", "exports", "m1", "m4"], function (require, exports, m1, m4) {
"use strict";
m1.f1("test");
m1.f2.a = 10;
m1.f2.person.age = "10"; // Error: Should be number
m1.f2.person.age = "10"; // Should error if loaded the .js files correctly
var r2 = 3 + m4.foo; // Should be OK if correctly using the @types .d.ts file
});
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
maxDepthIncreased/root.ts(4,1): error TS2322: Type 'string' is not assignable to type 'number'.
maxDepthIncreased/root.ts(7,1): error TS2322: Type 'string' is not assignable to type 'number'.


==== index.js (0 errors) ====
Expand Down Expand Up @@ -28,11 +28,19 @@ maxDepthIncreased/root.ts(4,1): error TS2322: Type 'string' is not assignable to

exports.f2 = m2;

==== entry.d.ts (0 errors) ====
export declare var foo: number;

==== maxDepthIncreased/root.ts (1 errors) ====
import * as m1 from "m1";
import * as m4 from "m4";

m1.f1("test");
m1.f2.a = 10;
m1.f2.person.age = "10"; // Error: Should be number

m1.f2.person.age = "10"; // Should error if loaded the .js files correctly
~~~~~~~~~~~~~~~~
!!! error TS2322: Type 'string' is not assignable to type 'number'.

let r2 = 3 + m4.foo; // Should be OK if correctly using the @types .d.ts file

Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
"maxDepthIncreased/node_modules/m2/node_modules/m3/index.js",
"maxDepthIncreased/node_modules/m2/entry.js",
"maxDepthIncreased/node_modules/m1/index.js",
"maxDepthIncreased/node_modules/@types/m4/entry.d.ts",
"maxDepthIncreased/root.ts"
],
"emittedFiles": [
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
"use strict";
var m1 = require("m1");
var m4 = require("m4");
m1.f1("test");
m1.f2.a = 10;
m1.f2.person.age = "10"; // Error: Should be number
m1.f2.person.age = "10"; // Should error if loaded the .js files correctly
var r2 = 3 + m4.foo; // Should be OK if correctly using the @types .d.ts file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
maxDepthIncreased/root.ts(4,1): error TS2322: Type 'string' is not assignable to type 'number'.
maxDepthIncreased/root.ts(7,1): error TS2322: Type 'string' is not assignable to type 'number'.


==== index.js (0 errors) ====
Expand Down Expand Up @@ -28,11 +28,19 @@ maxDepthIncreased/root.ts(4,1): error TS2322: Type 'string' is not assignable to

exports.f2 = m2;

==== entry.d.ts (0 errors) ====
export declare var foo: number;

==== maxDepthIncreased/root.ts (1 errors) ====
import * as m1 from "m1";
import * as m4 from "m4";

m1.f1("test");
m1.f2.a = 10;
m1.f2.person.age = "10"; // Error: Should be number

m1.f2.person.age = "10"; // Should error if loaded the .js files correctly
~~~~~~~~~~~~~~~~
!!! error TS2322: Type 'string' is not assignable to type 'number'.

let r2 = 3 + m4.foo; // Should be OK if correctly using the @types .d.ts file

Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
"maxDepthIncreased/node_modules/m2/node_modules/m3/index.js",
"maxDepthIncreased/node_modules/m2/entry.js",
"maxDepthIncreased/node_modules/m1/index.js",
"maxDepthIncreased/node_modules/@types/m4/entry.d.ts",
"maxDepthIncreased/root.ts"
],
"emittedFiles": [
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.

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,9 @@
import * as m1 from "m1";
import * as m4 from "m4";

m1.f1("test");
m1.f2.a = 10;
m1.f2.person.age = "10"; // Error: Should be number

m1.f2.person.age = "10"; // Should error if loaded the .js files correctly

let r2 = 3 + m4.foo; // Should be OK if correctly using the @types .d.ts file

0 comments on commit f7c9a77

Please sign in to comment.