Skip to content

To handle d.ts emit errors that could affect other files, in incremental mode use d.ts emit text + diagnostics as signature of the file #49543

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 7 commits into from
Jun 27, 2022
Merged

To handle d.ts emit errors that could affect other files, in incremental mode use d.ts emit text + diagnostics as signature of the file #49543

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
b8b9478
Add test when declaration emit has errors and d.ts emit doesnt change…
sheetalkamat Jun 13, 2022
d024e27
Refactor
sheetalkamat Jun 14, 2022
daceaa2
Use declaration diagnostics in the d.ts signature for the file so it …
sheetalkamat Jun 14, 2022
21298a0
Renames and clarifications
sheetalkamat Jun 23, 2022
fd38c2c
Merge branch 'main' into staleErrors
sheetalkamat Jun 23, 2022
196d93d
Simplify serialize declaration diagnostics for signature purpose
sheetalkamat Jun 27, 2022
1e39e7b
Fix baselines
sheetalkamat Jun 27, 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
131 changes: 107 additions & 24 deletions src/compiler/builder.ts
View file
Open in desktop

Large diffs are not rendered by default.

86 changes: 61 additions & 25 deletions src/compiler/builderState.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ namespace ts {
export function getFileEmitOutput(program: Program, sourceFile: SourceFile, emitOnlyDtsFiles: boolean,
cancellationToken?: CancellationToken, customTransformers?: CustomTransformers, forceDtsEmit?: boolean): EmitOutput {
const outputFiles: OutputFile[] = [];
const { emitSkipped, diagnostics, exportedModulesFromDeclarationEmit } = program.emit(sourceFile, writeFile, cancellationToken, emitOnlyDtsFiles, customTransformers, forceDtsEmit);
return { outputFiles, emitSkipped, diagnostics, exportedModulesFromDeclarationEmit };
const { emitSkipped, diagnostics } = program.emit(sourceFile, writeFile, cancellationToken, emitOnlyDtsFiles, customTransformers, forceDtsEmit);
return { outputFiles, emitSkipped, diagnostics };

function writeFile(fileName: string, text: string, writeByteOrderMark: boolean) {
outputFiles.push({ name: fileName, writeByteOrderMark, text });
Expand Down Expand Up @@ -321,24 +321,45 @@ namespace ts {
/**
* Gets the files affected by the path from the program
*/
export function getFilesAffectedBy(state: BuilderState, programOfThisState: Program, path: Path, cancellationToken: CancellationToken | undefined, computeHash: ComputeHash): readonly SourceFile[] {
const result = getFilesAffectedByWithOldState(state, programOfThisState, path, cancellationToken, computeHash);
export function getFilesAffectedBy(
state: BuilderState,
programOfThisState: Program,
path: Path,
cancellationToken: CancellationToken | undefined,
computeHash: ComputeHash,
getCanonicalFileName: GetCanonicalFileName,
): readonly SourceFile[] {
const result = getFilesAffectedByWithOldState(
state,
programOfThisState,
path,
cancellationToken,
computeHash,
getCanonicalFileName,
);
state.oldSignatures?.clear();
state.oldExportedModulesMap?.clear();
return result;
}

export function getFilesAffectedByWithOldState(state: BuilderState, programOfThisState: Program, path: Path, cancellationToken: CancellationToken | undefined, computeHash: ComputeHash): readonly SourceFile[] {
export function getFilesAffectedByWithOldState(
state: BuilderState,
programOfThisState: Program,
path: Path,
cancellationToken: CancellationToken | undefined,
computeHash: ComputeHash,
getCanonicalFileName: GetCanonicalFileName,
): readonly SourceFile[] {
const sourceFile = programOfThisState.getSourceFileByPath(path);
if (!sourceFile) {
return emptyArray;
}

if (!updateShapeSignature(state, programOfThisState, sourceFile, cancellationToken, computeHash)) {
if (!updateShapeSignature(state, programOfThisState, sourceFile, cancellationToken, computeHash, getCanonicalFileName)) {
return [sourceFile];
}

return (state.referencedMap ? getFilesAffectedByUpdatedShapeWhenModuleEmit : getFilesAffectedByUpdatedShapeWhenNonModuleEmit)(state, programOfThisState, sourceFile, cancellationToken, computeHash);
return (state.referencedMap ? getFilesAffectedByUpdatedShapeWhenModuleEmit : getFilesAffectedByUpdatedShapeWhenNonModuleEmit)(state, programOfThisState, sourceFile, cancellationToken, computeHash, getCanonicalFileName);
}

export function updateSignatureOfFile(state: BuilderState, signature: string | undefined, path: Path) {
Expand All @@ -349,30 +370,42 @@ namespace ts {
/**
* Returns if the shape of the signature has changed since last emit
*/
export function updateShapeSignature(state: BuilderState, programOfThisState: Program, sourceFile: SourceFile, cancellationToken: CancellationToken | undefined, computeHash: ComputeHash, useFileVersionAsSignature = state.useFileVersionAsSignature) {
export function updateShapeSignature(
state: BuilderState,
programOfThisState: Program,
sourceFile: SourceFile,
cancellationToken: CancellationToken | undefined,
computeHash: ComputeHash,
getCanonicalFileName: GetCanonicalFileName,
useFileVersionAsSignature = state.useFileVersionAsSignature
) {
// If we have cached the result for this file, that means hence forth we should assume file shape is uptodate
if (state.hasCalledUpdateShapeSignature?.has(sourceFile.resolvedPath)) return false;

const info = state.fileInfos.get(sourceFile.resolvedPath)!;
const prevSignature = info.signature;
let latestSignature: string | undefined;
if (!sourceFile.isDeclarationFile && !useFileVersionAsSignature) {
const emitOutput = getFileEmitOutput(
programOfThisState,
programOfThisState.emit(
sourceFile,
/*emitOnlyDtsFiles*/ true,
(fileName, text, _writeByteOrderMark, _onError, sourceFiles, data) => {
Debug.assert(isDeclarationFileName(fileName), `File extension for signature expected to be dts: Got:: ${fileName}`);
latestSignature = computeSignatureWithDiagnostics(
sourceFile,
text,
computeHash,
getCanonicalFileName,
data,
);
if (latestSignature !== prevSignature) {
updateExportedModules(state, sourceFile, sourceFiles![0].exportedModulesFromDeclarationEmit);
}
},
cancellationToken,
/*emitOnlyDtsFiles*/ true,
/*customTransformers*/ undefined,
/*forceDtsEmit*/ true
);
const firstDts = firstOrUndefined(emitOutput.outputFiles);
if (firstDts) {
Debug.assert(isDeclarationFileName(firstDts.name), "File extension for signature expected to be dts", () => `Found: ${getAnyExtensionFromPath(firstDts.name)} for ${firstDts.name}:: All output files: ${JSON.stringify(emitOutput.outputFiles.map(f => f.name))}`);
latestSignature = computeSignature(firstDts.text, computeHash);
if (latestSignature !== prevSignature) {
updateExportedModules(state, sourceFile, emitOutput.exportedModulesFromDeclarationEmit);
}
}
}
// Default is to use file version as signature
if (latestSignature === undefined) {
Expand All @@ -395,10 +428,6 @@ namespace ts {
return latestSignature !== prevSignature;
}

export function computeSignature(text: string, computeHash: ComputeHash | undefined) {
return (computeHash || generateDjb2Hash)(text);
}

/**
* Coverts the declaration emit result into exported modules map
*/
Expand Down Expand Up @@ -556,7 +585,14 @@ namespace ts {
/**
* When program emits modular code, gets the files affected by the sourceFile whose shape has changed
*/
function getFilesAffectedByUpdatedShapeWhenModuleEmit(state: BuilderState, programOfThisState: Program, sourceFileWithUpdatedShape: SourceFile, cancellationToken: CancellationToken | undefined, computeHash: ComputeHash) {
function getFilesAffectedByUpdatedShapeWhenModuleEmit(
state: BuilderState,
programOfThisState: Program,
sourceFileWithUpdatedShape: SourceFile,
cancellationToken: CancellationToken | undefined,
computeHash: ComputeHash,
getCanonicalFileName: GetCanonicalFileName,
) {
if (isFileAffectingGlobalScope(sourceFileWithUpdatedShape)) {
return getAllFilesExcludingDefaultLibraryFile(state, programOfThisState, sourceFileWithUpdatedShape);
}
Expand All @@ -579,7 +615,7 @@ namespace ts {
if (!seenFileNamesMap.has(currentPath)) {
const currentSourceFile = programOfThisState.getSourceFileByPath(currentPath)!;
seenFileNamesMap.set(currentPath, currentSourceFile);
if (currentSourceFile && updateShapeSignature(state, programOfThisState, currentSourceFile, cancellationToken, computeHash)) {
if (currentSourceFile && updateShapeSignature(state, programOfThisState, currentSourceFile, cancellationToken, computeHash, getCanonicalFileName)) {
queue.push(...getReferencedByPaths(state, currentSourceFile.resolvedPath));
}
}
Expand Down
1 change: 0 additions & 1 deletion src/compiler/builderStatePublic.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ namespace ts {
outputFiles: OutputFile[];
emitSkipped: boolean;
/* @internal */ diagnostics: readonly Diagnostic[];
/* @internal */ exportedModulesFromDeclarationEmit?: ExportedModulesFromDeclarationEmit;
}

export interface OutputFile {
Expand Down
33 changes: 14 additions & 19 deletions src/compiler/emitter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,6 @@ namespace ts {
const { enter, exit } = performance.createTimer("printTime", "beforePrint", "afterPrint");
let bundleBuildInfo: BundleBuildInfo | undefined;
let emitSkipped = false;
let exportedModulesFromDeclarationEmit: ExportedModulesFromDeclarationEmit | undefined;

// Emit each output file
enter();
Expand All @@ -308,7 +307,6 @@ namespace ts {
diagnostics: emitterDiagnostics.getDiagnostics(),
emittedFiles: emittedFilesList,
sourceMaps: sourceMapDataList,
exportedModulesFromDeclarationEmit
};

function emitSourceFileOrBundle({ jsFilePath, sourceMapFilePath, declarationFilePath, declarationMapPath, buildInfoPath }: EmitFileNames, sourceFileOrBundle: SourceFile | Bundle | undefined) {
Expand Down Expand Up @@ -414,7 +412,7 @@ namespace ts {
});

Debug.assert(transform.transformed.length === 1, "Should only see one output from the transform");
printSourceFileOrBundle(jsFilePath, sourceMapFilePath, transform.transformed[0], printer, compilerOptions);
printSourceFileOrBundle(jsFilePath, sourceMapFilePath, transform, printer, compilerOptions);

// Clean up emit nodes on parse tree
transform.dispose();
Expand Down Expand Up @@ -453,7 +451,7 @@ namespace ts {
noEmitHelpers: true,
module: compilerOptions.module,
target: compilerOptions.target,
sourceMap: compilerOptions.sourceMap,
sourceMap: !forceDtsEmit && compilerOptions.declarationMap,
inlineSourceMap: compilerOptions.inlineSourceMap,
extendedDiagnostics: compilerOptions.extendedDiagnostics,
onlyPrintJsDocStyle: true,
Expand All @@ -478,20 +476,16 @@ namespace ts {
printSourceFileOrBundle(
declarationFilePath,
declarationMapPath,
declarationTransform.transformed[0],
declarationTransform,
declarationPrinter,
{
sourceMap: !forceDtsEmit && compilerOptions.declarationMap,
sourceMap: printerOptions.sourceMap,
sourceRoot: compilerOptions.sourceRoot,
mapRoot: compilerOptions.mapRoot,
extendedDiagnostics: compilerOptions.extendedDiagnostics,
// Explicitly do not passthru either `inline` option
}
);
if (forceDtsEmit && declarationTransform.transformed[0].kind === SyntaxKind.SourceFile) {
const sourceFile = declarationTransform.transformed[0];
exportedModulesFromDeclarationEmit = sourceFile.exportedModulesFromDeclarationEmit;
}
}
declarationTransform.dispose();
if (bundleBuildInfo) bundleBuildInfo.dts = declarationPrinter.bundleFileInfo;
Expand All @@ -511,7 +505,8 @@ namespace ts {
forEachChild(node, collectLinkedAliases);
}

function printSourceFileOrBundle(jsFilePath: string, sourceMapFilePath: string | undefined, sourceFileOrBundle: SourceFile | Bundle, printer: Printer, mapOptions: SourceMapOptions) {
function printSourceFileOrBundle(jsFilePath: string, sourceMapFilePath: string | undefined, transform: TransformationResult<SourceFile | Bundle>, printer: Printer, mapOptions: SourceMapOptions) {
const sourceFileOrBundle = transform.transformed[0];
const bundle = sourceFileOrBundle.kind === SyntaxKind.Bundle ? sourceFileOrBundle : undefined;
const sourceFile = sourceFileOrBundle.kind === SyntaxKind.SourceFile ? sourceFileOrBundle : undefined;
const sourceFiles = bundle ? bundle.sourceFiles : [sourceFile!];
Expand Down Expand Up @@ -559,7 +554,7 @@ namespace ts {
if (sourceMapFilePath) {
const sourceMap = sourceMapGenerator.toString();
writeFile(host, emitterDiagnostics, sourceMapFilePath, sourceMap, /*writeByteOrderMark*/ false, sourceFiles);
if (printer.bundleFileInfo) printer.bundleFileInfo.mapHash = BuilderState.computeSignature(sourceMap, maybeBind(host, host.createHash));
if (printer.bundleFileInfo) printer.bundleFileInfo.mapHash = computeSignature(sourceMap, maybeBind(host, host.createHash));
}
}
else {
Expand All @@ -568,10 +563,10 @@ namespace ts {

// Write the output file
const text = writer.getText();
writeFile(host, emitterDiagnostics, jsFilePath, text, !!compilerOptions.emitBOM, sourceFiles, { sourceMapUrlPos });
writeFile(host, emitterDiagnostics, jsFilePath, text, !!compilerOptions.emitBOM, sourceFiles, { sourceMapUrlPos, diagnostics: transform.diagnostics });
// We store the hash of the text written in the buildinfo to ensure that text of the referenced d.ts file is same as whats in the buildinfo
// This is needed because incremental can be toggled between two runs and we might use stale file text to do text manipulation in prepend mode
if (printer.bundleFileInfo) printer.bundleFileInfo.hash = BuilderState.computeSignature(text, maybeBind(host, host.createHash));
if (printer.bundleFileInfo) printer.bundleFileInfo.hash = computeSignature(text, maybeBind(host, host.createHash));

// Reset state
writer.clear();
Expand Down Expand Up @@ -774,20 +769,20 @@ namespace ts {
const jsFileText = host.readFile(Debug.checkDefined(jsFilePath));
if (!jsFileText) return jsFilePath!;
// If the jsFileText is not same has what it was created with, tsbuildinfo is stale so dont use it
if (BuilderState.computeSignature(jsFileText, createHash) !== buildInfo.bundle.js.hash) return jsFilePath!;
if (computeSignature(jsFileText, createHash) !== buildInfo.bundle.js.hash) return jsFilePath!;
const sourceMapText = sourceMapFilePath && host.readFile(sourceMapFilePath);
// error if no source map or for now if inline sourcemap
if ((sourceMapFilePath && !sourceMapText) || config.options.inlineSourceMap) return sourceMapFilePath || "inline sourcemap decoding";
if (sourceMapFilePath && BuilderState.computeSignature(sourceMapText!, createHash) !== buildInfo.bundle.js.mapHash) return sourceMapFilePath;
if (sourceMapFilePath && computeSignature(sourceMapText!, createHash) !== buildInfo.bundle.js.mapHash) return sourceMapFilePath;

// read declaration text
const declarationText = declarationFilePath && host.readFile(declarationFilePath);
if (declarationFilePath && !declarationText) return declarationFilePath;
if (declarationFilePath && BuilderState.computeSignature(declarationText!, createHash) !== buildInfo.bundle.dts!.hash) return declarationFilePath;
if (declarationFilePath && computeSignature(declarationText!, createHash) !== buildInfo.bundle.dts!.hash) return declarationFilePath;
const declarationMapText = declarationMapPath && host.readFile(declarationMapPath);
// error if no source map or for now if inline sourcemap
if ((declarationMapPath && !declarationMapText) || config.options.inlineSourceMap) return declarationMapPath || "inline sourcemap decoding";
if (declarationMapPath && BuilderState.computeSignature(declarationMapText!, createHash) !== buildInfo.bundle.dts!.mapHash) return declarationMapPath;
if (declarationMapPath && computeSignature(declarationMapText!, createHash) !== buildInfo.bundle.dts!.mapHash) return declarationMapPath;

const buildInfoDirectory = getDirectoryPath(getNormalizedAbsolutePath(buildInfoPath!, host.getCurrentDirectory()));
const ownPrependInput = createInputFiles(
Expand Down Expand Up @@ -836,7 +831,7 @@ namespace ts {
newBuildInfo.program = buildInfo.program;
if (newBuildInfo.program && changedDtsText !== undefined && config.options.composite) {
// Update the output signature
(newBuildInfo.program as ProgramBundleEmitBuildInfo).outSignature = computeSignature(changedDtsText, changedDtsData, createHash);
(newBuildInfo.program as ProgramBundleEmitBuildInfo).outSignature = computeSignature(changedDtsText, createHash, changedDtsData);
newBuildInfo.program.dtsChangeTime = getCurrentTime(host).getTime();
}
// Update sourceFileInfo
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4026,6 +4026,7 @@ namespace ts {
export interface WriteFileCallbackData {
/*@internal*/ sourceMapUrlPos?: number;
/*@internal*/ buildInfo?: BuildInfo;
/*@internal*/ diagnostics?: readonly DiagnosticWithLocation[];
}
export type WriteFileCallback = (
fileName: string,
Expand Down Expand Up @@ -4335,7 +4336,6 @@ namespace ts {
diagnostics: readonly Diagnostic[];
emittedFiles?: string[]; // Array of files the compiler wrote to disk
/* @internal */ sourceMaps?: SourceMapEmitResult[]; // Array of sourceMapData if compiler emitted sourcemaps
/* @internal */ exportedModulesFromDeclarationEmit?: ExportedModulesFromDeclarationEmit;
}

/* @internal */
Expand Down
3 changes: 2 additions & 1 deletion src/server/project.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -709,7 +709,8 @@ namespace ts.server {
this.program!,
scriptInfo.path,
this.cancellationToken,
maybeBind(this.projectService.host, this.projectService.host.createHash)
maybeBind(this.projectService.host, this.projectService.host.createHash),
this.getCanonicalFileName,
),
sourceFile => this.shouldEmitFile(this.projectService.getScriptInfoForPath(sourceFile.path)) ? sourceFile.fileName : undefined
);
Expand Down
2 changes: 0 additions & 2 deletions src/testRunner/unittests/services/languageService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ export function Component(x: Config): any;`
emitSkipped: true,
diagnostics: emptyArray,
outputFiles: emptyArray,
exportedModulesFromDeclarationEmit: undefined
}
);

Expand All @@ -80,7 +79,6 @@ export function Component(x: Config): any;`
text: "export {};\r\n",
writeByteOrderMark: false
}],
exportedModulesFromDeclarationEmit: undefined
}
);
});
Expand Down
Loading