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

During emit, if shape signature for the file is same as version, then update it with emitted d.ts file #48616

Merged
merged 6 commits into from
Apr 21, 2022
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
44 changes: 42 additions & 2 deletions src/compiler/builder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,8 @@ namespace ts {
* true if program has been emitted
*/
programEmitComplete?: true;
/** Stores list of files that change signature during emit - test only */
filesChangingSignature?: Set<Path>;
}

function hasSameKeys(map1: ReadonlyCollection<string> | undefined, map2: ReadonlyCollection<string> | undefined): boolean {
Expand Down Expand Up @@ -1150,7 +1152,9 @@ namespace ts {
// Otherwise just affected file
Debug.checkDefined(state.program).emit(
affected === state.program ? undefined : affected as SourceFile,
writeFile || maybeBind(host, host.writeFile),
affected !== state.program && getEmitDeclarations(state.compilerOptions) && !customTransformers ?
getWriteFileUpdatingSignatureCallback(writeFile) :
writeFile || maybeBind(host, host.writeFile),
cancellationToken,
emitOnlyDtsFiles || emitKind === BuilderFileEmit.DtsOnly,
customTransformers
Expand All @@ -1161,6 +1165,34 @@ namespace ts {
);
}

function getWriteFileUpdatingSignatureCallback(writeFile: WriteFileCallback | undefined): WriteFileCallback {
return (fileName, text, writeByteOrderMark, onError, sourceFiles, data) => {
if (isDeclarationFileName(fileName)) {
Debug.assert(sourceFiles?.length === 1);
const file = sourceFiles[0];
const info = state.fileInfos.get(file.resolvedPath)!;
const signature = state.currentAffectedFilesSignatures?.get(file.resolvedPath) || info.signature;
if (signature === file.version) {
const newSignature = (computeHash || generateDjb2Hash)(data?.sourceMapUrlPos !== undefined ? text.substring(0, data.sourceMapUrlPos) : text);
if (newSignature !== file.version) { // Update it
if (host.storeFilesChangingSignatureDuringEmit) (state.filesChangingSignature ||= new Set()).add(file.resolvedPath);
if (state.exportedModulesMap) BuilderState.updateExportedModules(file, file.exportedModulesFromDeclarationEmit, state.currentAffectedFilesExportedModulesMap ||= BuilderState.createManyToManyPathMap());
if (state.affectedFiles && state.affectedFilesIndex! < state.affectedFiles.length) {
Copy link
Member

Choose a reason for hiding this comment

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

What is this condition? What’s the difference between storing the new signature in state.currentAffectedFilesSignatures and storing it in info.signature?

Copy link
Member Author

@sheetalkamat sheetalkamat Apr 20, 2022

Choose a reason for hiding this comment

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

when we are emitting for affectedFiles things are maintained in currentAffectedFilesSignatures which are commited only after handling of all the affected files is completed.. Not that if we arent handling affected files that means we are doing files that are pending emit only.. So we can directly comit the change to info.signature
This is done so that affected files are handled correctly if the processing for them is cancelled midway, we can revert back as if we didnt process the change at all..
As part of #48784 i am looking into simplifying that esp now that we have affected files pending emit etc.. (this signature cache dates back when we didnt have all these things to track)

state.currentAffectedFilesSignatures!.set(file.resolvedPath, newSignature);
}
else {
info.signature = newSignature;
if (state.exportedModulesMap) BuilderState.updateExportedFilesMapFromCache(state, state.currentAffectedFilesExportedModulesMap);
}
}
}
}
if (writeFile) writeFile(fileName, text, writeByteOrderMark, onError, sourceFiles, data);
else if (host.writeFile) host.writeFile(fileName, text, writeByteOrderMark, onError, sourceFiles, data);
else state.program!.writeFile(fileName, text, writeByteOrderMark, onError, sourceFiles, data);
};
}

/**
* Emits the JavaScript and declaration files.
* When targetSource file is specified, emits the files corresponding to that source file,
Expand Down Expand Up @@ -1215,7 +1247,15 @@ namespace ts {
}
}
}
return Debug.checkDefined(state.program).emit(targetSourceFile, writeFile || maybeBind(host, host.writeFile), cancellationToken, emitOnlyDtsFiles, customTransformers);
return Debug.checkDefined(state.program).emit(
targetSourceFile,
!outFile(state.compilerOptions) && getEmitDeclarations(state.compilerOptions) && !customTransformers ?
getWriteFileUpdatingSignatureCallback(writeFile) :
writeFile || maybeBind(host, host.writeFile),
cancellationToken,
emitOnlyDtsFiles,
customTransformers
);
}

/**
Expand Down
5 changes: 5 additions & 0 deletions src/compiler/builderPublic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@ namespace ts {
*/
/*@internal*/
disableUseFileVersionAsSignature?: boolean;
/**
* Store the list of files that update signature during the emit
*/
/*@internal*/
storeFilesChangingSignatureDuringEmit?: boolean;
Copy link
Member Author

Choose a reason for hiding this comment

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

This one is for test purposes and done so we can print it in the baseline

}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/builderState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -450,7 +450,7 @@ namespace ts {
/**
* Coverts the declaration emit result into exported modules map
*/
function updateExportedModules(sourceFile: SourceFile, exportedModulesFromDeclarationEmit: ExportedModulesFromDeclarationEmit | undefined, exportedModulesMapCache: ManyToManyPathMap) {
export function updateExportedModules(sourceFile: SourceFile, exportedModulesFromDeclarationEmit: ExportedModulesFromDeclarationEmit | undefined, exportedModulesMapCache: ManyToManyPathMap) {
if (!exportedModulesFromDeclarationEmit) {
exportedModulesMapCache.deleteKey(sourceFile.resolvedPath);
return;
Expand Down
4 changes: 3 additions & 1 deletion src/compiler/emitter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -531,6 +531,7 @@ namespace ts {
printer.writeFile(sourceFile!, writer, sourceMapGenerator);
}

let sourceMapUrlPos;
if (sourceMapGenerator) {
if (sourceMapDataList) {
sourceMapDataList.push({
Expand All @@ -548,6 +549,7 @@ namespace ts {

if (sourceMappingURL) {
if (!writer.isAtStartOfLine()) writer.rawWrite(newLine);
sourceMapUrlPos = writer.getTextPos();
writer.writeComment(`//# ${"sourceMappingURL"}=${sourceMappingURL}`); // Tools can sometimes see this line as a source mapping url comment
}

Expand All @@ -562,7 +564,7 @@ namespace ts {
}

// Write the output file
writeFile(host, emitterDiagnostics, jsFilePath, writer.getText(), !!compilerOptions.emitBOM, sourceFiles);
writeFile(host, emitterDiagnostics, jsFilePath, writer.getText(), !!compilerOptions.emitBOM, sourceFiles, { sourceMapUrlPos });

// Reset state
writer.clear();
Expand Down
19 changes: 15 additions & 4 deletions src/compiler/program.ts
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ namespace ts {
return newValue;
};
if (originalWriteFile) {
host.writeFile = (fileName, data, writeByteOrderMark, onError, sourceFiles) => {
host.writeFile = (fileName, data, ...rest) => {
const key = toPath(fileName);
fileExistsCache.delete(key);

Expand All @@ -277,7 +277,7 @@ namespace ts {
sourceFileCache.delete(key);
}
}
originalWriteFile.call(host, fileName, data, writeByteOrderMark, onError, sourceFiles);
originalWriteFile.call(host, fileName, data, ...rest);
};
}

Expand Down Expand Up @@ -1340,6 +1340,7 @@ namespace ts {
useCaseSensitiveFileNames: () => host.useCaseSensitiveFileNames(),
getFileIncludeReasons: () => fileReasons,
structureIsReused,
writeFile,
};

onProgramCreateComplete();
Expand Down Expand Up @@ -1894,8 +1895,7 @@ namespace ts {
getProjectReferenceRedirect,
isSourceOfProjectReferenceRedirect,
getSymlinkCache,
writeFile: writeFileCallback || (
(fileName, data, writeByteOrderMark, onError, sourceFiles) => host.writeFile(fileName, data, writeByteOrderMark, onError, sourceFiles)),
writeFile: writeFileCallback || writeFile,
isEmitBlocked,
readFile: f => host.readFile(f),
fileExists: f => {
Expand All @@ -1914,6 +1914,17 @@ namespace ts {
};
}

function writeFile(
fileName: string,
text: string,
writeByteOrderMark: boolean,
onError?: (message: string) => void,
sourceFiles?: readonly SourceFile[],
data?: WriteFileCallbackData
) {
host.writeFile(fileName, text, writeByteOrderMark, onError, sourceFiles, data);
}

function emitBuildInfo(writeFileCallback?: WriteFileCallback): EmitResult {
Debug.assert(!outFile(options));
tracing?.push(tracing.Phase.Emit, "emitBuildInfo", {}, /*separateBeginAndEnd*/ true);
Expand Down
6 changes: 4 additions & 2 deletions src/compiler/sys.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1210,11 +1210,13 @@ namespace ts {
base64decode?(input: string): string;
base64encode?(input: string): string;
/*@internal*/ bufferFrom?(input: string, encoding?: string): Buffer;
/*@internal*/ require?(baseDir: string, moduleName: string): RequireResult;
/*@internal*/ defaultWatchFileKind?(): WatchFileKind | undefined;

// For testing
/*@internal*/ now?(): Date;
/*@internal*/ disableUseFileVersionAsSignature?: boolean;
/*@internal*/ require?(baseDir: string, moduleName: string): RequireResult;
/*@internal*/ defaultWatchFileKind?(): WatchFileKind | undefined;
/*@internal*/ storeFilesChangingSignatureDuringEmit?: boolean;
}

export interface FileWatcher {
Expand Down
9 changes: 8 additions & 1 deletion src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3856,12 +3856,16 @@ namespace ts {
*/
export type ResolvedConfigFileName = string & { _isResolvedConfigFileName: never };

export interface WriteFileCallbackData {
/*@internal*/ sourceMapUrlPos?: number;
}
export type WriteFileCallback = (
fileName: string,
data: string,
text: string,
writeByteOrderMark: boolean,
onError?: (message: string) => void,
sourceFiles?: readonly SourceFile[],
data?: WriteFileCallbackData,
) => void;

export class OperationCanceledException { }
Expand Down Expand Up @@ -4067,6 +4071,8 @@ namespace ts {
* This implementation handles file exists to be true if file is source of project reference redirect when program is created using useSourceOfProjectReferenceRedirect
*/
/*@internal*/ fileExists(fileName: string): boolean;
/** Call compilerHost.writeFile on host program was created with */
/*@internal*/ writeFile: WriteFileCallback;
}

/*@internal*/
Expand Down Expand Up @@ -6782,6 +6788,7 @@ namespace ts {

// For testing:
/*@internal*/ disableUseFileVersionAsSignature?: boolean;
/*@internal*/ storeFilesChangingSignatureDuringEmit?: boolean;
}

/** true if --out otherwise source file name */
Expand Down
6 changes: 3 additions & 3 deletions src/compiler/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4459,10 +4459,10 @@ namespace ts {
return combinePaths(newDirPath, sourceFilePath);
}

export function writeFile(host: { writeFile: WriteFileCallback; }, diagnostics: DiagnosticCollection, fileName: string, data: string, writeByteOrderMark: boolean, sourceFiles?: readonly SourceFile[]) {
host.writeFile(fileName, data, writeByteOrderMark, hostErrorMessage => {
export function writeFile(host: { writeFile: WriteFileCallback; }, diagnostics: DiagnosticCollection, fileName: string, text: string, writeByteOrderMark: boolean, sourceFiles?: readonly SourceFile[], data?: WriteFileCallbackData) {
host.writeFile(fileName, text, writeByteOrderMark, hostErrorMessage => {
diagnostics.add(createCompilerDiagnostic(Diagnostics.Could_not_write_file_0_Colon_1, fileName, hostErrorMessage));
}, sourceFiles);
}, sourceFiles, data);
}

function ensureDirectoriesExist(
Expand Down
2 changes: 2 additions & 0 deletions src/compiler/watch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -573,6 +573,7 @@ namespace ts {
createHash: maybeBind(host, host.createHash),
readDirectory: maybeBind(host, host.readDirectory),
disableUseFileVersionAsSignature: host.disableUseFileVersionAsSignature,
storeFilesChangingSignatureDuringEmit: host.storeFilesChangingSignatureDuringEmit,
};

function writeFile(fileName: string, text: string, writeByteOrderMark: boolean, onError: (message: string) => void) {
Expand Down Expand Up @@ -637,6 +638,7 @@ namespace ts {
createHash: maybeBind(system, system.createHash),
createProgram: createProgram || createEmitAndSemanticDiagnosticsBuilderProgram as any as CreateProgram<T>,
disableUseFileVersionAsSignature: system.disableUseFileVersionAsSignature,
storeFilesChangingSignatureDuringEmit: system.storeFilesChangingSignatureDuringEmit,
};
}

Expand Down
2 changes: 2 additions & 0 deletions src/compiler/watchPublic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ namespace ts {
const host = createCompilerHostWorker(options, /*setParentNodes*/ undefined, system);
host.createHash = maybeBind(system, system.createHash);
host.disableUseFileVersionAsSignature = system.disableUseFileVersionAsSignature;
host.storeFilesChangingSignatureDuringEmit = system.storeFilesChangingSignatureDuringEmit;
setGetSourceFileAsHashVersioned(host, system);
changeCompilerHostLikeToUseCache(host, fileName => toPath(fileName, host.getCurrentDirectory(), host.getCanonicalFileName));
return host;
Expand Down Expand Up @@ -114,6 +115,7 @@ namespace ts {
writeFile?(path: string, data: string, writeByteOrderMark?: boolean): void;
// For testing
disableUseFileVersionAsSignature?: boolean;
storeFilesChangingSignatureDuringEmit?: boolean;
}

export interface WatchCompilerHost<T extends BuilderProgram> extends ProgramHost<T>, WatchHost {
Expand Down
1 change: 1 addition & 0 deletions src/harness/virtualFileSystemWithWatch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,7 @@ interface Array<T> { length: number; [n: number]: T; }`
private readonly currentDirectory: string;
public require: ((initialPath: string, moduleName: string) => RequireResult) | undefined;
public defaultWatchFileKind?: () => WatchFileKind | undefined;
public storeFilesChangingSignatureDuringEmit = true;
watchFile: HostWatchFile;
watchDirectory: HostWatchDirectory;
constructor(
Expand Down
11 changes: 1 addition & 10 deletions src/testRunner/unittests/tsbuild/outputPaths.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,16 +55,7 @@ namespace ts {
}
})
}),
edits: [
noChangeRun,
{
...noChangeProject,
cleanBuildDiscrepancies: () => new Map([
["/src/dist/tsconfig.tsbuildinfo", CleanBuildDescrepancy.CleanFileTextDifferent], // tsbuildinfo will have -p setting when built using -p vs no build happens incrementally because of no change.
["/src/dist/tsconfig.tsbuildinfo.readable.baseline.txt", CleanBuildDescrepancy.CleanFileTextDifferent] // tsbuildinfo will have -p setting when built using -p vs no build happens incrementally because of no change.
]),
}
],
edits,
}, ["/src/dist/src/index.js", "/src/dist/src/index.d.ts"]);

verify({
Expand Down
2 changes: 2 additions & 0 deletions src/testRunner/unittests/tsc/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ namespace ts {
writtenFiles: Set<Path>;
baseLine(): { file: string; text: string; };
disableUseFileVersionAsSignature?: boolean;
storeFilesChangingSignatureDuringEmit?: boolean;
};

export const noChangeRun: TestTscEdit = {
Expand Down Expand Up @@ -84,6 +85,7 @@ namespace ts {
// Create system
const sys = new fakes.System(fs, { executingFilePath: "/lib/tsc", env: environmentVariables }) as TscCompileSystem;
if (input.disableUseFileVersionAsSignature) sys.disableUseFileVersionAsSignature = true;
sys.storeFilesChangingSignatureDuringEmit = true;
sys.write(`${sys.getExecutingFilePath()} ${commandLineArgs.join(" ")}\n`);
sys.exit = exitCode => sys.exitCode = exitCode;
worker(sys);
Expand Down
7 changes: 5 additions & 2 deletions src/testRunner/unittests/tscWatch/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,7 @@ namespace ts.tscWatch {
if (!builderProgram) return;
if (builderProgram !== oldProgram?.[1]) {
const state = builderProgram.getState();
const internalState = state as unknown as BuilderState;
const internalState = state as unknown as BuilderProgramState;
if (state.semanticDiagnosticsPerFile?.size) {
baseline.push("Semantic diagnostics in builder refreshed for::");
for (const file of program.getSourceFiles()) {
Expand All @@ -354,9 +354,12 @@ namespace ts.tscWatch {
baseline.push("Shape signatures in builder refreshed for::");
internalState.hasCalledUpdateShapeSignature.forEach((path: Path) => {
const info = state.fileInfos.get(path);
if(info?.version === info?.signature || !info?.signature) {
if (info?.version === info?.signature || !info?.signature) {
baseline.push(path + " (used version)");
}
else if (internalState.filesChangingSignature?.has(path)) {
baseline.push(path + " (computed .d.ts during emit)");
}
else {
baseline.push(path + " (computed .d.ts)");
}
Expand Down
4 changes: 3 additions & 1 deletion tests/baselines/reference/api/tsserverlibrary.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2172,7 +2172,9 @@ declare namespace ts {
export type ResolvedConfigFileName = string & {
_isResolvedConfigFileName: never;
};
export type WriteFileCallback = (fileName: string, data: string, writeByteOrderMark: boolean, onError?: (message: string) => void, sourceFiles?: readonly SourceFile[]) => void;
export interface WriteFileCallbackData {
}
export type WriteFileCallback = (fileName: string, text: string, writeByteOrderMark: boolean, onError?: (message: string) => void, sourceFiles?: readonly SourceFile[], data?: WriteFileCallbackData) => void;
export class OperationCanceledException {
}
export interface CancellationToken {
Expand Down
4 changes: 3 additions & 1 deletion tests/baselines/reference/api/typescript.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2172,7 +2172,9 @@ declare namespace ts {
export type ResolvedConfigFileName = string & {
_isResolvedConfigFileName: never;
};
export type WriteFileCallback = (fileName: string, data: string, writeByteOrderMark: boolean, onError?: (message: string) => void, sourceFiles?: readonly SourceFile[]) => void;
export interface WriteFileCallbackData {
}
export type WriteFileCallback = (fileName: string, text: string, writeByteOrderMark: boolean, onError?: (message: string) => void, sourceFiles?: readonly SourceFile[], data?: WriteFileCallbackData) => void;
export class OperationCanceledException {
}
export interface CancellationToken {
Expand Down
Loading