Skip to content

Delay "File change detected" reporting until createProgram #47427

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.

Sign up for GitHub

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

Merged
merged 10 commits into from
Jan 19, 2022
Merged

Delay "File change detected" reporting until createProgram #47427

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
590166a
Add test based on issue
jakebailey Jan 13, 2022
a2c9d9c
Delay "File change detected" reporting until createProgram
jakebailey Jan 13, 2022
7f5f56e
Accept new baseline for new test
jakebailey Jan 13, 2022
3c844cf
Update baselines for existing tests
jakebailey Jan 13, 2022
e59d8f5
Ensure diag is cleared on synchronizeProgram
jakebailey Jan 13, 2022
582a8da
Be less clever
jakebailey Jan 13, 2022
67e8cff
Fix missing baselines
jakebailey Jan 13, 2022
8b21e9b
Switch to a boolean
jakebailey Jan 18, 2022
d5d0bc6
Clarify bool
jakebailey Jan 18, 2022
e7fc25d
Add test for deleting a file declared in files
jakebailey Jan 19, 2022
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
10 changes: 9 additions & 1 deletion src/compiler/watchPublic.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,7 @@ namespace ts {
let sharedExtendedConfigFileWatchers: ESMap<Path, SharedExtendedConfigFileWatcher<Path>>; // Map of file watchers for extended files, shared between different referenced projects
let extendedConfigCache = host.extendedConfigCache; // Cache for extended config evaluation
let changesAffectResolution = false; // Flag for indicating non-config changes affect module resolution
let reportFileChangeDetectedOnCreateProgram = false; // True if synchronizeProgram should report "File change detected..." when a new program is created

const sourceFilesCache = new Map<string, HostFileInfo>(); // Cache that stores the source file and version info
let missingFilePathsRequestedForRelease: Path[] | undefined; // These paths are held temporarily so that we can remove the entry from source file cache if the file is not tracked by missing files
Expand Down Expand Up @@ -434,15 +435,22 @@ namespace ts {
const hasInvalidatedResolution = resolutionCache.createHasInvalidatedResolution(userProvidedResolution || changesAffectResolution);
if (isProgramUptoDate(getCurrentProgram(), rootFileNames, compilerOptions, getSourceVersion, fileExists, hasInvalidatedResolution, hasChangedAutomaticTypeDirectiveNames, getParsedCommandLine, projectReferences)) {
if (hasChangedConfigFileParsingErrors) {
if (reportFileChangeDetectedOnCreateProgram) {
reportWatchDiagnostic(Diagnostics.File_change_detected_Starting_incremental_compilation);
Copy link
Member

Choose a reason for hiding this comment

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

How much work is done between where this used to be reported and here? I'm concerned that it will look like the compiler is frozen if it reacts "late" to a file change.

Copy link
Member Author

Choose a reason for hiding this comment

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

That's a good question; my skimming says that isProgramUptoDate will:

  • Compare a few arrays for equality
  • Check for not-up-to-date source files, which appears to be checking a number of caches
  • Check for files that the program thought were missing on disk
  • Compare compiler options
  • Compare tsconfig contents by text for position changes for diagnostics

I'm not sure that it's a huge amount of work.

I could try and have it unconditionally run afterProgramCreate, but based on #37266, it seems like this was an intentional change (but, hard to tell entirely, given it's one part of a lot of other tweaks). I'll try it and see if anything breaks catastrophically, but I don't know exactly how to double check that I don't undo a perf fix.

Copy link
Member

Choose a reason for hiding this comment

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

I think we convinced ourselves that it should be cheap. I'm fine bugging this into shape if we get complaints.

}
builderProgram = createProgram(/*rootNames*/ undefined, /*options*/ undefined, compilerHost, builderProgram, configFileParsingDiagnostics, projectReferences);
hasChangedConfigFileParsingErrors = false;
}
}
else {
if (reportFileChangeDetectedOnCreateProgram) {
reportWatchDiagnostic(Diagnostics.File_change_detected_Starting_incremental_compilation);
}
createNewProgram(hasInvalidatedResolution);
}

changesAffectResolution = false; // reset for next sync
reportFileChangeDetectedOnCreateProgram = false;
Copy link
Member

Choose a reason for hiding this comment

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

Can this be cleared in updateProgramWithWatchStatus? I think that would make its lifetime/scope much clearer.

Copy link
Member Author

Choose a reason for hiding this comment

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

I can clear it there instead, but I had been modelling changesAffectResolution.

My main concern was a case where it may be left over for another synchronize later, but I don't know why I thought that.

Copy link
Member

Choose a reason for hiding this comment

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

Personally, I think it's clearer to set it and call it in the same place, but I'm also fine with following local conventions. If this is where things are cleared after sync, then go for it.


if (host.afterProgramCreate && program !== builderProgram) {
host.afterProgramCreate(builderProgram);
Expand Down Expand Up @@ -665,7 +673,7 @@ namespace ts {

function updateProgramWithWatchStatus() {
timerToUpdateProgram = undefined;
reportWatchDiagnostic(Diagnostics.File_change_detected_Starting_incremental_compilation);
reportFileChangeDetectedOnCreateProgram = true;
updateProgram();
}

Expand Down
52 changes: 52 additions & 0 deletions src/testRunner/unittests/tscWatch/programUpdates.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -638,6 +638,34 @@ export class A {
]
});

verifyTscWatch({
scenario,
subScenario: "file in files is deleted",
commandLineArgs: ["-w", "-p", configFilePath],
sys: () => {
const file1 = {
path: "/a/b/f1.ts",
content: "let x = 1"
};
const file2 = {
path: "/a/b/f2.ts",
content: "let y = 1"
};
const configFile = {
path: configFilePath,
content: JSON.stringify({ compilerOptions: {}, files: ["f1.ts", "f2.ts"] })
};
return createWatchedSystem([file1, file2, libFile, configFile]);
},
changes: [
{
caption: "Delete f2",
change: sys => sys.deleteFile("/a/b/f2.ts"),
timeouts: checkSingleTimeoutQueueLengthAndRun,
}
]
});

verifyTscWatch({
scenario,
subScenario: "config file is deleted",
Expand Down Expand Up @@ -1815,5 +1843,29 @@ import { x } from "../b";`),
},
]
});

verifyTscWatch({
scenario,
subScenario: "when creating extensionless file",
commandLineArgs: ["-w", "-p", ".", "--extendedDiagnostics"],
sys: () => {
const module1: File = {
path: `${projectRoot}/index.ts`,
content: ``
};
const config: File = {
path: `${projectRoot}/tsconfig.json`,
content: `{}`
};
return createWatchedSystem([module1, config, libFile], { currentDirectory: projectRoot });
},
changes: [
{
caption: "Create foo in project root",
change: sys => sys.writeFile(`${projectRoot}/foo`, ``),
timeouts: checkSingleTimeoutQueueLengthAndRun,
},
]
});
});
}
2 changes: 1 addition & 1 deletion tests/baselines/reference/tscWatch/consoleClearing/with---diagnostics.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,9 @@ Output::
FileWatcher:: Triggered with /f.ts 1:: WatchInfo: /f.ts 250 undefined Source file
Scheduling update
Elapsed:: *ms FileWatcher:: Triggered with /f.ts 1:: WatchInfo: /f.ts 250 undefined Source file
Synchronizing program
[12:00:17 AM] File change detected. Starting incremental compilation...

Synchronizing program
CreatingProgramWith::
roots: ["/f.ts"]
options: {"watch":true,"diagnostics":true}
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/tscWatch/consoleClearing/with---extendedDiagnostics.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,9 @@ Output::
FileWatcher:: Triggered with /f.ts 1:: WatchInfo: /f.ts 250 undefined Source file
Scheduling update
Elapsed:: *ms FileWatcher:: Triggered with /f.ts 1:: WatchInfo: /f.ts 250 undefined Source file
Synchronizing program
[12:00:17 AM] File change detected. Starting incremental compilation...

Synchronizing program
CreatingProgramWith::
roots: ["/f.ts"]
options: {"watch":true,"extendedDiagnostics":true}
Expand Down
3 changes: 0 additions & 3 deletions tests/baselines/reference/tscWatch/programUpdates/config-file-is-deleted.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,6 @@ Input::
//// [/a/b/tsconfig.json] deleted

Output::
>> Screen clear
[12:00:24 AM] File change detected. Starting incremental compilation...

error TS5083: Cannot read file '/a/b/tsconfig.json'.
Copy link
Member Author

@jakebailey jakebailey Jan 19, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

It's hard to tell in the diff here, but after this message is printed, the process exits (see exitCode below), so it's sort of pointless if this message is printed or not here other than to maybe indicate that we noticed because of a file watcher update.

This same error is printed if we start with a missing config file in the first place.



Expand Down
131 changes: 131 additions & 0 deletions tests/baselines/reference/tscWatch/programUpdates/file-in-files-is-deleted.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
Input::
//// [/a/b/f1.ts]
let x = 1

//// [/a/b/f2.ts]
let y = 1

//// [/a/lib/lib.d.ts]
/// <reference no-default-lib="true"/>
interface Boolean {}
interface Function {}
interface CallableFunction {}
interface NewableFunction {}
interface IArguments {}
interface Number { toExponential: any; }
interface Object {}
interface RegExp {}
interface String { charAt: any; }
interface Array<T> { length: number; [n: number]: T; }

//// [/a/b/tsconfig.json]
{"compilerOptions":{},"files":["f1.ts","f2.ts"]}


/a/lib/tsc.js -w -p /a/b/tsconfig.json
Output::
>> Screen clear
[12:00:17 AM] Starting compilation in watch mode...

[12:00:22 AM] Found 0 errors. Watching for file changes.



Program root files: ["/a/b/f1.ts","/a/b/f2.ts"]
Program options: {"watch":true,"project":"/a/b/tsconfig.json","configFilePath":"/a/b/tsconfig.json"}
Program structureReused: Not
Program files::
/a/lib/lib.d.ts
/a/b/f1.ts
/a/b/f2.ts

Semantic diagnostics in builder refreshed for::
/a/lib/lib.d.ts
/a/b/f1.ts
/a/b/f2.ts

Shape signatures in builder refreshed for::
/a/lib/lib.d.ts (used version)
/a/b/f1.ts (used version)
/a/b/f2.ts (used version)

WatchedFiles::
/a/b/tsconfig.json:
{"fileName":"/a/b/tsconfig.json","pollingInterval":250}
/a/b/f1.ts:
{"fileName":"/a/b/f1.ts","pollingInterval":250}
/a/b/f2.ts:
{"fileName":"/a/b/f2.ts","pollingInterval":250}
/a/lib/lib.d.ts:
{"fileName":"/a/lib/lib.d.ts","pollingInterval":250}

FsWatches::

FsWatchesRecursive::
/a/b/node_modules/@types:
{"directoryName":"/a/b/node_modules/@types","fallbackPollingInterval":500,"fallbackOptions":{"watchFile":"PriorityPollingInterval"}}

exitCode:: ExitStatus.undefined

//// [/a/b/f1.js]
var x = 1;


//// [/a/b/f2.js]
var y = 1;



Change:: Delete f2

Input::
//// [/a/b/f2.ts] deleted

Output::
>> Screen clear
[12:00:24 AM] File change detected. Starting incremental compilation...

error TS6053: File '/a/b/f2.ts' not found.
The file is in the program because:
Part of 'files' list in tsconfig.json

a/b/tsconfig.json:1:40
1 {"compilerOptions":{},"files":["f1.ts","f2.ts"]}
   ~~~~~~~
File is matched by 'files' list specified here.

[12:00:28 AM] Found 1 error. Watching for file changes.



Program root files: ["/a/b/f1.ts","/a/b/f2.ts"]
Program options: {"watch":true,"project":"/a/b/tsconfig.json","configFilePath":"/a/b/tsconfig.json"}
Program structureReused: Not
Program files::
/a/lib/lib.d.ts
/a/b/f1.ts

No cached semantic diagnostics in the builder::

Shape signatures in builder refreshed for::
/a/b/f1.ts (computed .d.ts)

WatchedFiles::
/a/b/tsconfig.json:
{"fileName":"/a/b/tsconfig.json","pollingInterval":250}
/a/b/f1.ts:
{"fileName":"/a/b/f1.ts","pollingInterval":250}
/a/lib/lib.d.ts:
{"fileName":"/a/lib/lib.d.ts","pollingInterval":250}
/a/b/f2.ts:
{"fileName":"/a/b/f2.ts","pollingInterval":250}

FsWatches::

FsWatchesRecursive::
/a/b/node_modules/@types:
{"directoryName":"/a/b/node_modules/@types","fallbackPollingInterval":500,"fallbackOptions":{"watchFile":"PriorityPollingInterval"}}

exitCode:: ExitStatus.undefined

//// [/a/b/f1.js] file written with same contents
111 changes: 111 additions & 0 deletions tests/baselines/reference/tscWatch/programUpdates/when-creating-extensionless-file.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
Input::
//// [/user/username/projects/myproject/index.ts]


//// [/user/username/projects/myproject/tsconfig.json]
{}

//// [/a/lib/lib.d.ts]
/// <reference no-default-lib="true"/>
interface Boolean {}
interface Function {}
interface CallableFunction {}
interface NewableFunction {}
interface IArguments {}
interface Number { toExponential: any; }
interface Object {}
interface RegExp {}
interface String { charAt: any; }
interface Array<T> { length: number; [n: number]: T; }


/a/lib/tsc.js -w -p . --extendedDiagnostics
Output::
[12:00:21 AM] Starting compilation in watch mode...

Current directory: /user/username/projects/myproject CaseSensitiveFileNames: false
FileWatcher:: Added:: WatchInfo: /user/username/projects/myproject/tsconfig.json 2000 undefined Config file
Synchronizing program
CreatingProgramWith::
roots: ["/user/username/projects/myproject/index.ts"]
options: {"watch":true,"project":"/user/username/projects/myproject","extendedDiagnostics":true,"configFilePath":"/user/username/projects/myproject/tsconfig.json"}
FileWatcher:: Added:: WatchInfo: /user/username/projects/myproject/index.ts 250 undefined Source file
FileWatcher:: Added:: WatchInfo: /a/lib/lib.d.ts 250 undefined Source file
DirectoryWatcher:: Added:: WatchInfo: /user/username/projects/myproject/node_modules/@types 1 undefined Type roots
Elapsed:: *ms DirectoryWatcher:: Added:: WatchInfo: /user/username/projects/myproject/node_modules/@types 1 undefined Type roots
[12:00:24 AM] Found 0 errors. Watching for file changes.

DirectoryWatcher:: Added:: WatchInfo: /user/username/projects/myproject 1 undefined Wild card directory
Elapsed:: *ms DirectoryWatcher:: Added:: WatchInfo: /user/username/projects/myproject 1 undefined Wild card directory


Program root files: ["/user/username/projects/myproject/index.ts"]
Program options: {"watch":true,"project":"/user/username/projects/myproject","extendedDiagnostics":true,"configFilePath":"/user/username/projects/myproject/tsconfig.json"}
Program structureReused: Not
Program files::
/a/lib/lib.d.ts
/user/username/projects/myproject/index.ts

Semantic diagnostics in builder refreshed for::
/a/lib/lib.d.ts
/user/username/projects/myproject/index.ts

Shape signatures in builder refreshed for::
/a/lib/lib.d.ts (used version)
/user/username/projects/myproject/index.ts (used version)

WatchedFiles::
/user/username/projects/myproject/tsconfig.json:
{"fileName":"/user/username/projects/myproject/tsconfig.json","pollingInterval":250}
/user/username/projects/myproject/index.ts:
{"fileName":"/user/username/projects/myproject/index.ts","pollingInterval":250}
/a/lib/lib.d.ts:
{"fileName":"/a/lib/lib.d.ts","pollingInterval":250}

FsWatches::

FsWatchesRecursive::
/user/username/projects/myproject/node_modules/@types:
{"directoryName":"/user/username/projects/myproject/node_modules/@types","fallbackPollingInterval":500,"fallbackOptions":{"watchFile":"PriorityPollingInterval"}}
/user/username/projects/myproject:
{"directoryName":"/user/username/projects/myproject","fallbackPollingInterval":500,"fallbackOptions":{"watchFile":"PriorityPollingInterval"}}

exitCode:: ExitStatus.undefined

//// [/user/username/projects/myproject/index.js]



Change:: Create foo in project root

Input::
//// [/user/username/projects/myproject/foo]



Output::
DirectoryWatcher:: Triggered with /user/username/projects/myproject/foo :: WatchInfo: /user/username/projects/myproject 1 undefined Wild card directory
Scheduling update
Elapsed:: *ms DirectoryWatcher:: Triggered with /user/username/projects/myproject/foo :: WatchInfo: /user/username/projects/myproject 1 undefined Wild card directory
Reloading new file names and options
Synchronizing program


WatchedFiles::
/user/username/projects/myproject/tsconfig.json:
{"fileName":"/user/username/projects/myproject/tsconfig.json","pollingInterval":250}
/user/username/projects/myproject/index.ts:
{"fileName":"/user/username/projects/myproject/index.ts","pollingInterval":250}
/a/lib/lib.d.ts:
{"fileName":"/a/lib/lib.d.ts","pollingInterval":250}

FsWatches::

FsWatchesRecursive::
/user/username/projects/myproject/node_modules/@types:
{"directoryName":"/user/username/projects/myproject/node_modules/@types","fallbackPollingInterval":500,"fallbackOptions":{"watchFile":"PriorityPollingInterval"}}
/user/username/projects/myproject:
{"directoryName":"/user/username/projects/myproject","fallbackPollingInterval":500,"fallbackOptions":{"watchFile":"PriorityPollingInterval"}}

exitCode:: ExitStatus.undefined

4 changes: 2 additions & 2 deletions ...baselines/reference/tscWatch/programUpdates/when-creating-new-file-in-symlinked-folder.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,10 +115,10 @@ Output::
DirectoryWatcher:: Triggered with /user/username/projects/myproject/folder2/module3.ts :: WatchInfo: /user/username/projects/myproject/folder2 1 undefined Wild card directory
Scheduling update
Elapsed:: *ms DirectoryWatcher:: Triggered with /user/username/projects/myproject/folder2/module3.ts :: WatchInfo: /user/username/projects/myproject/folder2 1 undefined Wild card directory
[12:00:41 AM] File change detected. Starting incremental compilation...

Reloading new file names and options
Synchronizing program
[12:00:41 AM] File change detected. Starting incremental compilation...

CreatingProgramWith::
roots: ["/user/username/projects/myproject/client/folder1/module1.ts","/user/username/projects/myproject/client/linktofolder2/module2.ts","/user/username/projects/myproject/client/linktofolder2/module3.ts"]
options: {"baseUrl":"/user/username/projects/myproject/client","paths":{"*":["*"]},"pathsBasePath":"/user/username/projects/myproject","watch":true,"project":"/user/username/projects/myproject","extendedDiagnostics":true,"configFilePath":"/user/username/projects/myproject/tsconfig.json"}
Expand Down
Loading