From ef2024a487e2178f8978e7a7cefc4db92becdb0d Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Wed, 12 Sep 2018 14:58:08 -0700 Subject: [PATCH] Handle circular project references --- src/compiler/tsbuild.ts | 20 ++- src/testRunner/unittests/tsbuildWatchMode.ts | 143 +++++++++++-------- 2 files changed, 99 insertions(+), 64 deletions(-) diff --git a/src/compiler/tsbuild.ts b/src/compiler/tsbuild.ts index 415e04adae9..763c62deccf 100644 --- a/src/compiler/tsbuild.ts +++ b/src/compiler/tsbuild.ts @@ -61,6 +61,7 @@ namespace ts { OutOfDateWithUpstream, UpstreamOutOfDate, UpstreamBlocked, + ComputingUpstream, /** * Projects with no outputs (i.e. "solution" files) @@ -76,6 +77,7 @@ namespace ts { | Status.OutOfDateWithUpstream | Status.UpstreamOutOfDate | Status.UpstreamBlocked + | Status.ComputingUpstream | Status.ContainerOnly; export namespace Status { @@ -145,6 +147,13 @@ namespace ts { upstreamProjectName: string; } + /** + * Computing status of upstream projects referenced + */ + export interface ComputingUpstream { + type: UpToDateStatusType.ComputingUpstream; + } + /** * One or more of the project's outputs is older than the newest output of * an upstream project. @@ -689,11 +698,17 @@ namespace ts { let usesPrepend = false; let upstreamChangedProject: string | undefined; if (project.projectReferences) { + projectStatus.setValue(project.options.configFilePath as ResolvedConfigFileName, { type: UpToDateStatusType.ComputingUpstream }); for (const ref of project.projectReferences) { usesPrepend = usesPrepend || !!(ref.prepend); const resolvedRef = resolveProjectReferencePath(ref); const refStatus = getUpToDateStatus(parseConfigFile(resolvedRef)); + // Its a circular reference ignore the status of this project + if (refStatus.type === UpToDateStatusType.ComputingUpstream) { + continue; + } + // An upstream project is blocked if (refStatus.type === UpToDateStatusType.Unbuildable) { return { @@ -928,9 +943,10 @@ namespace ts { // Circular if (temporaryMarks.hasKey(projPath)) { if (!inCircularContext) { + // TODO:: Do we report this as error? reportStatus(Diagnostics.Project_references_may_not_form_a_circular_graph_Cycle_detected_Colon_0, circularityReportStack.join("\r\n")); - return; } + return; } temporaryMarks.setValue(projPath, true); @@ -1263,6 +1279,8 @@ namespace ts { status.reason); case UpToDateStatusType.ContainerOnly: // Don't report status on "solution" projects + case UpToDateStatusType.ComputingUpstream: + // Should never leak from getUptoDateStatusWorker break; default: assertType(status); diff --git a/src/testRunner/unittests/tsbuildWatchMode.ts b/src/testRunner/unittests/tsbuildWatchMode.ts index e2abd2a0d04..17e61140b2d 100644 --- a/src/testRunner/unittests/tsbuildWatchMode.ts +++ b/src/testRunner/unittests/tsbuildWatchMode.ts @@ -95,7 +95,7 @@ namespace ts.tscWatch { const allFiles: ReadonlyArray = [libFile, ...core, ...logic, ...tests, ...ui]; const testProjectExpectedWatchedFiles = [core[0], core[1], core[2], ...logic, ...tests].map(f => f.path); - function createSolutionInWatchMode() { + function createSolutionInWatchMode(allFiles: ReadonlyArray) { const host = createWatchedSystem(allFiles, { currentDirectory: projectsLocation }); createSolutionBuilderWithWatch(host, [`${project}/${SubProject.tests}`]); verifyWatches(host); @@ -114,7 +114,7 @@ namespace ts.tscWatch { } it("creates solution in watch mode", () => { - createSolutionInWatchMode(); + createSolutionInWatchMode(allFiles); }); describe("validates the changes and watched files", () => { @@ -124,82 +124,99 @@ namespace ts.tscWatch { content: `export const newFileConst = 30;` }; - function createSolutionInWatchModeToVerifyChanges(additionalFiles?: ReadonlyArray<[SubProject, string]>) { - const host = createSolutionInWatchMode(); - return { host, verifyChangeWithFile, verifyChangeAfterTimeout, verifyWatches }; + function verifyProjectChanges(allFiles: ReadonlyArray) { + function createSolutionInWatchModeToVerifyChanges(additionalFiles?: ReadonlyArray<[SubProject, string]>) { + const host = createSolutionInWatchMode(allFiles); + return { host, verifyChangeWithFile, verifyChangeAfterTimeout, verifyWatches }; - function verifyChangeWithFile(fileName: string, content: string) { - const outputFileStamps = getOutputFileStamps(host, additionalFiles); - host.writeFile(fileName, content); - verifyChangeAfterTimeout(outputFileStamps); + function verifyChangeWithFile(fileName: string, content: string) { + const outputFileStamps = getOutputFileStamps(host, additionalFiles); + host.writeFile(fileName, content); + verifyChangeAfterTimeout(outputFileStamps); + } + + function verifyChangeAfterTimeout(outputFileStamps: OutputFileStamp[]) { + host.checkTimeoutQueueLengthAndRun(1); // Builds core + const changedCore = getOutputFileStamps(host, additionalFiles); + verifyChangedFiles(changedCore, outputFileStamps, [ + ...getOutputFileNames(SubProject.core, "anotherModule"), // This should not be written really + ...getOutputFileNames(SubProject.core, "index"), + ...(additionalFiles ? getOutputFileNames(SubProject.core, newFileWithoutExtension) : emptyArray) + ]); + host.checkTimeoutQueueLengthAndRun(1); // Builds logic + const changedLogic = getOutputFileStamps(host, additionalFiles); + verifyChangedFiles(changedLogic, changedCore, [ + ...getOutputFileNames(SubProject.logic, "index") // Again these need not be written + ]); + host.checkTimeoutQueueLengthAndRun(1); // Builds tests + const changedTests = getOutputFileStamps(host, additionalFiles); + verifyChangedFiles(changedTests, changedLogic, [ + ...getOutputFileNames(SubProject.tests, "index") // Again these need not be written + ]); + host.checkTimeoutQueueLength(0); + checkOutputErrorsIncremental(host, emptyArray); + verifyWatches(); + } + + function verifyWatches() { + checkWatchedFiles(host, additionalFiles ? testProjectExpectedWatchedFiles.concat(newFile.path) : testProjectExpectedWatchedFiles); + checkWatchedDirectories(host, emptyArray, /*recursive*/ false); + checkWatchedDirectories(host, [projectPath(SubProject.core), projectPath(SubProject.logic)], /*recursive*/ true); + } } - function verifyChangeAfterTimeout(outputFileStamps: OutputFileStamp[]) { - host.checkTimeoutQueueLengthAndRun(1); // Builds core - const changedCore = getOutputFileStamps(host, additionalFiles); - verifyChangedFiles(changedCore, outputFileStamps, [ - ...getOutputFileNames(SubProject.core, "anotherModule"), // This should not be written really - ...getOutputFileNames(SubProject.core, "index"), - ...(additionalFiles ? getOutputFileNames(SubProject.core, newFileWithoutExtension) : emptyArray) - ]); - host.checkTimeoutQueueLengthAndRun(1); // Builds logic - const changedLogic = getOutputFileStamps(host, additionalFiles); - verifyChangedFiles(changedLogic, changedCore, [ - ...getOutputFileNames(SubProject.logic, "index") // Again these need not be written - ]); - host.checkTimeoutQueueLengthAndRun(1); // Builds tests - const changedTests = getOutputFileStamps(host, additionalFiles); - verifyChangedFiles(changedTests, changedLogic, [ - ...getOutputFileNames(SubProject.tests, "index") // Again these need not be written - ]); - host.checkTimeoutQueueLength(0); - checkOutputErrorsIncremental(host, emptyArray); - verifyWatches(); - } - - function verifyWatches() { - checkWatchedFiles(host, additionalFiles ? testProjectExpectedWatchedFiles.concat(newFile.path) : testProjectExpectedWatchedFiles); - checkWatchedDirectories(host, emptyArray, /*recursive*/ false); - checkWatchedDirectories(host, [projectPath(SubProject.core), projectPath(SubProject.logic)], /*recursive*/ true); - } - } - - it("change builds changes and reports found errors message", () => { - const { host, verifyChangeWithFile, verifyChangeAfterTimeout } = createSolutionInWatchModeToVerifyChanges(); - verifyChange(`${core[1].content} + it("change builds changes and reports found errors message", () => { + const { host, verifyChangeWithFile, verifyChangeAfterTimeout } = createSolutionInWatchModeToVerifyChanges(); + verifyChange(`${core[1].content} export class someClass { }`); - // Another change requeues and builds it - verifyChange(core[1].content); + // Another change requeues and builds it + verifyChange(core[1].content); - // Two changes together report only single time message: File change detected. Starting incremental compilation... - const outputFileStamps = getOutputFileStamps(host); - const change1 = `${core[1].content} + // Two changes together report only single time message: File change detected. Starting incremental compilation... + const outputFileStamps = getOutputFileStamps(host); + const change1 = `${core[1].content} export class someClass { }`; - host.writeFile(core[1].path, change1); - host.writeFile(core[1].path, `${change1} + host.writeFile(core[1].path, change1); + host.writeFile(core[1].path, `${change1} export class someClass2 { }`); - verifyChangeAfterTimeout(outputFileStamps); + verifyChangeAfterTimeout(outputFileStamps); - function verifyChange(coreContent: string) { - verifyChangeWithFile(core[1].path, coreContent); - } - }); + function verifyChange(coreContent: string) { + verifyChangeWithFile(core[1].path, coreContent); + } + }); - it("builds when new file is added, and its subsequent updates", () => { - const additinalFiles: ReadonlyArray<[SubProject, string]> = [[SubProject.core, newFileWithoutExtension]]; - const { verifyChangeWithFile } = createSolutionInWatchModeToVerifyChanges(additinalFiles); - verifyChange(newFile.content); + it("builds when new file is added, and its subsequent updates", () => { + const additinalFiles: ReadonlyArray<[SubProject, string]> = [[SubProject.core, newFileWithoutExtension]]; + const { verifyChangeWithFile } = createSolutionInWatchModeToVerifyChanges(additinalFiles); + verifyChange(newFile.content); - // Another change requeues and builds it - verifyChange(`${newFile.content} + // Another change requeues and builds it + verifyChange(`${newFile.content} export class someClass2 { }`); - function verifyChange(newFileContent: string) { - verifyChangeWithFile(newFile.path, newFileContent); - } + function verifyChange(newFileContent: string) { + verifyChangeWithFile(newFile.path, newFileContent); + } + }); + } + + describe("with simple project reference graph", () => { + verifyProjectChanges(allFiles); }); + describe("with circular project reference", () => { + const [coreTsconfig, ...otherCoreFiles] = core; + const circularCoreConfig: File = { + path: coreTsconfig.path, + content: JSON.stringify({ + compilerOptions: { composite: true, declaration: true }, + references: [{ path: "../tests", circular: true }] + }) + }; + verifyProjectChanges([libFile, circularCoreConfig, ...otherCoreFiles, ...logic, ...tests]); + }); }); it("watches config files that are not present", () => {