Skip to content

Commit 569ecab

Browse files
amcaseyamcasey
committed
Address PR feedback
Make Program.getMissingFilePaths required Assume getMissingFilePaths always returns a defined value Make getMissingFilePaths internal Replace nullable-bool with enum Update type to reflect possibility of undefined Use deepEqual to simplify tests Make condition const Don't bother cleaning up map before freeing it Switch from foreach to for-of to simplify debugging Use a Map, rather than a FileMap, to track open FileWatchers Fix compilation errors Introduce and consume arrayToSet Fix lint warnings about misplaced braces Delete incorrect comment Delete from map during iteration Eliminate unnecessary type annotations
1 parent 0f683ac commit 569ecab

12 files changed

Lines changed: 112 additions & 115 deletions

File tree

‎src/compiler/core.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1097,6 +1097,15 @@ namespace ts {
10971097
return result;
10981098
}
10991099

1100+
/**
1101+
* Creates a set from the elements of an array.
1102+
*
1103+
* @param array the array of input elements.
1104+
*/
1105+
export function arrayToSet<T>(array: T[], makeKey: (value: T) => string): Map<true> {
1106+
return arrayToMap<T, true>(array, makeKey, () => true);
1107+
}
1108+
11001109
export function cloneMap<T>(map: Map<T>) {
11011110
const clone = createMap<T>();
11021111
copyEntries(map, clone);

‎src/compiler/program.ts‎

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -472,7 +472,7 @@ namespace ts {
472472
resolveTypeReferenceDirectiveNamesWorker = (typeReferenceDirectiveNames, containingFile) => loadWithLocalCache(typeReferenceDirectiveNames, containingFile, loader);
473473
}
474474

475-
const filesByName = createMap<SourceFile>();
475+
const filesByName = createMap<SourceFile | undefined>();
476476
// stores 'filename -> file association' ignoring case
477477
// used to track cases when two file names differ only in casing
478478
const filesByNameIgnoreCase = host.useCaseSensitiveFileNames() ? createFileMap<SourceFile>(fileName => fileName.toLowerCase()) : undefined;
@@ -513,7 +513,7 @@ namespace ts {
513513
}
514514
}
515515

516-
const missingFilePaths = filesByName.getKeys().filter(p => !filesByName.get(p));
516+
const missingFilePaths = arrayFrom(filesByName.keys(), p => <Path>p).filter(p => !filesByName.get(p));
517517

