Skip to content

Commit 8dff98d

Browse files
sheetalkamatsheetalkamat
authored
Merge pull request microsoft#28992 from Microsoft/tscWatchExportUpdate
When removing the errors for the exports from the file, apart from re…moving transitive exports, remove the diagnostics of file that import these exports
2 parents d35ea02 + c426fc6 commit 8dff98d

File tree

6 files changed

+349
-216
lines changed

6 files changed

+349
-216
lines changed

‎src/compiler/builder.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -281,10 +281,19 @@ namespace ts {
281281
}
282282

283283
// If exported from path is not from cache and exported modules has path, all files referencing file exported from are affected
284-
return !!forEachEntry(state.exportedModulesMap!, (exportedModules, exportedFromPath) =>
284+
if (forEachEntry(state.exportedModulesMap!, (exportedModules, exportedFromPath) =>
285285
!state.currentAffectedFilesExportedModulesMap!.has(exportedFromPath) && // If we already iterated this through cache, ignore it
286286
exportedModules.has(filePath) &&
287287
removeSemanticDiagnosticsOfFileAndExportsOfFile(state, exportedFromPath as Path, seenFileAndExportsOfFile)
288+
)) {
289+
return true;
290+
}
291+
292+
// Remove diagnostics of files that import this file (without going to exports of referencing files)
293+
return !!forEachEntry(state.referencedMap!, (referencesInFile, referencingFilePath) =>
294+
referencesInFile.has(filePath) &&
295+
!seenFileAndExportsOfFile.has(referencingFilePath) && // Not already removed diagnostic file
296+
removeSemanticDiagnosticsOf(state, referencingFilePath as Path) // Dont add to seen since this is not yet done with the export removal
288297
);
289298
}
290299

‎src/testRunner/tsconfig.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@
8989
"unittests/services/transpile.ts",
9090
"unittests/tscWatch/consoleClearing.ts",
9191
"unittests/tscWatch/emit.ts",
92+
"unittests/tscWatch/emitAndErrorUpdates.ts",
9293
"unittests/tscWatch/programUpdates.ts",
9394
"unittests/tscWatch/resolutionCache.ts",
9495
"unittests/tscWatch/watchEnvironment.ts",

‎src/testRunner/unittests/tsbuildWatchMode.ts

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -479,7 +479,7 @@ let x: string = 10;`);
479479
const { host, solutionBuilder } = createSolutionOfProject(allFiles, currentDirectory, solutionBuilderconfig, getOutputFileStamps);
480480

481481
// Build in watch mode
482-
const watch = createWatchOfConfigFileReturningBuilder(watchConfig, host);
482+
const watch = createWatchOfConfigFile(watchConfig, host);
483483
checkOutputErrorsInitial(host, emptyArray);
484484

485485
return { host, solutionBuilder, watch };
@@ -505,10 +505,8 @@ let x: string = 10;`);
505505
projectSystem.checkProjectActualFiles(service.configuredProjects.get(configFile.toLowerCase())!, expectedFiles);
506506
}
507507

508-
type Watch = () => BuilderProgram;
509-
510508
function verifyDependencies(watch: Watch, filePath: string, expected: ReadonlyArray<string>) {
511-
checkArray(`${filePath} dependencies`, watch().getAllDependencies(watch().getSourceFile(filePath)!), expected);
509+
checkArray(`${filePath} dependencies`, watch.getBuilderProgram().getAllDependencies(watch().getSourceFile(filePath)!), expected);
512510
}
513511

