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

With --isolatedModules, --declaration emit is now allowed and builder handles it for incremental compilation #33380

Merged
merged 3 commits into from
Sep 19, 2019
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
32 changes: 29 additions & 3 deletions src/compiler/builder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -173,8 +173,7 @@ namespace ts {
const compilerOptions = newProgram.getCompilerOptions();
state.compilerOptions = compilerOptions;
// With --out or --outFile, any change affects all semantic diagnostics so no need to cache them
// With --isolatedModules, emitting changed file doesnt emit dependent files so we cant know of dependent files to retrieve errors so dont cache the errors
if (!compilerOptions.outFile && !compilerOptions.out && !compilerOptions.isolatedModules) {
if (!compilerOptions.outFile && !compilerOptions.out) {
state.semanticDiagnosticsPerFile = createMap<readonly Diagnostic[]>();
}
state.changedFilesSet = createMap<true>();
Expand Down Expand Up @@ -483,16 +482,43 @@ namespace ts {
return !state.semanticDiagnosticsFromOldState.size;
}

function isChangedSignagure(state: BuilderProgramState, path: Path) {
const newSignature = Debug.assertDefined(state.currentAffectedFilesSignatures).get(path);
const oldSignagure = Debug.assertDefined(state.fileInfos.get(path)).signature;
return newSignature !== oldSignagure;
}

/**
* Iterate on referencing modules that export entities from affected file
*/
function forEachReferencingModulesOfExportOfAffectedFile(state: BuilderProgramState, affectedFile: SourceFile, fn: (state: BuilderProgramState, filePath: Path) => boolean) {
// If there was change in signature (dts output) for the changed file,
// then only we need to handle pending file emit
if (!state.exportedModulesMap || state.affectedFiles!.length === 1 || !state.changedFilesSet.has(affectedFile.path)) {
if (!state.exportedModulesMap || !state.changedFilesSet.has(affectedFile.path)) {
return;
}

if (!isChangedSignagure(state, affectedFile.path)) return;

// Since isolated modules dont change js files, files affected by change in signature is itself
// But we need to cleanup semantic diagnostics and queue dts emit for affected files
if (state.compilerOptions.isolatedModules) {
const seenFileNamesMap = createMap<true>();
seenFileNamesMap.set(affectedFile.path, true);
const queue = BuilderState.getReferencedByPaths(state, affectedFile.resolvedPath);
while (queue.length > 0) {
const currentPath = queue.pop()!;
if (!seenFileNamesMap.has(currentPath)) {
seenFileNamesMap.set(currentPath, true);
const result = fn(state, currentPath);
if (result && isChangedSignagure(state, currentPath)) {
const currentSourceFile = Debug.assertDefined(state.program).getSourceFileByPath(currentPath)!;
queue.push(...BuilderState.getReferencedByPaths(state, currentSourceFile.resolvedPath));
}
}
}
}

Debug.assert(!!state.currentAffectedFilesExportedModulesMap);
const seenFileAndExportsOfFile = createMap<true>();
// Go through exported modules from cache first
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/builderState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -466,7 +466,7 @@ namespace ts.BuilderState {
/**
* Gets the files referenced by the the file path
*/
function getReferencedByPaths(state: Readonly<BuilderState>, referencedFilePath: Path) {
export function getReferencedByPaths(state: Readonly<BuilderState>, referencedFilePath: Path) {
return arrayFrom(mapDefinedIterator(state.referencedMap!.entries(), ([filePath, referencesInFile]) =>
referencesInFile.has(referencedFilePath) ? filePath as Path : undefined
));
Expand Down
4 changes: 0 additions & 4 deletions src/compiler/program.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2808,10 +2808,6 @@ namespace ts {
}

if (options.isolatedModules) {
if (getEmitDeclarations(options)) {
createDiagnosticForOptionName(Diagnostics.Option_0_cannot_be_specified_with_option_1, getEmitDeclarationOptionName(options), "isolatedModules");
}

if (options.out) {
createDiagnosticForOptionName(Diagnostics.Option_0_cannot_be_specified_with_option_1, "out", "isolatedModules");
}
Expand Down
10 changes: 5 additions & 5 deletions src/testRunner/unittests/tsbuild/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ interface Symbol {
}
}

interface BuildInput {
export interface BuildInput {
fs: vfs.FileSystem;
tick: () => void;
rootNames: readonly string[];
Expand All @@ -239,7 +239,7 @@ interface Symbol {
baselineBuildInfo?: true;
}

function build({ fs, tick, rootNames, modifyFs, baselineSourceMap, baselineBuildInfo }: BuildInput) {
export function tscBuild({ fs, tick, rootNames, modifyFs, baselineSourceMap, baselineBuildInfo }: BuildInput) {
const actualReadFileMap = createMap<number>();
modifyFs(fs);
tick();
Expand Down Expand Up @@ -344,7 +344,7 @@ Mismatch Actual(path, actual, expected): ${JSON.stringify(arrayFrom(mapDefinedIt
let host: fakes.SolutionBuilderHost;
let initialWrittenFiles: Map<true>;
before(() => {
const result = build({
const result = tscBuild({
fs: projFs().shadow(),
tick,
rootNames,
Expand Down Expand Up @@ -390,7 +390,7 @@ Mismatch Actual(path, actual, expected): ${JSON.stringify(arrayFrom(mapDefinedIt
tick();
newFs = fs.shadow();
tick();
({ actualReadFileMap, host } = build({
({ actualReadFileMap, host } = tscBuild({
fs: newFs,
tick,
rootNames,
Expand Down Expand Up @@ -429,7 +429,7 @@ Mismatch Actual(path, actual, expected): ${JSON.stringify(arrayFrom(mapDefinedIt
});
}
it(`Verify emit output file text is same when built clean`, () => {
const { fs, writtenFiles } = build({
const { fs, writtenFiles } = tscBuild({
fs: newFs.shadow(),
tick,
rootNames,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ namespace ts {
]
},
incrementalDtsChangedBuild: {
modifyFs: fs => replaceText(fs, "/src/bar.ts", "param: string", ""),
modifyFs: changeBarParam,
expectedDiagnostics: [
getExpectedDiagnosticForProjectsInBuild("src/tsconfig.json"),
[Diagnostics.Project_0_is_out_of_date_because_oldest_output_1_is_older_than_newest_input_2, "src/tsconfig.json", "src/obj/bar.js", "src/bar.ts"],
Expand All @@ -36,5 +36,55 @@ namespace ts {
baselineOnly: true,
verifyDiagnostics: true
});

verifyTsbuildOutput({
scenario: "inferred type from transitive module with isolatedModules",
projFs: () => projFs,
time,
tick,
proj: "inferredTypeFromTransitiveModule",
rootNames: ["/src"],
initialBuild: { modifyFs: changeToIsolatedModules },
incrementalDtsChangedBuild: { modifyFs: changeBarParam },
baselineOnly: true,
});

it("reports errors in files affected by change in signature", () => {
const { fs, host } = tscBuild({
fs: projFs.shadow(),
tick,
rootNames: ["/src"],
modifyFs: fs => {
changeToIsolatedModules(fs);
appendText(fs, "/src/lazyIndex.ts", `
import { default as bar } from './bar';
bar("hello");`);
}
});
host.assertErrors(/*empty*/);

tick();
const { fs: newFs, host: newHost, writtenFiles } = tscBuild({
fs: fs.shadow(),
tick,
rootNames: ["/src"],
modifyFs: changeBarParam
});
// Has errors
newHost.assertErrors({
message: [Diagnostics.Expected_0_arguments_but_got_1, 0, 1],
location: expectedLocationIndexOf(newFs, "/src/lazyIndex.ts", `"hello"`)
});
// No written files
assert.equal(writtenFiles.size, 0);
});
});

function changeToIsolatedModules(fs: vfs.FileSystem) {
replaceText(fs, "/src/tsconfig.json", `"incremental": true`, `"incremental": true, "isolatedModules": true`);
}

function changeBarParam(fs: vfs.FileSystem) {
replaceText(fs, "/src/bar.ts", "param: string", "");
}
}
Loading