Merge pull request #9421 from Microsoft/dontEmitNodeModules

Don't emit any source files found under node_modules

Author: billti

src/compiler/program.ts

@@ -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;
+                    }
                 }
             }
 
@@ -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();
 
@@ -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,
@@ -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) {
@@ -2089,7 +2096,7 @@ namespace ts {
                                 file.imports[i].end);
                     }
 
-                    if (isJsFileUnderNodeModules) {
+                    if (isJsFileFromNodeModules) {
                         currentNodeModulesJsDepth--;
                     }
                 }

src/compiler/utilities.ts

@@ -36,7 +36,7 @@ namespace ts {
         getSourceFiles(): SourceFile[];
 
         /* @internal */
-        getFilesFromNodeModules(): Map<boolean>;
+        isSourceFileFromExternalLibrary(file: SourceFile): boolean;
 
         getCommonSourceDirectory(): string;
         getCanonicalFileName(fileName: string): string;
@@ -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);
                 }
             }
@@ -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) {

tests/baselines/reference/moduleAugmentationInDependency2.js

@@ -8,8 +8,6 @@ export {};
 //// [app.ts]
 import "A"
 
-//// [index.js]
-"use strict";
 //// [app.js]
 "use strict";
 require("A");

tests/baselines/reference/pathMappingBasedModuleResolution5_node.js

@@ -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");

tests/baselines/reference/project/nodeModulesMaxDepthIncreased/amd/maxDepthIncreased/root.js

@@ -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
 });

tests/baselines/reference/project/nodeModulesMaxDepthIncreased/amd/nodeModulesMaxDepthIncreased.errors.txt

@@ -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) ====
@@ -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
 

tests/baselines/reference/project/nodeModulesMaxDepthIncreased/amd/nodeModulesMaxDepthIncreased.json

@@ -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": [

tests/baselines/reference/project/nodeModulesMaxDepthIncreased/node/maxDepthIncreased/root.js

@@ -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

tests/baselines/reference/project/nodeModulesMaxDepthIncreased/node/nodeModulesMaxDepthIncreased.errors.txt

@@ -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) ====
@@ -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
 

tests/baselines/reference/project/nodeModulesMaxDepthIncreased/node/nodeModulesMaxDepthIncreased.json

@@ -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": [

tests/cases/projects/NodeModulesSearch/maxDepthIncreased/node_modules/@types/m4/entry.d.ts

@@ -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