514512
describe("on sample project", () => {
@@ -542,7 +540,7 @@ let x: string = 10;`);
542540

543541
host.checkTimeoutQueueLengthAndRun(1);
544542
checkOutputErrorsIncremental(host, emptyArray);
545-
checkProgramActualFiles(watch().getProgram(), expectedFilesAfterEdit);
543+
checkProgramActualFiles(watch(), expectedFilesAfterEdit);
546544

547545
});
548546

@@ -642,7 +640,7 @@ export function gfoo() {
642640
expectedWatchedDirectoriesRecursive: ReadonlyArray<string>,
643641
dependencies: ReadonlyArray<[string, ReadonlyArray<string>]>,
644642
expectedWatchedDirectories?: ReadonlyArray<string>) {
645-
checkProgramActualFiles(watch().getProgram(), expectedProgramFiles);
643+
checkProgramActualFiles(watch(), expectedProgramFiles);
646644
verifyWatchesOfProject(host, expectedWatchedFiles, expectedWatchedDirectoriesRecursive, expectedWatchedDirectories);
647645
for (const [file, deps] of dependencies) {
648646
verifyDependencies(watch, file, deps);

‎src/testRunner/unittests/tscWatch/emitAndErrorUpdates.ts

Lines changed: 307 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,307 @@
1+
namespace ts.tscWatch {
2+
describe("unittests:: tsc-watch:: Emit times and Error updates in builder after program changes", () => {
3+
const currentDirectory = "/user/username/projects/myproject";
4+
const config: File = {
5+
path: `${currentDirectory}/tsconfig.json`,
6+
content: `{}`
7+
};
8+
function getOutputFileStampAndError(host: WatchedSystem, watch: Watch, file: File) {
9+
const builderProgram = watch.getBuilderProgram();
10+
const state = builderProgram.getState();
11+
return {
12+
file,
13+
fileStamp: host.getModifiedTime(file.path.replace(".ts", ".js")),
14+
errors: builderProgram.getSemanticDiagnostics(watch().getSourceFileByPath(file.path as Path)),
15+
errorsFromOldState: !!state.semanticDiagnosticsFromOldState && state.semanticDiagnosticsFromOldState.has(file.path)
16+
};
17+
}
18+
19+
function getOutputFileStampsAndErrors(host: WatchedSystem, watch: Watch, directoryFiles: ReadonlyArray<File>) {
20+
return directoryFiles.map(d => getOutputFileStampAndError(host, watch, d));
21+
}
22+
23+
function findStampAndErrors(stampsAndErrors: ReadonlyArray<ReturnType<typeof getOutputFileStampAndError>>, file: File) {
24+
return find(stampsAndErrors, info => info.file === file)!;
25+
}
26+
27+
function verifyOutputFileStampsAndErrors(
28+
file: File,
29+
emitExpected: boolean,
30+
errorRefershExpected: boolean,
31+
beforeChangeFileStampsAndErrors: ReadonlyArray<ReturnType<typeof getOutputFileStampAndError>>,
32+
afterChangeFileStampsAndErrors: ReadonlyArray<ReturnType<typeof getOutputFileStampAndError>>
33+
) {
34+
const beforeChange = findStampAndErrors(beforeChangeFileStampsAndErrors, file);
35+
const afterChange = findStampAndErrors(afterChangeFileStampsAndErrors, file);
36+
if (emitExpected) {
37+
assert.notStrictEqual(afterChange.fileStamp, beforeChange.fileStamp, `Expected emit for file ${file.path}`);
38+
}
39+
else {
40+
assert.strictEqual(afterChange.fileStamp, beforeChange.fileStamp, `Did not expect new emit for file ${file.path}`);
41+
}
42+
if (errorRefershExpected) {
43+
if (afterChange.errors !== emptyArray || beforeChange.errors !== emptyArray) {
44+
assert.notStrictEqual(afterChange.errors, beforeChange.errors, `Expected new errors for file ${file.path}`);
45+
}
46+
assert.isFalse(afterChange.errorsFromOldState, `Expected errors to be not copied from old state for file ${file.path}`);
47+
}
48+
else {
49+
assert.strictEqual(afterChange.errors, beforeChange.errors, `Expected errors to not change for file ${file.path}`);
50+
assert.isTrue(afterChange.errorsFromOldState, `Expected errors to be copied from old state for file ${file.path}`);
51+
}
52+
}
53+
54+
interface VerifyEmitAndErrorUpdates {
55+
change: (host: WatchedSystem) => void;
56+
getInitialErrors: (watch: Watch) => ReadonlyArray<Diagnostic> | ReadonlyArray<string>;
57+
getIncrementalErrors: (watch: Watch) => ReadonlyArray<Diagnostic> | ReadonlyArray<string>;
58+
filesWithNewEmit: ReadonlyArray<File>;
59+
filesWithOnlyErrorRefresh: ReadonlyArray<File>;
60+
filesNotTouched: ReadonlyArray<File>;
61+
configFile?: File;
62+
}
63+
64+
function verifyEmitAndErrorUpdates({ filesWithNewEmit, filesWithOnlyErrorRefresh, filesNotTouched, configFile = config, change, getInitialErrors, getIncrementalErrors }: VerifyEmitAndErrorUpdates) {
65+
const nonLibFiles = [...filesWithNewEmit, ...filesWithOnlyErrorRefresh, ...filesNotTouched];
66+
const files = [...nonLibFiles, configFile, libFile];
67+
const host = createWatchedSystem(files, { currentDirectory });
68+
const watch = createWatchOfConfigFile("tsconfig.json", host);
69+
checkProgramActualFiles(watch(), [...nonLibFiles.map(f => f.path), libFile.path]);
70+
checkOutputErrorsInitial(host, getInitialErrors(watch));
71+
const beforeChange = getOutputFileStampsAndErrors(host, watch, nonLibFiles);
72+
change(host);
73+
host.runQueuedTimeoutCallbacks();
74+
checkOutputErrorsIncremental(host, getIncrementalErrors(watch));
75+
const afterChange = getOutputFileStampsAndErrors(host, watch, nonLibFiles);
76+
filesWithNewEmit.forEach(file => verifyOutputFileStampsAndErrors(file, /*emitExpected*/ true, /*errorRefershExpected*/ true, beforeChange, afterChange));
77+
filesWithOnlyErrorRefresh.forEach(file => verifyOutputFileStampsAndErrors(file, /*emitExpected*/ false, /*errorRefershExpected*/ true, beforeChange, afterChange));
78+
filesNotTouched.forEach(file => verifyOutputFileStampsAndErrors(file, /*emitExpected*/ false, /*errorRefershExpected*/ false, beforeChange, afterChange));
79+
}
80+
81+
describe("deep import changes", () => {
82+
const aFile: File = {
83+
path: `${currentDirectory}/a.ts`,
84+
content: `import {B} from './b';
85+
declare var console: any;
86+
let b = new B();
87+
console.log(b.c.d);`
88+
};
89+
90+
function verifyDeepImportChange(bFile: File, cFile: File) {
91+
const filesWithNewEmit: File[] = [];
92+
const filesWithOnlyErrorRefresh = [aFile];
93+
addImportedModule(bFile);
94+
addImportedModule(cFile);
95+
verifyEmitAndErrorUpdates({
96+
filesWithNewEmit,
97+
filesWithOnlyErrorRefresh,
98+
filesNotTouched: emptyArray,
99+
change: host => host.writeFile(cFile.path, cFile.content.replace("d", "d2")),
100+
getInitialErrors: () => emptyArray,
101+
getIncrementalErrors: watch => [
102+
getDiagnosticOfFileFromProgram(watch(), aFile.path, aFile.content.lastIndexOf("d"), 1, Diagnostics.Property_0_does_not_exist_on_type_1, "d", "C")
103+
]
104+
});
105+
106+
function addImportedModule(file: File) {
107+
if (file.path.endsWith(".d.ts")) {
108+
filesWithOnlyErrorRefresh.push(file);
109+
}
110+
else {
111+
filesWithNewEmit.push(file);
112+
}
113+
}
114+
}
115+
116+
it("updates errors when deep import file changes", () => {
117+
const bFile: File = {
118+
path: `${currentDirectory}/b.ts`,
119+
content: `import {C} from './c';
120+
export class B
121+
{
122+
c = new C();
123+
}`
124+
};
125+
const cFile: File = {
126+
path: `${currentDirectory}/c.ts`,
127+
content: `export class C
128+
{
129+
d = 1;
130+
}`
131+
};
132+
verifyDeepImportChange(bFile, cFile);
133+
});
134+
135+
it("updates errors when deep import through declaration file changes", () => {
136+
const bFile: File = {
137+
path: `${currentDirectory}/b.d.ts`,
138+
content: `import {C} from './c';
139+
export class B
140+
{
141+
c: C;
142+
}`
143+
};
144+
const cFile: File = {
145+
path: `${currentDirectory}/c.d.ts`,
146+
content: `export class C
147+
{
148+
d: number;
149+
}`
150+
};
151+
verifyDeepImportChange(bFile, cFile);
152+
});
153+
});
154+
155+
it("updates errors in file not exporting a deep multilevel import that changes", () => {
156+
const aFile: File = {
157+
path: `${currentDirectory}/a.ts`,
158+
content: `export interface Point {
159+
name: string;
160+
c: Coords;
161+
}
162+
export interface Coords {
163+
x2: number;
164+
y: number;
165+
}`
166+
};
167+
const bFile: File = {
168+
path: `${currentDirectory}/b.ts`,
169+
content: `import { Point } from "./a";
170+
export interface PointWrapper extends Point {
171+
}`
172+
};
173+
const cFile: File = {
174+
path: `${currentDirectory}/c.ts`,
175+
content: `import { PointWrapper } from "./b";
176+
export function getPoint(): PointWrapper {
177+
return {
178+
name: "test",
179+
c: {
180+
x: 1,
181+
y: 2
182+
}
183+
}
184+
};`
185+
};
186+
const dFile: File = {
187+
path: `${currentDirectory}/d.ts`,
188+
content: `import { getPoint } from "./c";
189+
getPoint().c.x;`
190+
};
191+
const eFile: File = {
192+
path: `${currentDirectory}/e.ts`,
193+
content: `import "./d";`
194+
};
195+
verifyEmitAndErrorUpdates({
196+
filesWithNewEmit: [aFile, bFile],
197+
filesWithOnlyErrorRefresh: [cFile, dFile],
198+
filesNotTouched: [eFile],
199+
change: host => host.writeFile(aFile.path, aFile.content.replace("x2", "x")),
200+
getInitialErrors: watch => [
201+
getDiagnosticOfFileFromProgram(watch(), cFile.path, cFile.content.indexOf("x: 1"), 4, chainDiagnosticMessages(
202+
chainDiagnosticMessages(/*details*/ undefined, Diagnostics.Object_literal_may_only_specify_known_properties_and_0_does_not_exist_in_type_1, "x", "Coords"),
203+
Diagnostics.Type_0_is_not_assignable_to_type_1,
204+
"{ x: number; y: number; }",
205+
"Coords"
206+
)),
207+
getDiagnosticOfFileFromProgram(watch(), dFile.path, dFile.content.lastIndexOf("x"), 1, Diagnostics.Property_0_does_not_exist_on_type_1, "x", "Coords")
208+
],
209+
getIncrementalErrors: () => emptyArray
210+
});
211+
});
212+
213+
describe("updates errors when file transitively exported file changes", () => {
214+
const config: File = {
215+
path: `${currentDirectory}/tsconfig.json`,
216+
content: JSON.stringify({
217+
files: ["app.ts"],
218+
compilerOptions: { baseUrl: "." }
219+
})
220+
};
221+
const app: File = {
222+
path: `${currentDirectory}/app.ts`,
223+
content: `import { Data } from "lib2/public";
224+
export class App {
225+
public constructor() {
226+
new Data().test();
227+
}
228+
}`
229+
};
230+
const lib2Public: File = {
231+
path: `${currentDirectory}/lib2/public.ts`,
232+
content: `export * from "./data";`
233+
};
234+
const lib2Data: File = {
235+
path: `${currentDirectory}/lib2/data.ts`,
236+
content: `import { ITest } from "lib1/public";
237+
export class Data {
238+
public test() {
239+
const result: ITest = {
240+
title: "title"
241+
}
242+
return result;
243+
}
244+
}`
245+
};
246+
const lib1Public: File = {
247+
path: `${currentDirectory}/lib1/public.ts`,
248+
content: `export * from "./tools/public";`
249+
};
250+
const lib1ToolsPublic: File = {
251+
path: `${currentDirectory}/lib1/tools/public.ts`,
252+
content: `export * from "./tools.interface";`
253+
};
254+
const lib1ToolsInterface: File = {
255+
path: `${currentDirectory}/lib1/tools/tools.interface.ts`,
256+
content: `export interface ITest {
257+
title: string;
258+
}`
259+
};
260+
261+
function verifyTransitiveExports(lib2Data: File, lib2Data2?: File) {
262+
const filesWithNewEmit = [lib1ToolsInterface, lib1ToolsPublic];
263+
const filesWithOnlyErrorRefresh = [app, lib2Public, lib1Public, lib2Data];
264+
if (lib2Data2) {
265+
filesWithOnlyErrorRefresh.push(lib2Data2);
266+
}
267+
verifyEmitAndErrorUpdates({
268+
filesWithNewEmit,
269+
filesWithOnlyErrorRefresh,
270+
filesNotTouched: emptyArray,
271+
configFile: config,
272+
change: host => host.writeFile(lib1ToolsInterface.path, lib1ToolsInterface.content.replace("title", "title2")),
273+
getInitialErrors: () => emptyArray,
274+
getIncrementalErrors: () => [
275+
"lib2/data.ts(5,13): error TS2322: Type '{ title: string; }' is not assignable to type 'ITest'.\n Object literal may only specify known properties, but 'title' does not exist in type 'ITest'. Did you mean to write 'title2'?\n"
276+
]
277+
});
278+
}
279+
it("when there are no circular import and exports", () => {
280+
verifyTransitiveExports(lib2Data);
281+
});
282+
283+
it("when there are circular import and exports", () => {
284+
const lib2Data: File = {
285+
path: `${currentDirectory}/lib2/data.ts`,
286+
content: `import { ITest } from "lib1/public"; import { Data2 } from "./data2";
287+
export class Data {
288+
public dat?: Data2; public test() {
289+
const result: ITest = {
290+
title: "title"
291+
}
292+
return result;
293+
}
294+
}`
295+
};
296+
const lib2Data2: File = {
297+
path: `${currentDirectory}/lib2/data2.ts`,
298+
content: `import { Data } from "./data";
299+
export class Data2 {
300+
public dat?: Data;
301+
}`
302+
};
303+
verifyTransitiveExports(lib2Data, lib2Data2);
304+
});
305+
});
306+
});
307+
}

0 commit comments

Comments
 (0)