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.

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

Jump to bottom

If module exports const enum, invalidate js files along with dts as it can impact js emit as well #58594

Merged
merged 5 commits into from
May 21, 2024
Merged

If module exports const enum, invalidate js files along with dts as it can impact js emit as well #58594

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e7dac86
Test
sheetalkamat May 20, 2024
2407d2d
If module exports const enum, invalidate js files along with dts as i…
sheetalkamat May 20, 2024
ec73dba
Lint
sheetalkamat May 20, 2024
0afacb5
Add test for exported from another file
sheetalkamat May 21, 2024
afd521f
only consider current file const enums for js file invalidation
sheetalkamat May 21, 2024
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: 38 additions & 6 deletions src/compiler/builder.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ import {
getOptionsNameMap,
getOwnKeys,
getRelativePathFromDirectory,
getSourceFileOfNode,
getTsBuildInfoEmitOutputFilePath,
handleNoEmitOptions,
HostForComputeHash,
Expand All @@ -74,10 +75,13 @@ import {
sameMap,
SemanticDiagnosticsBuilderProgram,
SignatureInfo,
skipAlias,
skipTypeChecking,
some,
SourceFile,
sourceFileMayBeEmitted,
SourceMapEmitResult,
SymbolFlags,
toPath,
tryAddToSet,
WriteFileCallback,
Expand Down Expand Up @@ -762,6 +766,7 @@ function handleDtsMayChangeOfAffectedFile(
function handleDtsMayChangeOf(
state: BuilderProgramState,
path: Path,
invalidateJsFiles: boolean,
cancellationToken: CancellationToken | undefined,
host: BuilderProgramHost,
): void {
Expand All @@ -785,7 +790,10 @@ function handleDtsMayChangeOf(
/*useFileVersionAsSignature*/ true,
);
// If not dts emit, nothing more to do
if (getEmitDeclarations(state.compilerOptions)) {
if (invalidateJsFiles) {
addToAffectedFilesPendingEmit(state, path, getBuilderFileEmit(state.compilerOptions));
}
else if (getEmitDeclarations(state.compilerOptions)) {
addToAffectedFilesPendingEmit(state, path, state.compilerOptions.declarationMap ? BuilderFileEmit.AllDts : BuilderFileEmit.Dts);
}
}
Expand Down Expand Up @@ -814,6 +822,7 @@ function isChangedSignature(state: BuilderProgramState, path: Path) {
function handleDtsMayChangeOfGlobalScope(
state: BuilderProgramState,
filePath: Path,
invalidateJsFiles: boolean,
cancellationToken: CancellationToken | undefined,
host: BuilderProgramHost,
): boolean {
Expand All @@ -824,6 +833,7 @@ function handleDtsMayChangeOfGlobalScope(
handleDtsMayChangeOf(
state,
file.resolvedPath,
invalidateJsFiles,
cancellationToken,
host,
)
Expand Down Expand Up @@ -856,8 +866,16 @@ function handleDtsMayChangeOfReferencingExportOfAffectedFile(
const currentPath = queue.pop()!;
if (!seenFileNamesMap.has(currentPath)) {
seenFileNamesMap.set(currentPath, true);
if (handleDtsMayChangeOfGlobalScope(state, currentPath, cancellationToken, host)) return;
handleDtsMayChangeOf(state, currentPath, cancellationToken, host);
if (
handleDtsMayChangeOfGlobalScope(
state,
currentPath,
/*invalidateJsFiles*/ false,
cancellationToken,
host,
)
) return;
handleDtsMayChangeOf(state, currentPath, /*invalidateJsFiles*/ false, cancellationToken, host);
if (isChangedSignature(state, currentPath)) {
const currentSourceFile = Debug.checkDefined(state.program).getSourceFileByPath(currentPath)!;
queue.push(...BuilderState.getReferencedByPaths(state, currentSourceFile.resolvedPath));
Expand All @@ -867,14 +885,26 @@ function handleDtsMayChangeOfReferencingExportOfAffectedFile(
}

const seenFileAndExportsOfFile = new Set<string>();
// If exported const enum, we need to ensure that js files are emitted as well since the const enum value changed
const invalidateJsFiles = !!affectedFile.symbol?.exports && !!forEachEntry(
affectedFile.symbol.exports,
exported => {
if ((exported.flags & SymbolFlags.ConstEnum) !== 0) return true;
const aliased = skipAlias(exported, state.program!.getTypeChecker());
if (aliased === exported) return false;
return (aliased.flags & SymbolFlags.ConstEnum) !== 0 &&
some(aliased.declarations, d => getSourceFileOfNode(d) === affectedFile);
},
);
// Go through files that reference affected file and handle dts emit and semantic diagnostics for them and their references
state.referencedMap.getKeys(affectedFile.resolvedPath)?.forEach(exportedFromPath => {
if (handleDtsMayChangeOfGlobalScope(state, exportedFromPath, cancellationToken, host)) return true;
if (handleDtsMayChangeOfGlobalScope(state, exportedFromPath, invalidateJsFiles, cancellationToken, host)) return true;
const references = state.referencedMap!.getKeys(exportedFromPath);
return references && forEachKey(references, filePath =>
handleDtsMayChangeOfFileAndExportsOfFile(
state,
filePath,
invalidateJsFiles,
seenFileAndExportsOfFile,
cancellationToken,
host,
Expand All @@ -889,20 +919,22 @@ function handleDtsMayChangeOfReferencingExportOfAffectedFile(
function handleDtsMayChangeOfFileAndExportsOfFile(
state: BuilderProgramState,
filePath: Path,
invalidateJsFiles: boolean,
seenFileAndExportsOfFile: Set<string>,
cancellationToken: CancellationToken | undefined,
host: BuilderProgramHost,
): boolean | undefined {
if (!tryAddToSet(seenFileAndExportsOfFile, filePath)) return undefined;

if (handleDtsMayChangeOfGlobalScope(state, filePath, cancellationToken, host)) return true;
handleDtsMayChangeOf(state, filePath, cancellationToken, host);
if (handleDtsMayChangeOfGlobalScope(state, filePath, invalidateJsFiles, cancellationToken, host)) return true;
handleDtsMayChangeOf(state, filePath, invalidateJsFiles, cancellationToken, host);

// Remove the diagnostics of files that import this file and handle all its exports too
state.referencedMap!.getKeys(filePath)?.forEach(referencingFilePath =>
handleDtsMayChangeOfFileAndExportsOfFile(
state,
referencingFilePath,
invalidateJsFiles,
seenFileAndExportsOfFile,
cancellationToken,
host,
Expand Down
125 changes: 100 additions & 25 deletions src/testRunner/unittests/tsc/incremental.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import * as ts from "../../_namespaces/ts.js";
import * as Utils from "../../_namespaces/Utils.js";
import * as vfs from "../../_namespaces/vfs.js";
import { dedent } from "../../_namespaces/Utils.js";
import { FileSystem } from "../../_namespaces/vfs.js";
import { jsonToReadableText } from "../helpers.js";
import {
compilerOptionsToConfigJson,
Expand All @@ -27,7 +27,7 @@ describe("unittests:: tsc:: incremental::", () => {
fs: () =>
loadProjectFromFiles({
"/src/project/src/main.ts": "export const x = 10;",
"/src/project/tsconfig.json": Utils.dedent`
"/src/project/tsconfig.json": dedent`
{
"compilerOptions": {
"target": "es5",
Expand All @@ -48,7 +48,7 @@ describe("unittests:: tsc:: incremental::", () => {
fs: () =>
loadProjectFromFiles({
"/src/project/src/main.ts": "export const x = 10;",
"/src/project/tsconfig.json": Utils.dedent`
"/src/project/tsconfig.json": dedent`
{
"compilerOptions": {
"incremental": true,
Expand Down Expand Up @@ -85,7 +85,7 @@ describe("unittests:: tsc:: incremental::", () => {
fs: () =>
loadProjectFromFiles({
"/src/project/src/main.ts": "export const x = 10;",
"/src/project/tsconfig.json": Utils.dedent`
"/src/project/tsconfig.json": dedent`
{
"compilerOptions": {
"incremental": true,
Expand Down Expand Up @@ -115,7 +115,7 @@ describe("unittests:: tsc:: incremental::", () => {
});

describe("with noEmitOnError", () => {
let projFs: vfs.FileSystem;
let projFs: FileSystem;
before(() => {
projFs = getFsForNoEmitOnError();
});
Expand Down Expand Up @@ -275,25 +275,25 @@ const a: string = 10;`,

function fs() {
return loadProjectFromFiles({
"/src/project/src/class.ts": Utils.dedent`
"/src/project/src/class.ts": dedent`
export class classC {
prop = 1;
}`,
"/src/project/src/indirectClass.ts": Utils.dedent`
"/src/project/src/indirectClass.ts": dedent`
import { classC } from './class';
export class indirectClass {
classC = new classC();
}`,
"/src/project/src/directUse.ts": Utils.dedent`
"/src/project/src/directUse.ts": dedent`
import { indirectClass } from './indirectClass';
new indirectClass().classC.prop;`,
"/src/project/src/indirectUse.ts": Utils.dedent`
"/src/project/src/indirectUse.ts": dedent`
import { indirectClass } from './indirectClass';
new indirectClass().classC.prop;`,
"/src/project/src/noChangeFile.ts": Utils.dedent`
"/src/project/src/noChangeFile.ts": dedent`
export function writeLog(s: string) {
}`,
"/src/project/src/noChangeFileWithEmitSpecificError.ts": Utils.dedent`
"/src/project/src/noChangeFileWithEmitSpecificError.ts": dedent`
function someFunc(arguments: boolean, ...rest: any[]) {
}`,
"/src/project/tsconfig.json": jsonToReadableText({ compilerOptions }),
Expand All @@ -307,12 +307,12 @@ const a: string = 10;`,
subScenario: `when global file is added, the signatures are updated`,
fs: () =>
loadProjectFromFiles({
"/src/project/src/main.ts": Utils.dedent`
"/src/project/src/main.ts": dedent`
/// <reference path="./filePresent.ts"/>
/// <reference path="./fileNotFound.ts"/>
function main() { }
`,
"/src/project/src/anotherFileWithSameReferenes.ts": Utils.dedent`
"/src/project/src/anotherFileWithSameReferenes.ts": dedent`
/// <reference path="./filePresent.ts"/>
/// <reference path="./fileNotFound.ts"/>
function anotherFileWithSameReferenes() { }
Expand Down Expand Up @@ -495,7 +495,7 @@ declare global {
module: "esnext",
},
}),
"/src/project/index.tsx": Utils.dedent`
"/src/project/index.tsx": dedent`
declare namespace JSX {
interface ElementChildrenAttribute { children: {}; }
interface IntrinsicElements { div: {} }
Expand All @@ -518,7 +518,7 @@ declare global {
subScenario: "ts file with no-default-lib that augments the global scope",
fs: () =>
loadProjectFromFiles({
"/src/project/src/main.ts": Utils.dedent`
"/src/project/src/main.ts": dedent`
/// <reference no-default-lib="true"/>
/// <reference lib="esnext" />

Expand All @@ -529,7 +529,7 @@ declare global {

export {};
`,
"/src/project/tsconfig.json": Utils.dedent`
"/src/project/tsconfig.json": dedent`
{
"compilerOptions": {
"target": "ESNext",
Expand Down Expand Up @@ -590,12 +590,12 @@ console.log(a);`,
fs: () =>
loadProjectFromFiles({
"/src/project/tsconfig.json": jsonToReadableText({ compilerOptions: { declaration } }),
"/src/project/main.ts": Utils.dedent`
"/src/project/main.ts": dedent`
import MessageablePerson from './MessageablePerson.js';
function logMessage( person: MessageablePerson ) {
console.log( person.message );
}`,
"/src/project/MessageablePerson.ts": Utils.dedent`
"/src/project/MessageablePerson.ts": dedent`
const Messageable = () => {
return class MessageableClass {
public message = 'hello';
Expand All @@ -609,7 +609,7 @@ console.log(a);`,
appendText(
fs,
"/lib/lib.d.ts",
Utils.dedent`
dedent`
type ReturnType<T extends (...args: any) => any> = T extends (...args: any) => infer R ? R : any;
type InstanceType<T extends abstract new (...args: any) => any> = T extends abstract new (...args: any) => infer R ? R : any;`,
),
Expand Down Expand Up @@ -922,12 +922,12 @@ console.log(a);`,
},
include: ["src"],
}),
"/src/project/src/box.ts": Utils.dedent`
"/src/project/src/box.ts": dedent`
export interface Box<T> {
unbox(): T
}
`,
"/src/project/src/bug.js": Utils.dedent`
"/src/project/src/bug.js": dedent`
import * as B from "./box.js"
import * as W from "./wrap.js"

Expand All @@ -947,7 +947,7 @@ console.log(a);`,

export const bug = wrap({ n: box(1) });
`,
"/src/project/src/wrap.ts": Utils.dedent`
"/src/project/src/wrap.ts": dedent`
export type Wrap<C> = {
[K in keyof C]: { wrapped: C[K] }
}
Expand All @@ -974,14 +974,14 @@ console.log(a);`,
skipDefaultLibCheck: true,
},
}),
"/src/project/index.ts": Utils.dedent`
"/src/project/index.ts": dedent`
import ky from 'ky';
export const api = ky.extend({});
`,
"/src/project/package.json": jsonToReadableText({
type: "module",
}),
"/src/project/node_modules/ky/distribution/index.d.ts": Utils.dedent`
"/src/project/node_modules/ky/distribution/index.d.ts": dedent`
type KyInstance = {
extend(options: Record<string,unknown>): KyInstance;
}
Expand All @@ -1003,4 +1003,79 @@ console.log(a);`,
},
],
});

describe("with const enums", () => {
enum AliasType {
None = "",
SameFile = "aliased ",
DifferentFile = "aliased in different file ",
}
function fileWithEnum(withAlias: AliasType) {
return withAlias !== AliasType.DifferentFile ? "/src/project/b.d.ts" : "/src/project/worker.d.ts";
}
function verify(withAlias: AliasType, preserveConstEnums: boolean) {
verifyTsc({
scenario: "incremental",
subScenario: `with ${withAlias}const enums${preserveConstEnums ? " with preserveConstEnums" : ""}`,
commandLineArgs: ["-i", `/src/project/a.ts`, "--tsbuildinfofile", "/src/project/a.tsbuildinfo", ...preserveConstEnums ? ["--preserveConstEnums"] : []],
fs: () =>
loadProjectFromFiles({
"/src/project/a.ts": dedent`
import {A} from "./c"
let a = A.ONE
`,
"/src/project/b.d.ts": withAlias === AliasType.SameFile ?
dedent`
declare const enum AWorker {
ONE = 1
}
export { AWorker as A };
` :
withAlias === AliasType.DifferentFile ?
dedent`
export { AWorker as A } from "./worker";
` :
dedent`
export const enum A {
ONE = 1
}
`,
"/src/project/c.ts": dedent`
import {A} from "./b"
let b = A.ONE
export {A}
`,
"/src/project/worker.d.ts": dedent`
export const enum AWorker {
ONE = 1
}
`,
}),
edits: [
{
caption: "change enum value",
edit: fs => replaceText(fs, fileWithEnum(withAlias), "1", "2"),
},
{
caption: "change enum value again",
edit: fs => replaceText(fs, fileWithEnum(withAlias), "2", "3"),
},
{
caption: "something else changes in b.d.ts",
edit: fs => appendText(fs, "/src/project/b.d.ts", "export const randomThing = 10;"),
},
{
caption: "something else changes in b.d.ts again",
edit: fs => appendText(fs, "/src/project/b.d.ts", "export const randomThing2 = 10;"),
},
],
});
}
verify(/*withAlias*/ AliasType.None, /*preserveConstEnums*/ false);
verify(/*withAlias*/ AliasType.SameFile, /*preserveConstEnums*/ false);
verify(/*withAlias*/ AliasType.DifferentFile, /*preserveConstEnums*/ false);
verify(/*withAlias*/ AliasType.None, /*preserveConstEnums*/ true);
verify(/*withAlias*/ AliasType.SameFile, /*preserveConstEnums*/ true);
verify(/*withAlias*/ AliasType.DifferentFile, /*preserveConstEnums*/ true);
});
});
Loading