Skip to content

Commit 9a3dd3e

Browse files
author
Andy
authored
Merge pull request #11993 from Microsoft/common_source_directory
Leave files from `node_modules` out when calculating `getCommonSourceDirectory`
2 parents 382891f + b65729e commit 9a3dd3e

17 files changed

+321
-39
lines changed

‎src/compiler/program.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -431,13 +431,14 @@ namespace ts {
431431
return program;
432432

433433
function getCommonSourceDirectory() {
434-
if (typeof commonSourceDirectory === "undefined") {
435-
if (options.rootDir && checkSourceFilesBelongToPath(files, options.rootDir)) {
434+
if (commonSourceDirectory === undefined) {
435+
const emittedFiles = filterSourceFilesInDirectory(files, isSourceFileFromExternalLibrary);
436+
if (options.rootDir && checkSourceFilesBelongToPath(emittedFiles, options.rootDir)) {
436437
// If a rootDir is specified and is valid use it as the commonSourceDirectory
437438
commonSourceDirectory = getNormalizedAbsolutePath(options.rootDir, currentDirectory);
438439
}
439440
else {
440-
commonSourceDirectory = computeCommonSourceDirectory(files);
441+
commonSourceDirectory = computeCommonSourceDirectory(emittedFiles);
441442
}
442443
if (commonSourceDirectory && commonSourceDirectory[commonSourceDirectory.length - 1] !== directorySeparator) {
443444
// Make sure directory path ends with directory separator so this string can directly

‎src/compiler/utilities.ts

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2576,20 +2576,33 @@ namespace ts {
25762576
}
25772577
else {
25782578
const sourceFiles = targetSourceFile === undefined ? getAllEmittableSourceFiles() : [targetSourceFile];
2579-
return filter(sourceFiles, isNonDeclarationFile);
2579+
return filterSourceFilesInDirectory(sourceFiles, file => host.isSourceFileFromExternalLibrary(file));
25802580
}
25812581

25822582
function getAllEmittableSourceFiles() {
25832583
return options.noEmitForJsFiles ? filter(host.getSourceFiles(), sourceFile => !isSourceFileJavaScript(sourceFile)) : host.getSourceFiles();
25842584
}
25852585
}
25862586

2587+
/** Don't call this for `--outFile`, just for `--outDir` or plain emit. */
2588+
export function filterSourceFilesInDirectory(sourceFiles: SourceFile[], isSourceFileFromExternalLibrary: (file: SourceFile) => boolean): SourceFile[] {
2589+
return filter(sourceFiles, file => shouldEmitInDirectory(file, isSourceFileFromExternalLibrary));
2590+
}
2591+
25872592
function isNonDeclarationFile(sourceFile: SourceFile) {
25882593
return !isDeclarationFile(sourceFile);
25892594
}
25902595

2596+
/**
2597+
* Whether a file should be emitted in a non-`--outFile` case.
2598+
* Don't emit if source file is a declaration file, or was located under node_modules
2599+
*/
2600+
function shouldEmitInDirectory(sourceFile: SourceFile, isSourceFileFromExternalLibrary: (file: SourceFile) => boolean): boolean {
2601+
return isNonDeclarationFile(sourceFile) && !isSourceFileFromExternalLibrary(sourceFile);
2602+
}
2603+
25912604
function isBundleEmitNonExternalModule(sourceFile: SourceFile) {
2592-
return !isDeclarationFile(sourceFile) && !isExternalModule(sourceFile);
2605+
return isNonDeclarationFile(sourceFile) && !isExternalModule(sourceFile);
25932606
}
25942607

25952608
/**
@@ -2677,8 +2690,7 @@ namespace ts {
26772690
else {
26782691
const sourceFiles = targetSourceFile === undefined ? getSourceFilesToEmit(host) : [targetSourceFile];
26792692
for (const sourceFile of sourceFiles) {
2680-
// Don't emit if source file is a declaration file, or was located under node_modules
2681-
if (!isDeclarationFile(sourceFile) && !host.isSourceFileFromExternalLibrary(sourceFile)) {
2693+
if (shouldEmitInDirectory(sourceFile, file => host.isSourceFileFromExternalLibrary(file))) {
26822694
onSingleFileEmit(host, sourceFile);
26832695
}
26842696
}

‎src/harness/compilerRunner.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ class CompilerBaselineRunner extends RunnerBase {
170170
});
171171

172172
it("Correct Sourcemap output for " + fileName, () => {
173-
Harness.Compiler.doSourcemapBaseline(justName, options, result);
173+
Harness.Compiler.doSourcemapBaseline(justName, options, result, harnessSettings);
174174
});
175175

176176
it("Correct type/symbol baselines for " + fileName, () => {

‎src/harness/harness.ts

Lines changed: 18 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -470,19 +470,6 @@ namespace Utils {
470470
}
471471
}
472472

473-
namespace Harness.Path {
474-
export function getFileName(fullPath: string) {
475-
return fullPath.replace(/^.*[\\\/]/, "");
476-
}
477-
478-
export function filePath(fullPath: string) {
479-
fullPath = ts.normalizeSlashes(fullPath);
480-
const components = fullPath.split("/");
481-
const path: string[] = components.slice(0, components.length - 1);
482-
return path.join("/") + "/";
483-
}
484-
}
485-
486473
namespace Harness {
487474
export interface IO {
488475
newLine(): string;
@@ -1090,7 +1077,9 @@ namespace Harness {
10901077
{ name: "suppressOutputPathCheck", type: "boolean" },
10911078
{ name: "noImplicitReferences", type: "boolean" },
10921079
{ name: "currentDirectory", type: "string" },
1093-
{ name: "symlink", type: "string" }
1080+
{ name: "symlink", type: "string" },
1081+
// Emitted js baseline will print full paths for every output file
1082+
{ name: "fullEmitPaths", type: "boolean" }
10941083
];
10951084

10961085
let optionsIndex: ts.Map<ts.CommandLineOption>;
@@ -1565,7 +1554,7 @@ namespace Harness {
15651554
return file.writeByteOrderMark ? "\u00EF\u00BB\u00BF" : "";
15661555
}
15671556

1568-
export function doSourcemapBaseline(baselinePath: string, options: ts.CompilerOptions, result: CompilerResult) {
1557+
export function doSourcemapBaseline(baselinePath: string, options: ts.CompilerOptions, result: CompilerResult, harnessSettings: Harness.TestCaseParser.CompilerSettings) {
15691558
if (options.inlineSourceMap) {
15701559
if (result.sourceMaps.length > 0) {
15711560
throw new Error("No sourcemap files should be generated if inlineSourceMaps was set.");
@@ -1587,10 +1576,8 @@ namespace Harness {
15871576
}
15881577

15891578
let sourceMapCode = "";
1590-
for (let i = 0; i < result.sourceMaps.length; i++) {
1591-
sourceMapCode += "//// [" + Harness.Path.getFileName(result.sourceMaps[i].fileName) + "]\r\n";
1592-
sourceMapCode += getByteOrderMarkText(result.sourceMaps[i]);
1593-
sourceMapCode += result.sourceMaps[i].code;
1579+
for (const sourceMap of result.sourceMaps) {
1580+
sourceMapCode += fileOutput(sourceMap, harnessSettings);
15941581
}
15951582

15961583
return sourceMapCode;
@@ -1611,23 +1598,19 @@ namespace Harness {
16111598
tsCode += "//// [" + header + "] ////\r\n\r\n";
16121599
}
16131600
for (let i = 0; i < tsSources.length; i++) {
1614-
tsCode += "//// [" + Harness.Path.getFileName(tsSources[i].unitName) + "]\r\n";
1601+
tsCode += "//// [" + ts.getBaseFileName(tsSources[i].unitName) + "]\r\n";
16151602
tsCode += tsSources[i].content + (i < (tsSources.length - 1) ? "\r\n" : "");
16161603
}
16171604

16181605
let jsCode = "";
1619-
for (let i = 0; i < result.files.length; i++) {
1620-
jsCode += "//// [" + Harness.Path.getFileName(result.files[i].fileName) + "]\r\n";
1621-
jsCode += getByteOrderMarkText(result.files[i]);
1622-
jsCode += result.files[i].code;
1606+
for (const file of result.files) {
1607+
jsCode += fileOutput(file, harnessSettings);
16231608
}
16241609

16251610
if (result.declFilesCode.length > 0) {
16261611
jsCode += "\r\n\r\n";
1627-
for (let i = 0; i < result.declFilesCode.length; i++) {
1628-
jsCode += "//// [" + Harness.Path.getFileName(result.declFilesCode[i].fileName) + "]\r\n";
1629-
jsCode += getByteOrderMarkText(result.declFilesCode[i]);
1630-
jsCode += result.declFilesCode[i].code;
1612+
for (const declFile of result.declFilesCode) {
1613+
jsCode += fileOutput(declFile, harnessSettings);
16311614
}
16321615
}
16331616

@@ -1652,6 +1635,11 @@ namespace Harness {
16521635
});
16531636
}
16541637

1638+
function fileOutput(file: GeneratedFile, harnessSettings: Harness.TestCaseParser.CompilerSettings): string {
1639+
const fileName = harnessSettings["fullEmitPaths"] ? file.fileName : ts.getBaseFileName(file.fileName);
1640+
return "//// [" + fileName + "]\r\n" + getByteOrderMarkText(file) + file.code;
1641+
}
1642+
16551643
export function collateOutputs(outputFiles: Harness.Compiler.GeneratedFile[]): string {
16561644
// Collect, test, and sort the fileNames
16571645
outputFiles.sort((a, b) => ts.compareStrings(cleanName(a.fileName), cleanName(b.fileName)));
@@ -1848,7 +1836,7 @@ namespace Harness {
18481836
}
18491837

18501838
// normalize the fileName for the single file case
1851-
currentFileName = testUnitData.length > 0 || currentFileName ? currentFileName : Path.getFileName(fileName);
1839+
currentFileName = testUnitData.length > 0 || currentFileName ? currentFileName : ts.getBaseFileName(fileName);
18521840

18531841
// EOF, push whatever remains
18541842
const newTestFile2 = {
@@ -2012,7 +2000,7 @@ namespace Harness {
20122000

20132001
export function isDefaultLibraryFile(filePath: string): boolean {
20142002
// We need to make sure that the filePath is prefixed with "lib." not just containing "lib." and end with ".d.ts"
2015-
const fileName = Path.getFileName(filePath);
2003+
const fileName = ts.getBaseFileName(filePath);
20162004
return ts.startsWith(fileName, "lib.") && ts.endsWith(fileName, ".d.ts");
20172005
}
20182006

‎src/harness/runnerbase.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ abstract class RunnerBase {
3232
/** Replaces instances of full paths with fileNames only */
3333
static removeFullPaths(path: string) {
3434
// If its a full path (starts with "C:" or "/") replace with just the filename
35-
let fixedPath = /^(\w:|\/)/.test(path) ? Harness.Path.getFileName(path) : path;
35+
let fixedPath = /^(\w:|\/)/.test(path) ? ts.getBaseFileName(path) : path;
3636

3737
// when running in the browser the 'full path' is the host name, shows up in error baselines
3838
const localHost = /http:\/localhost:\d+/g;

‎tests/baselines/reference/commonSourceDirectory.js

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
//// [tests/cases/compiler/commonSourceDirectory.ts] ////
2+
3+
//// [index.ts]
4+
// Test that importing a file from `node_modules` does not affect calculation of the common source directory.
5+
6+
export const x = 0;
7+
8+
//// [bar.d.ts]
9+
declare module "bar" {
10+
export const y = 0;
11+
}
12+
13+
//// [index.ts]
14+
/// <reference path="../types/bar.d.ts"/>
15+
import { x } from "foo";
16+
import { y } from "bar";
17+
x + y;
18+
19+
20+
//// [/app/bin/index.js]
21+
"use strict";
22+
/// <reference path="../types/bar.d.ts"/>
23+
var foo_1 = require("foo");
24+
var bar_1 = require("bar");
25+
foo_1.x + bar_1.y;
26+
//# sourceMappingURL=/app/myMapRoot/index.js.map
27+
28+
//// [/app/bin/index.d.ts]
29+
/// <reference path="../../types/bar.d.ts" />

‎tests/baselines/reference/commonSourceDirectory.js.map

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎tests/baselines/reference/commonSourceDirectory.sourcemap.txt

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
===================================================================
2+
JsFile: index.js
3+
mapUrl: /app/myMapRoot/index.js.map
4+
sourceRoot: /app/mySourceRoot/
5+
sources: index.ts
6+
===================================================================
7+
-------------------------------------------------------------------
8+
emittedFile:/app/bin/index.js
9+
sourceFile:index.ts
10+
-------------------------------------------------------------------
11+
>>>"use strict";
12+
>>>/// <reference path="../types/bar.d.ts"/>
13+
1 >
14+
2 >^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
15+
1 >
16+
2 >/// <reference path="../types/bar.d.ts"/>
17+
1 >Emitted(2, 1) Source(1, 1) + SourceIndex(0)
18+
2 >Emitted(2, 42) Source(1, 42) + SourceIndex(0)
19+
---
20+
>>>var foo_1 = require("foo");
21+
1 >
22+
2 >^^^^^^^^^^^^^^^^^^^^^^^^^^^
23+
3 > ^->
24+
1 >
25+
>
26+
2 >import { x } from "foo";
27+
1 >Emitted(3, 1) Source(2, 1) + SourceIndex(0)
28+
2 >Emitted(3, 28) Source(2, 25) + SourceIndex(0)
29+
---
30+
>>>var bar_1 = require("bar");
31+
1->
32+
2 >^^^^^^^^^^^^^^^^^^^^^^^^^^^
33+
1->
34+
>
35+
2 >import { y } from "bar";
36+
1->Emitted(4, 1) Source(3, 1) + SourceIndex(0)
37+
2 >Emitted(4, 28) Source(3, 25) + SourceIndex(0)
38+
---
39+
>>>foo_1.x + bar_1.y;
40+
1 >
41+
2 >^^^^^^^
42+
3 > ^^^
43+
4 > ^^^^^^^
44+
5 > ^
45+
6 > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^->
46+
1 >
47+
>
48+
2 >x
49+
3 > +
50+
4 > y
51+
5 > ;
52+
1 >Emitted(5, 1) Source(4, 1) + SourceIndex(0)
53+
2 >Emitted(5, 8) Source(4, 2) + SourceIndex(0)
54+
3 >Emitted(5, 11) Source(4, 5) + SourceIndex(0)
55+
4 >Emitted(5, 18) Source(4, 6) + SourceIndex(0)
56+
5 >Emitted(5, 19) Source(4, 7) + SourceIndex(0)
57+
---
58+
>>>//# sourceMappingURL=/app/myMapRoot/index.js.map

‎tests/baselines/reference/commonSourceDirectory.symbols

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
=== /app/index.ts ===
2+
/// <reference path="../types/bar.d.ts"/>
3+
import { x } from "foo";
4+
>x : Symbol(x, Decl(index.ts, 1, 8))
5+
6+
import { y } from "bar";
7+
>y : Symbol(y, Decl(index.ts, 2, 8))
8+
9+
x + y;
10+
>x : Symbol(x, Decl(index.ts, 1, 8))
11+
>y : Symbol(y, Decl(index.ts, 2, 8))
12+
13+
=== /node_modules/foo/index.ts ===
14+
// Test that importing a file from `node_modules` does not affect calculation of the common source directory.
15+
16+
export const x = 0;
17+
>x : Symbol(x, Decl(index.ts, 2, 12))
18+
19+
=== /types/bar.d.ts ===
20+
declare module "bar" {
21+
export const y = 0;
22+
>y : Symbol(y, Decl(bar.d.ts, 1, 16))
23+
}
24+

‎tests/baselines/reference/commonSourceDirectory.types

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
=== /app/index.ts ===
2+
/// <reference path="../types/bar.d.ts"/>
3+
import { x } from "foo";
4+
>x : 0
5+
6+
import { y } from "bar";
7+
>y : 0
8+
9+
x + y;
10+
>x + y : number
11+
>x : 0
12+
>y : 0
13+
14+
=== /node_modules/foo/index.ts ===
15+
// Test that importing a file from `node_modules` does not affect calculation of the common source directory.
16+
17+
export const x = 0;
18+
>x : 0
19+
>0 : 0
20+
21+
=== /types/bar.d.ts ===
22+
declare module "bar" {
23+
export const y = 0;
24+
>y : 0
25+
>0 : 0
26+
}
27+

‎tests/baselines/reference/commonSourceDirectory_dts.js

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
//// [tests/cases/compiler/commonSourceDirectory_dts.ts] ////
2+
3+
//// [bar.d.ts]
4+
// Test that importing a file from `node_modules` does not affect calculation of the common source directory.
5+
6+
declare const y: number;
7+
8+
//// [index.ts]
9+
/// <reference path="../lib/bar.d.ts" />
10+
export const x = y;
11+
12+
13+
//// [/app/bin/index.js]
14+
"use strict";
15+
/// <reference path="../lib/bar.d.ts" />
16+
exports.x = y;
17+
//# sourceMappingURL=/app/myMapRoot/index.js.map
18+
19+
//// [/app/bin/index.d.ts]
20+
/// <reference path="../lib/bar.d.ts" />
21+
export declare const x: number;

‎tests/baselines/reference/commonSourceDirectory_dts.js.map

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)