Skip to content
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.

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

Delay the calculation of common source root if it would be needed when calculation dts files #59070

Merged
merged 2 commits into from
Jun 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion src/compiler/program.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4441,7 +4441,8 @@ export function createProgram(rootNamesOrOptions: readonly string[] | CreateProg
options.outDir || // there is --outDir specified
options.rootDir || // there is --rootDir specified
options.sourceRoot || // there is --sourceRoot specified
options.mapRoot // there is --mapRoot specified
options.mapRoot || // there is --mapRoot specified
(getEmitDeclarations(options) && options.declarationDir) // there is --declarationDir specified
) {
// Precalculate and cache the common source directory
const dir = getCommonSourceDirectory();
Expand Down
6 changes: 3 additions & 3 deletions src/compiler/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6315,15 +6315,15 @@ export function getOwnEmitOutputFilePath(fileName: string, host: EmitHost, exten

/** @internal */
export function getDeclarationEmitOutputFilePath(fileName: string, host: EmitHost) {
return getDeclarationEmitOutputFilePathWorker(fileName, host.getCompilerOptions(), host.getCurrentDirectory(), host.getCommonSourceDirectory(), f => host.getCanonicalFileName(f));
return getDeclarationEmitOutputFilePathWorker(fileName, host.getCompilerOptions(), host);
}

/** @internal */
export function getDeclarationEmitOutputFilePathWorker(fileName: string, options: CompilerOptions, currentDirectory: string, commonSourceDirectory: string, getCanonicalFileName: GetCanonicalFileName): string {
export function getDeclarationEmitOutputFilePathWorker(fileName: string, options: CompilerOptions, host: Pick<EmitHost, "getCommonSourceDirectory" | "getCurrentDirectory" | "getCanonicalFileName">): string {
const outputDir = options.declarationDir || options.outDir; // Prefer declaration folder if specified

const path = outputDir
? getSourceFilePathInNewDirWorker(fileName, outputDir, currentDirectory, commonSourceDirectory, getCanonicalFileName)
? getSourceFilePathInNewDirWorker(fileName, outputDir, host.getCurrentDirectory(), host.getCommonSourceDirectory(), f => host.getCanonicalFileName(f))
: fileName;
const declarationExtension = getDeclarationEmitExtensionForPath(path);
return removeFileExtension(path) + declarationExtension;
Expand Down
2 changes: 1 addition & 1 deletion src/server/project.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1699,7 +1699,7 @@ export abstract class Project implements LanguageServiceHost, ModuleResolutionHo
!sourceFile ||
sourceFile.resolvedPath !== source ||
!this.isValidGeneratedFileWatcher(
getDeclarationEmitOutputFilePathWorker(sourceFile.fileName, this.compilerOptions, this.currentDirectory, this.program!.getCommonSourceDirectory(), this.getCanonicalFileName),
getDeclarationEmitOutputFilePathWorker(sourceFile.fileName, this.compilerOptions, this.program!),
watcher,
)
) {
Expand Down
2 changes: 1 addition & 1 deletion src/services/sourcemaps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ export function getSourceMapper(host: SourceMapperHost): SourceMapper {

const declarationPath = outPath ?
removeFileExtension(outPath) + Extension.Dts :
getDeclarationEmitOutputFilePathWorker(info.fileName, program.getCompilerOptions(), currentDirectory, program.getCommonSourceDirectory(), getCanonicalFileName);
getDeclarationEmitOutputFilePathWorker(info.fileName, program.getCompilerOptions(), program);
if (declarationPath === undefined) return undefined;

const newLoc = getDocumentPositionMapper(declarationPath, info.fileName).getGeneratedPosition(info);
Expand Down
27 changes: 27 additions & 0 deletions src/testRunner/unittests/tsserver/projectErrors.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import * as ts from "../../_namespaces/ts.js";
import { jsonToReadableText } from "../helpers.js";
import { libContent } from "../helpers/contents.js";
import {
baselineTsserverLogs,
closeFilesForSession,
Expand Down Expand Up @@ -833,3 +834,29 @@ describe("unittests:: tsserver:: projectErrors:: with file rename on wsl2::", ()
baselineTsserverLogs("projectErrors", "file rename on wsl2", session);
});
});

describe("unittests:: tsserver:: projectErrors:: dts errors when files dont belong to common root", () => {
[undefined, "decls"].forEach(declarationDir => {
it(`dts errors when files dont belong to common root${declarationDir ? " with declarationDir" : ""}`, () => {
const host = createServerHost({
"/home/src/projects/project/src/file.ts": `import { a } from "../a";`,
"/home/src/projects/project/a.ts": `export const a = 10;`,
"/home/src/projects/project/src/tsconfig.json": jsonToReadableText({
compilerOptions: {
composite: true,
noEmit: true,
Copy link
Member Author

Choose a reason for hiding this comment

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

We use to calculate common source dir and report error if OutDir, rootDir, mapRoot or sourceRoot were specified or files would be emitted. So if you had noEmit we wouldnt cache this and then when getting dts diagnostics it would crash because it was over eager (or didnt cache for declarationDir

declarationDir,
},
}),
[libFile.path]: libContent,
});
const session = new TestSession(host);
openFilesForSession(["/home/src/projects/project/src/file.ts"], session);
verifyGetErrRequest({
session,
files: ["/home/src/projects/project/src/file.ts"],
});
baselineTsserverLogs("projectErrors", `dts errors when files dont belong to common root${declarationDir ? " with declarationDir" : ""}`, session);
});
});
});
Loading