518518
// unconditionally set moduleResolutionCache to undefined to avoid unnecessary leaks
519519
moduleResolutionCache = undefined;
@@ -872,17 +872,12 @@ namespace ts {
872872
// will be created until we encounter a change that prevents complete structure reuse.
873873
// During this interval, creation of the file will go unnoticed. We expect this to be
874874
// both rare and low-impact.
875-
if (oldProgram.getMissingFilePaths) {
876-
const missingFilePaths: Path[] = oldProgram.getMissingFilePaths() || emptyArray;
877-
for (const missingFilePath of missingFilePaths) {
878-
if (host.fileExists(missingFilePath)) {
879-
return oldProgram.structureIsReused = StructureIsReused.SafeModules;
880-
}
881-
}
875+
if (oldProgram.getMissingFilePaths().some(missingFilePath => host.fileExists(missingFilePath))) {
876+
return oldProgram.structureIsReused = StructureIsReused.SafeModules;
877+
}
882878

883-
for (const p of oldProgram.getMissingFilePaths()) {
884-
filesByName.set(p, undefined);
885-
}
879+
for (const p of oldProgram.getMissingFilePaths()) {
880+
filesByName.set(p, undefined);
886881
}
887882

888883
// update fileName -> file mapping

‎src/compiler/sys.ts‎

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,13 @@ declare function setTimeout(handler: (...args: any[]) => void, timeout: number):
44
declare function clearTimeout(handle: any): void;
55

66
namespace ts {
7-
export type FileWatcherCallback = (fileName: string, removed?: boolean) => void;
7+
export enum FileWatcherEventKind {
8+
Created,
9+
Changed,
10+
Deleted
11+
}
12+
13+
export type FileWatcherCallback = (fileName: string, eventKind: FileWatcherEventKind) => void;
814
export type DirectoryWatcherCallback = (fileName: string) => void;
915
export interface WatchedFile {
1016
fileName: string;
@@ -174,7 +180,7 @@ namespace ts {
174180
const callbacks = fileWatcherCallbacks.get(fileName);
175181
if (callbacks) {
176182
for (const fileCallback of callbacks) {
177-
fileCallback(fileName);
183+
fileCallback(fileName, FileWatcherEventKind.Changed);
178184
}
179185
}
180186
}
@@ -342,18 +348,20 @@ namespace ts {
342348
function fileChanged(curr: any, prev: any) {
343349
const isCurrZero = +curr.mtime === 0;
344350
const isPrevZero = +prev.mtime === 0;
345-
const added = !isCurrZero && isPrevZero;
351+
const created = !isCurrZero && isPrevZero;
346352
const deleted = isCurrZero && !isPrevZero;
347353

348-
// This value is consistent with poll() in createPollingWatchedFileSet()
349-
// and depended upon by the file watchers created in Project.updateGraphWorker.
350-
const removed = deleted ? true : (added ? false : undefined);
354+
const eventKind = created
355+
? FileWatcherEventKind.Created
356+
: deleted
357+
? FileWatcherEventKind.Deleted
358+
: FileWatcherEventKind.Changed;
351359

352-
if (!added && !deleted && +curr.mtime <= +prev.mtime) {
360+
if (eventKind === FileWatcherEventKind.Changed && +curr.mtime <= +prev.mtime) {
353361
return;
354362
}
355363

356-
callback(fileName, removed);
364+
callback(fileName, eventKind);
357365
}
358366
},
359367
watchDirectory: (directoryName, callback, recursive) => {

‎src/compiler/tsc.ts‎

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -286,18 +286,15 @@ namespace ts {
286286
setCachedProgram(compileResult.program);
287287
reportWatchDiagnostic(createCompilerDiagnostic(Diagnostics.Compilation_complete_Watching_for_file_changes));
288288

289-
if (compileResult.program.getMissingFilePaths) {
290-
const missingPaths = compileResult.program.getMissingFilePaths() || [];
291-
missingPaths.forEach((path: Path): void => {
292-
const fileWatcher = sys.watchFile(path, (_fileName: string, removed?: boolean) => {
293-
// removed = deleted ? true : (added ? false : undefined)
294-
if (removed === false) {
295-
fileWatcher.close();
296-
startTimerForRecompilation();
297-
}
298-
});
289+
const missingPaths = compileResult.program.getMissingFilePaths();
290+
missingPaths.forEach(path => {
291+
const fileWatcher = sys.watchFile(path, (_fileName, eventKind) => {
292+
if (eventKind === FileWatcherEventKind.Created) {
293+
fileWatcher.close();
294+
startTimerForRecompilation();
295+
}
299296
});
300-
}
297+
});
301298
}
302299

303300
function cachedFileExists(fileName: string): boolean {
@@ -321,7 +318,7 @@ namespace ts {
321318
const sourceFile = hostGetSourceFile(fileName, languageVersion, onError);
322319
if (sourceFile && isWatchSet(compilerOptions) && sys.watchFile) {
323320
// Attach a file watcher
324-
sourceFile.fileWatcher = sys.watchFile(sourceFile.fileName, (_fileName: string, removed?: boolean) => sourceFileChanged(sourceFile, removed));
321+
sourceFile.fileWatcher = sys.watchFile(sourceFile.fileName, (_fileName, eventKind) => sourceFileChanged(sourceFile, eventKind));
325322
}
326323
return sourceFile;
327324
}
@@ -343,10 +340,10 @@ namespace ts {
343340
}
344341

345342
// If a source file changes, mark it as unwatched and start the recompilation timer
346-
function sourceFileChanged(sourceFile: SourceFile, removed?: boolean) {
343+
function sourceFileChanged(sourceFile: SourceFile, eventKind: FileWatcherEventKind) {
347344
sourceFile.fileWatcher.close();
348345
sourceFile.fileWatcher = undefined;
349-
if (removed) {
346+
if (eventKind === FileWatcherEventKind.Deleted) {
350347
unorderedRemoveItem(rootFileNames, sourceFile.fileName);
351348
}
352349
startTimerForRecompilation();

‎src/compiler/types.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2430,7 +2430,8 @@ namespace ts {
24302430
* Get a list of file names that were passed to 'createProgram' or referenced in a
24312431
* program source file but could not be located.
24322432
*/
2433-
getMissingFilePaths?(): Path[];
2433+
/* @internal */
2434+
getMissingFilePaths(): Path[];
24342435

24352436
/**
24362437
* Emits the JavaScript and declaration files. If targetSourceFile is not specified, then

‎src/harness/unittests/compileOnSave.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,7 @@ namespace ts.projectSystem {
208208

209209
file1Consumer1.content = `let y = 10;`;
210210
host.reloadFS([moduleFile1, file1Consumer1, file1Consumer2, configFile, libFile]);
211-
host.triggerFileWatcherCallback(file1Consumer1.path, /*removed*/ false);
211+
host.triggerFileWatcherCallback(file1Consumer1.path, FileWatcherEventKind.Changed);
212212

213213
session.executeCommand(changeModuleFile1ShapeRequest1);
214214
sendAffectedFileRequestAndCheckResult(session, moduleFile1FileListRequest, [{ projectFileName: configFile.path, files: [moduleFile1, file1Consumer2] }]);
@@ -225,7 +225,7 @@ namespace ts.projectSystem {
225225
session.executeCommand(changeModuleFile1ShapeRequest1);
226226
// Delete file1Consumer2
227227
host.reloadFS([moduleFile1, file1Consumer1, configFile, libFile]);
228-
host.triggerFileWatcherCallback(file1Consumer2.path, /*removed*/ true);
228+
host.triggerFileWatcherCallback(file1Consumer2.path, FileWatcherEventKind.Deleted);
229229
sendAffectedFileRequestAndCheckResult(session, moduleFile1FileListRequest, [{ projectFileName: configFile.path, files: [moduleFile1, file1Consumer1] }]);
230230
});
231231

@@ -475,7 +475,7 @@ namespace ts.projectSystem {
475475

476476
openFilesForSession([referenceFile1], session);
477477
host.reloadFS([referenceFile1, configFile]);
478-
host.triggerFileWatcherCallback(moduleFile1.path, /*removed*/ true);
478+
host.triggerFileWatcherCallback(moduleFile1.path, FileWatcherEventKind.Deleted);
479479

480480
const request = makeSessionRequest<server.protocol.FileRequestArgs>(CommandNames.CompileOnSaveAffectedFileList, { file: referenceFile1.path });
481481
sendAffectedFileRequestAndCheckResult(session, request, [

‎src/harness/unittests/programMissingFiles.ts‎

Lines changed: 17 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -41,39 +41,33 @@ namespace ts {
4141
const program = createProgram([emptyFileRelativePath], options, testCompilerHost);
4242
const missing = program.getMissingFilePaths();
4343
assert.isDefined(missing);
44-
assert.equal(missing.length, 0);
44+
assert.deepEqual(missing, []);
4545
});
4646

4747
it("handles missing root file", () => {
4848
const program = createProgram(["./nonexistent.ts"], options, testCompilerHost);
4949
const missing = program.getMissingFilePaths();
5050
assert.isDefined(missing);
51-
assert.equal(missing.length, 1);
52-
assert.equal(missing[0].toString(), "d:/pretend/nonexistent.ts"); // Absolute path
51+
assert.deepEqual(missing, ["d:/pretend/nonexistent.ts"]); // Absolute path
5352
});
5453

5554
it("handles multiple missing root files", () => {
5655
const program = createProgram(["./nonexistent0.ts", "./nonexistent1.ts"], options, testCompilerHost);
5756
const missing = program.getMissingFilePaths().sort();
58-
assert.equal(missing.length, 2);
59-
assert.equal(missing[0].toString(), "d:/pretend/nonexistent0.ts");
60-
assert.equal(missing[1].toString(), "d:/pretend/nonexistent1.ts");
57+
assert.deepEqual(missing, ["d:/pretend/nonexistent0.ts", "d:/pretend/nonexistent1.ts"]);
6158
});
6259

6360
it("handles a mix of present and missing root files", () => {
6461
const program = createProgram(["./nonexistent0.ts", emptyFileRelativePath, "./nonexistent1.ts"], options, testCompilerHost);
6562
const missing = program.getMissingFilePaths().sort();
66-
assert.equal(missing.length, 2);
67-
assert.equal(missing[0].toString(), "d:/pretend/nonexistent0.ts");
68-
assert.equal(missing[1].toString(), "d:/pretend/nonexistent1.ts");
63+
assert.deepEqual(missing, ["d:/pretend/nonexistent0.ts", "d:/pretend/nonexistent1.ts"]);
6964
});
7065

7166
it("handles repeatedly specified root files", () => {
7267
const program = createProgram(["./nonexistent.ts", "./nonexistent.ts"], options, testCompilerHost);
7368
const missing = program.getMissingFilePaths();
7469
assert.isDefined(missing);
75-
assert.equal(missing.length, 1);
76-
assert.equal(missing[0].toString(), "d:/pretend/nonexistent.ts");
70+
assert.deepEqual(missing, ["d:/pretend/nonexistent.ts"]);
7771
});
7872

7973
it("normalizes file paths", () => {
@@ -89,21 +83,21 @@ namespace ts {
8983
const program = createProgram([referenceFileRelativePath], options, testCompilerHost);
9084
const missing = program.getMissingFilePaths().sort();
9185
assert.isDefined(missing);
92-
assert.equal(missing.length, 6);
86+
assert.deepEqual(missing, [
87+
// From absolute reference
88+
"d:/imaginary/nonexistent1.ts",
9389

94-
// From absolute reference
95-
assert.equal(missing[0].toString(), "d:/imaginary/nonexistent1.ts");
90+
// From relative reference
91+
"d:/pretend/nonexistent2.ts",
9692

97-
// From relative reference
98-
assert.equal(missing[1].toString(), "d:/pretend/nonexistent2.ts");
93+
// From unqualified reference
94+
"d:/pretend/nonexistent3.ts",
9995

100-
// From unqualified reference
101-
assert.equal(missing[2].toString(), "d:/pretend/nonexistent3.ts");
102-
103-
// From no-extension reference
104-
assert.equal(missing[3].toString(), "d:/pretend/nonexistent4.d.ts");
105-
assert.equal(missing[4].toString(), "d:/pretend/nonexistent4.ts");
106-
assert.equal(missing[5].toString(), "d:/pretend/nonexistent4.tsx");
96+
// From no-extension reference
97+
"d:/pretend/nonexistent4.d.ts",
98+
"d:/pretend/nonexistent4.ts",
99+
"d:/pretend/nonexistent4.tsx"
100+
]);
107101
});
108102
});
109103
}

‎src/harness/unittests/projectErrors.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ namespace ts.projectSystem {
135135
}
136136
// fix config and trigger watcher
137137
host.reloadFS([file1, file2, correctConfig]);
138-
host.triggerFileWatcherCallback(correctConfig.path, /*false*/);
138+
host.triggerFileWatcherCallback(correctConfig.path, FileWatcherEventKind.Changed);
139139
{
140140
projectService.checkNumberOfProjects({ configuredProjects: 1 });
141141
const configuredProject = forEach(projectService.synchronizeProjectList([]), f => f.info.projectName === corruptedConfig.path && f);
@@ -177,7 +177,7 @@ namespace ts.projectSystem {
177177
}
178178
// break config and trigger watcher
179179
host.reloadFS([file1, file2, corruptedConfig]);
180-
host.triggerFileWatcherCallback(corruptedConfig.path, /*false*/);
180+
host.triggerFileWatcherCallback(corruptedConfig.path, FileWatcherEventKind.Changed);
181181
{
182182
projectService.checkNumberOfProjects({ configuredProjects: 1 });
183183
const configuredProject = forEach(projectService.synchronizeProjectList([]), f => f.info.projectName === corruptedConfig.path && f);

0 commit comments

Comments
 (0)