Skip to content

Commit

Permalink
fix(@angular-devkit/build-angular): improve sharing of TypeScript com…
Browse files Browse the repository at this point in the history
…pilation state between various esbuild instances during rebuilds

This commit improves the logic to block and share a TypeScript results across multiple esbuild instances (browser and server builds) which fixes an issue that previously during rebuilds in some cases did not block the build correctly.
  • Loading branch information
alan-agius4 committed Oct 9, 2023
1 parent 99d9037 commit 8981d8c
Show file tree
Hide file tree
Showing 3 changed files with 153 additions and 16 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
/**
* @license
* Copyright Google LLC All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/

import type { logging } from '@angular-devkit/core';
import { concatMap, count, firstValueFrom, timeout } from 'rxjs';
import { buildApplication } from '../../index';
import { OutputHashing } from '../../schema';
import { APPLICATION_BUILDER_INFO, BASE_OPTIONS, describeBuilder } from '../setup';

describeBuilder(buildApplication, APPLICATION_BUILDER_INFO, (harness) => {
beforeEach(async () => {
await harness.modifyFile('src/tsconfig.app.json', (content) => {
const tsConfig = JSON.parse(content);
tsConfig.files = ['main.server.ts', 'main.ts'];

return JSON.stringify(tsConfig);
});

await harness.writeFiles({
'src/lazy.ts': `export const foo: number = 1;`,
'src/main.ts': `export async function fn () {
const lazy = await import('./lazy');
return lazy.foo;
}`,
'src/main.server.ts': `export { fn as default } from './main';`,
});
});

describe('Behavior: "Rebuild both server and browser bundles when using lazy loading"', () => {
it('detect changes and errors when expected', async () => {
harness.useTarget('build', {
...BASE_OPTIONS,
watch: true,
namedChunks: true,
outputHashing: OutputHashing.None,
server: 'src/main.server.ts',
ssr: true,
});

const builderAbort = new AbortController();
const buildCount = await firstValueFrom(
harness.execute({ outputLogsOnFailure: true, signal: builderAbort.signal }).pipe(
timeout(20_000),
concatMap(async ({ result, logs }, index) => {
switch (index) {
case 0:
expect(result?.success).toBeTrue();

// Add valid code
await harness.appendToFile('src/lazy.ts', `console.log('foo');`);

break;
case 1:
expect(result?.success).toBeTrue();

// Update type of 'foo' to invalid (number -> string)
await harness.writeFile('src/lazy.ts', `export const foo: string = 1;`);

break;
case 2:
expect(result?.success).toBeFalse();
expect(logs).toContain(
jasmine.objectContaining<logging.LogEntry>({
message: jasmine.stringMatching(
`Type 'number' is not assignable to type 'string'.`,
),
}),
);

// Fix TS error
await harness.writeFile('src/lazy.ts', `export const foo: string = "1";`);

break;
case 3:
expect(result?.success).toBeTrue();

builderAbort.abort();
break;
}
}),
count(),
),
);

expect(buildCount).toBe(4);
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
/**
* @license
* Copyright Google LLC All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/

export class SharedTSCompilationState {
#pendingCompilation = true;
#resolveCompilationReady: (() => void) | undefined;
#compilationReadyPromise: Promise<void> | undefined;

get waitUntilReady(): Promise<void> {
if (!this.#pendingCompilation) {
return Promise.resolve();
}

this.#compilationReadyPromise ??= new Promise((resolve) => {
this.#resolveCompilationReady = resolve;
});

return this.#compilationReadyPromise;
}

markAsReady(): void {
this.#resolveCompilationReady?.();
this.#compilationReadyPromise = undefined;
this.#pendingCompilation = false;
}

markAsInProgress(): void {
this.#pendingCompilation = true;
}

dispose(): void {
this.markAsReady();
globalSharedCompilationState = undefined;
}
}

let globalSharedCompilationState: SharedTSCompilationState | undefined;

export function getSharedCompilationState(): SharedTSCompilationState {
return (globalSharedCompilationState ??= new SharedTSCompilationState());
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import {
import { BundleStylesheetOptions } from '../stylesheets/bundle-options';
import { AngularHostOptions } from './angular-host';
import { AngularCompilation, AotCompilation, JitCompilation, NoopCompilation } from './compilation';
import { SharedTSCompilationState, getSharedCompilationState } from './compilation-state';
import { ComponentStylesheetBundler } from './component-stylesheets';
import { setupJitPluginCallbacks } from './jit-plugin-callbacks';
import { SourceFileCache } from './source-file-cache';
Expand All @@ -48,29 +49,18 @@ export interface CompilerPluginOptions {
loadResultCache?: LoadResultCache;
}

// TODO: find a better way to unblock TS compilation of server bundles.
let TS_COMPILATION_READY: Promise<void> | undefined;

// eslint-disable-next-line max-lines-per-function
export function createCompilerPlugin(
pluginOptions: CompilerPluginOptions,
styleOptions: BundleStylesheetOptions & { inlineStyleLanguage: string },
): Plugin {
let resolveCompilationReady: (() => void) | undefined;

if (!pluginOptions.noopTypeScriptCompilation) {
TS_COMPILATION_READY = new Promise<void>((resolve) => {
resolveCompilationReady = resolve;
});
}

return {
name: 'angular-compiler',
// eslint-disable-next-line max-lines-per-function
async setup(build: PluginBuild): Promise<void> {
let setupWarnings: PartialMessage[] | undefined = [];

const preserveSymlinks = build.initialOptions.preserveSymlinks;

let tsconfigPath = pluginOptions.tsconfig;
if (!preserveSymlinks) {
// Use the real path of the tsconfig if not preserving symlinks.
Expand Down Expand Up @@ -112,8 +102,14 @@ export function createCompilerPlugin(
styleOptions,
pluginOptions.loadResultCache,
);
let sharedTSCompilationState: SharedTSCompilationState | undefined;

build.onStart(async () => {
sharedTSCompilationState = getSharedCompilationState();
if (!(compilation instanceof NoopCompilation)) {
sharedTSCompilationState.markAsInProgress();
}

const result: OnStartResult = {
warnings: setupWarnings,
};
Expand Down Expand Up @@ -259,7 +255,7 @@ export function createCompilerPlugin(
shouldTsIgnoreJs = !allowJs;

if (compilation instanceof NoopCompilation) {
await TS_COMPILATION_READY;
await sharedTSCompilationState.waitUntilReady;

return result;
}
Expand Down Expand Up @@ -287,8 +283,7 @@ export function createCompilerPlugin(
// Reset the setup warnings so that they are only shown during the first build.
setupWarnings = undefined;

// TODO: find a better way to unblock TS compilation of server bundles.
resolveCompilationReady?.();
sharedTSCompilationState.markAsReady();

return result;
});
Expand Down Expand Up @@ -388,7 +383,10 @@ export function createCompilerPlugin(
logCumulativeDurations();
});

build.onDispose(() => void stylesheetBundler.dispose());
build.onDispose(() => {
sharedTSCompilationState?.dispose();
void stylesheetBundler.dispose();
});
},
};
}
Expand Down

0 comments on commit 8981d8c

Please sign in to comment.