From 5233bcc064200e4b8fa9304d5f732aab8dacf604 Mon Sep 17 00:00:00 2001 From: Ben Lichtman Date: Fri, 2 Aug 2019 15:53:54 -0700 Subject: [PATCH 1/4] Avoid compile on save for declaration files --- src/server/session.ts | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/src/server/session.ts b/src/server/session.ts index d663ca09249..66f21e88a76 100644 --- a/src/server/session.ts +++ b/src/server/session.ts @@ -1604,15 +1604,22 @@ namespace ts.server { path => this.projectService.getScriptInfoForPath(path)!, projects, (project, info) => { - let result: protocol.CompileOnSaveAffectedFileListSingleProject | undefined; - if (project.compileOnSaveEnabled && project.languageServiceEnabled && !project.isOrphan() && !project.getCompilationSettings().noEmit) { - result = { - projectFileName: project.getProjectName(), - fileNames: project.getCompileOnSaveAffectedFileList(info), - projectUsesOutFile: !!project.getCompilationSettings().outFile || !!project.getCompilationSettings().out - }; + if (!project.compileOnSaveEnabled || !project.languageServiceEnabled || project.isOrphan()) { + return undefined; } - return result; + + const compilationSettings = project.getCompilationSettings(); + + if (!!compilationSettings.noEmit || fileExtensionIs(info.fileName, Extension.Dts) && !getEmitDeclarations(compilationSettings)) { + // avoid triggering emit when a change is made in a .d.ts when declaration emit is disabled + return undefined; + } + + return { + projectFileName: project.getProjectName(), + fileNames: project.getCompileOnSaveAffectedFileList(info), + projectUsesOutFile: !!compilationSettings.outFile || !!compilationSettings.out + }; } ); } From 0cab79fc567bb2b0c00e6cb0713ba6d1c451a9ee Mon Sep 17 00:00:00 2001 From: Ben Lichtman Date: Fri, 2 Aug 2019 15:56:01 -0700 Subject: [PATCH 2/4] Add test --- .../unittests/tsserver/compileOnSave.ts | 97 +++++++++++++++++++ 1 file changed, 97 insertions(+) diff --git a/src/testRunner/unittests/tsserver/compileOnSave.ts b/src/testRunner/unittests/tsserver/compileOnSave.ts index 7dc9ea0497c..2773edb7d8d 100644 --- a/src/testRunner/unittests/tsserver/compileOnSave.ts +++ b/src/testRunner/unittests/tsserver/compileOnSave.ts @@ -503,6 +503,103 @@ namespace ts.projectSystem { }); }); + describe("for changes in declaration files", () => { + function testDTS(dtsFileContents: string, tsFileContents: string, opts: CompilerOptions, expectDTSEmit: boolean) { + const dtsFile = { + path: "/a/runtime/a.d.ts", + content: dtsFileContents + }; + const f2 = { + path: "/a/b.ts", + content: tsFileContents + }; + const config = { + path: "/a/tsconfig.json", + content: JSON.stringify({ + compilerOptions: opts, + compileOnSave: true + }) + }; + const host = createServerHost([dtsFile, f2, config]); + const session = projectSystem.createSession(host); + session.executeCommand({ + seq: 1, + type: "request", + command: "open", + arguments: { file: dtsFile.path } + }); + const projectService = session.getProjectService(); + checkNumberOfProjects(projectService, { configuredProjects: 1 }); + const project = projectService.configuredProjects.get(config.path)!; + checkProjectRootFiles(project, [dtsFile.path, f2.path]); + session.executeCommand({ + seq: 2, + type: "request", + command: "open", + arguments: { file: f2.path } + }); + checkNumberOfProjects(session.getProjectService(), { configuredProjects: 1 }); + const { response } = session.executeCommand({ + seq: 3, + type: "request", + command: "compileOnSaveAffectedFileList", + arguments: { file: dtsFile.path } + }); + if (expectDTSEmit) { + assert.equal((response as protocol.CompileOnSaveAffectedFileListSingleProject[]).length, 1, "expected output from 1 project"); + assert.equal((response as protocol.CompileOnSaveAffectedFileListSingleProject[])[0].fileNames.length, 2, "expected to affect 2 files"); + } + else { + assert.equal((response as protocol.CompileOnSaveAffectedFileListSingleProject[]).length, 0, "expected no output"); + } + + + const { response: response2 } = session.executeCommand({ + seq: 4, + type: "request", + command: "compileOnSaveAffectedFileList", + arguments: { file: f2.path } + }); + assert.equal((response2 as protocol.CompileOnSaveAffectedFileListSingleProject[]).length, 1, "expected output from 1 project"); + } + + it("should return empty array if change is made in a global declaration file", () => { + testDTS( + /*dtsFileContents*/ "declare const x: string;", + /*tsFileContents*/ "var y = 1;", + /*opts*/ {}, + /*expectDTSEmit*/ false + ); + }); + + it("should return empty array if change is made in a module declaration file", () => { + testDTS( + /*dtsFileContents*/ "export const x: string;", + /*tsFileContents*/ "import { x } from './runtime/a;", + /*opts*/ {}, + /*expectDTSEmit*/ false + ); + }); + + it("should return results if change is made in a global declaration file with declaration emit", () => { + testDTS( + /*dtsFileContents*/ "declare const x: string;", + /*tsFileContents*/ "var y = 1;", + /*opts*/ { declaration: true }, + /*expectDTSEmit*/ true + ); + }); + + it("should return results if change is made in a global declaration file with composite enabled", () => { + testDTS( + /*dtsFileContents*/ "declare const x: string;", + /*tsFileContents*/ "var y = 1;", + /*opts*/ { composite: true }, + /*expectDTSEmit*/ true + ); + }); + }); + describe("tsserverProjectSystem emit with outFile or out setting", () => { function test(opts: CompilerOptions, expectedUsesOutFile: boolean) { const f1 = { From c0f187a4e81bf3c29d29a733a5483b0deceb2263 Mon Sep 17 00:00:00 2001 From: Ben Lichtman Date: Tue, 6 Aug 2019 13:39:18 -0700 Subject: [PATCH 3/4] Allow compile on save with decorator emit --- src/server/session.ts | 8 ++++++-- src/testRunner/unittests/tsserver/compileOnSave.ts | 9 +++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/src/server/session.ts b/src/server/session.ts index 66f21e88a76..2db58d67460 100644 --- a/src/server/session.ts +++ b/src/server/session.ts @@ -1599,6 +1599,10 @@ namespace ts.server { return emptyArray; } + function dtsChangeCanAffectEmit(compilationSettings: CompilerOptions) { + return getEmitDeclarations(compilationSettings) || !!compilationSettings.emitDecoratorMetadata; + } + return combineProjectOutput( info, path => this.projectService.getScriptInfoForPath(path)!, @@ -1610,8 +1614,8 @@ namespace ts.server { const compilationSettings = project.getCompilationSettings(); - if (!!compilationSettings.noEmit || fileExtensionIs(info.fileName, Extension.Dts) && !getEmitDeclarations(compilationSettings)) { - // avoid triggering emit when a change is made in a .d.ts when declaration emit is disabled + if (!!compilationSettings.noEmit || fileExtensionIs(info.fileName, Extension.Dts) && !dtsChangeCanAffectEmit(compilationSettings)) { + // avoid triggering emit when a change is made in a .d.ts when declaration emit and decorator metadata emit are disabled return undefined; } diff --git a/src/testRunner/unittests/tsserver/compileOnSave.ts b/src/testRunner/unittests/tsserver/compileOnSave.ts index 2773edb7d8d..b129c8ecdfc 100644 --- a/src/testRunner/unittests/tsserver/compileOnSave.ts +++ b/src/testRunner/unittests/tsserver/compileOnSave.ts @@ -598,6 +598,15 @@ namespace ts.projectSystem { /*expectDTSEmit*/ true ); }); + + it("should return results if change is made in a global declaration file with decorator emit enabled", () => { + testDTS( + /*dtsFileContents*/ "declare const x: string;", + /*tsFileContents*/ "var y = 1;", + /*opts*/ { experimentalDecorators: true, emitDecoratorMetadata: true }, + /*expectDTSEmit*/ true + ); + }); }); describe("tsserverProjectSystem emit with outFile or out setting", () => { From 3646809ecdcaca24627c4de112abd84941f90316 Mon Sep 17 00:00:00 2001 From: Ben Lichtman Date: Wed, 7 Aug 2019 09:46:49 -0700 Subject: [PATCH 4/4] Move helper function outside class --- src/server/session.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/server/session.ts b/src/server/session.ts index 2db58d67460..03ed5e10f8c 100644 --- a/src/server/session.ts +++ b/src/server/session.ts @@ -42,6 +42,11 @@ namespace ts.server { return false; } + + function dtsChangeCanAffectEmit(compilationSettings: CompilerOptions) { + return getEmitDeclarations(compilationSettings) || !!compilationSettings.emitDecoratorMetadata; + } + function formatDiag(fileName: NormalizedPath, project: Project, diag: Diagnostic): protocol.Diagnostic { const scriptInfo = project.getScriptInfoForNormalizedPath(fileName)!; // TODO: GH#18217 return { @@ -1599,10 +1604,6 @@ namespace ts.server { return emptyArray; } - function dtsChangeCanAffectEmit(compilationSettings: CompilerOptions) { - return getEmitDeclarations(compilationSettings) || !!compilationSettings.emitDecoratorMetadata; - } - return combineProjectOutput( info, path => this.projectService.getScriptInfoForPath(path)!,