Skip to content

Commit

Permalink
fix(@angular-devkit/build-angular): use component style load result c…
Browse files Browse the repository at this point in the history
…aching information for file watching

When using the esbuild-based builders (`application`/`browser-esbuild`) in watch mode including `ng serve`,
component stylesheets that used Sass and imported other stylesheets were previously no properly tracked.
As a result, changes to the imported stylesheets would not invalidate the component and the rebuild would
not contain the updated styles. The files used by the Sass (and Less) stylesheet processing are now correctly
tracked in these cases.

(cherry picked from commit 7229a3d)
  • Loading branch information
clydin committed Oct 23, 2023
1 parent 254a680 commit ca4d163
Show file tree
Hide file tree
Showing 5 changed files with 185 additions and 40 deletions.
@@ -0,0 +1,77 @@
/**
* @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 { concatMap, count, timeout } from 'rxjs';
import { buildApplication } from '../../index';
import { APPLICATION_BUILDER_INFO, BASE_OPTIONS, describeBuilder } from '../setup';

/**
* Maximum time in milliseconds for single build/rebuild
* This accounts for CI variability.
*/
export const BUILD_TIMEOUT = 30_000;

describeBuilder(buildApplication, APPLICATION_BUILDER_INFO, (harness) => {
describe('Behavior: "Rebuilds when component stylesheets change"', () => {
it('updates component when imported sass changes', async () => {
harness.useTarget('build', {
...BASE_OPTIONS,
watch: true,
});

await harness.modifyFile('src/app/app.component.ts', (content) =>
content.replace('app.component.css', 'app.component.scss'),
);
await harness.writeFile('src/app/app.component.scss', "@import './a';");
await harness.writeFile('src/app/a.scss', '$primary: aqua;\\nh1 { color: $primary; }');

const builderAbort = new AbortController();
const buildCount = await harness
.execute({ signal: builderAbort.signal })
.pipe(
timeout(30000),
concatMap(async ({ result }, index) => {
expect(result?.success).toBe(true);

switch (index) {
case 0:
harness.expectFile('dist/browser/main.js').content.toContain('color: aqua');
harness.expectFile('dist/browser/main.js').content.not.toContain('color: blue');

await harness.writeFile(
'src/app/a.scss',
'$primary: blue;\\nh1 { color: $primary; }',
);
break;
case 1:
harness.expectFile('dist/browser/main.js').content.not.toContain('color: aqua');
harness.expectFile('dist/browser/main.js').content.toContain('color: blue');

await harness.writeFile(
'src/app/a.scss',
'$primary: green;\\nh1 { color: $primary; }',
);
break;
case 2:
harness.expectFile('dist/browser/main.js').content.not.toContain('color: aqua');
harness.expectFile('dist/browser/main.js').content.not.toContain('color: blue');
harness.expectFile('dist/browser/main.js').content.toContain('color: green');

// Test complete - abort watch mode
builderAbort.abort();
break;
}
}),
count(),
)
.toPromise();

expect(buildCount).toBe(3);
});
});
});
Expand Up @@ -98,7 +98,6 @@ export function createCompilerPlugin(
const stylesheetBundler = new ComponentStylesheetBundler(
styleOptions,
pluginOptions.incremental,
pluginOptions.loadResultCache,
);
let sharedTSCompilationState: SharedTSCompilationState | undefined;

Expand Down Expand Up @@ -131,6 +130,7 @@ export function createCompilerPlugin(
// TODO: Differentiate between changed input files and stale output files
modifiedFiles = referencedFileTracker.update(pluginOptions.sourceFileCache.modifiedFiles);
pluginOptions.sourceFileCache.invalidate(modifiedFiles);
stylesheetBundler.invalidate(modifiedFiles);
}

if (
Expand Down
Expand Up @@ -10,7 +10,6 @@ import { OutputFile } from 'esbuild';
import { createHash } from 'node:crypto';
import path from 'node:path';
import { BuildOutputFileType, BundleContextResult, BundlerContext } from '../bundler-context';
import { LoadResultCache } from '../load-result-cache';
import {
BundleStylesheetOptions,
createStylesheetBundleOptions,
Expand Down Expand Up @@ -46,15 +45,16 @@ export class ComponentStylesheetBundler {
constructor(
private readonly options: BundleStylesheetOptions,
private readonly incremental: boolean,
private readonly cache?: LoadResultCache,
) {}

async bundleFile(entry: string) {
const bundlerContext = this.#fileContexts.getOrCreate(entry, () => {
const buildOptions = createStylesheetBundleOptions(this.options, this.cache);
buildOptions.entryPoints = [entry];
return new BundlerContext(this.options.workspaceRoot, this.incremental, (loadCache) => {
const buildOptions = createStylesheetBundleOptions(this.options, loadCache);
buildOptions.entryPoints = [entry];

return new BundlerContext(this.options.workspaceRoot, this.incremental, buildOptions);
return buildOptions;
});
});

return extractResult(await bundlerContext.bundle(), bundlerContext.watchFiles);
Expand All @@ -69,40 +69,56 @@ export class ComponentStylesheetBundler {

const bundlerContext = this.#inlineContexts.getOrCreate(entry, () => {
const namespace = 'angular:styles/component';
const buildOptions = createStylesheetBundleOptions(this.options, this.cache, {
[entry]: data,
});
buildOptions.entryPoints = [`${namespace};${entry}`];
buildOptions.plugins.push({
name: 'angular-component-styles',
setup(build) {
build.onResolve({ filter: /^angular:styles\/component;/ }, (args) => {
if (args.kind !== 'entry-point') {
return null;
}

return {
path: entry,
namespace,
};
});
build.onLoad({ filter: /^css;/, namespace }, async () => {
return {
contents: data,
loader: 'css',
resolveDir: path.dirname(filename),
};
});
},
});

return new BundlerContext(this.options.workspaceRoot, this.incremental, buildOptions);
return new BundlerContext(this.options.workspaceRoot, this.incremental, (loadCache) => {
const buildOptions = createStylesheetBundleOptions(this.options, loadCache, {
[entry]: data,
});
buildOptions.entryPoints = [`${namespace};${entry}`];
buildOptions.plugins.push({
name: 'angular-component-styles',
setup(build) {
build.onResolve({ filter: /^angular:styles\/component;/ }, (args) => {
if (args.kind !== 'entry-point') {
return null;
}

return {
path: entry,
namespace,
};
});
build.onLoad({ filter: /^css;/, namespace }, async () => {
return {
contents: data,
loader: 'css',
resolveDir: path.dirname(filename),
};
});
},
});

return buildOptions;
});
});

// Extract the result of the bundling from the output files
return extractResult(await bundlerContext.bundle(), bundlerContext.watchFiles);
}

invalidate(files: Iterable<string>) {
if (!this.incremental) {
return;
}

for (const bundler of this.#fileContexts.values()) {
bundler.invalidate(files);
}
for (const bundler of this.#inlineContexts.values()) {
bundler.invalidate(files);
}
}

async dispose(): Promise<void> {
const contexts = [...this.#fileContexts.values(), ...this.#inlineContexts.values()];
this.#fileContexts.clear();
Expand Down
Expand Up @@ -17,6 +17,7 @@ import {
context,
} from 'esbuild';
import { basename, dirname, extname, join, relative } from 'node:path';
import { LoadResultCache, MemoryLoadResultCache } from './load-result-cache';
import { convertOutputFile } from './utils';

export type BundleContextResult =
Expand Down Expand Up @@ -49,6 +50,10 @@ export interface BuildOutputFile extends OutputFile {
clone: () => BuildOutputFile;
}

export type BundlerOptionsFactory<T extends BuildOptions = BuildOptions> = (
loadCache: LoadResultCache | undefined,
) => T;

/**
* Determines if an unknown value is an esbuild BuildFailure error object thrown by esbuild.
* @param value A potential esbuild BuildFailure error object.
Expand All @@ -60,20 +65,26 @@ function isEsBuildFailure(value: unknown): value is BuildFailure {

export class BundlerContext {
#esbuildContext?: BuildContext<{ metafile: true; write: false }>;
#esbuildOptions: BuildOptions & { metafile: true; write: false };
#esbuildOptions?: BuildOptions & { metafile: true; write: false };
#optionsFactory: BundlerOptionsFactory<BuildOptions & { metafile: true; write: false }>;

#loadCache?: MemoryLoadResultCache;
readonly watchFiles = new Set<string>();

constructor(
private workspaceRoot: string,
private incremental: boolean,
options: BuildOptions,
options: BuildOptions | BundlerOptionsFactory,
private initialFilter?: (initial: Readonly<InitialFileRecord>) => boolean,
) {
this.#esbuildOptions = {
...options,
metafile: true,
write: false,
this.#optionsFactory = (...args) => {
const baseOptions = typeof options === 'function' ? options(...args) : options;

return {
...baseOptions,
metafile: true,
write: false,
};
};
}

Expand Down Expand Up @@ -131,6 +142,14 @@ export class BundlerContext {
* warnings and errors for the attempted build.
*/
async bundle(): Promise<BundleContextResult> {
// Create esbuild options if not present
if (this.#esbuildOptions === undefined) {
if (this.incremental) {
this.#loadCache = new MemoryLoadResultCache();
}
this.#esbuildOptions = this.#optionsFactory(this.#loadCache);
}

let result;
try {
if (this.#esbuildContext) {
Expand Down Expand Up @@ -162,7 +181,15 @@ export class BundlerContext {
// Add input files except virtual angular files which do not exist on disk
Object.keys(result.metafile.inputs)
.filter((input) => !input.startsWith('angular:'))
// input file paths are always relative to the workspace root
.forEach((input) => this.watchFiles.add(join(this.workspaceRoot, input)));
// Also add any files from the load result cache
if (this.#loadCache) {
this.#loadCache.watchFiles
.filter((file) => !file.startsWith('angular:'))
// watch files are fully resolved paths
.forEach((file) => this.watchFiles.add(file));
}
}

// Return if the build encountered any errors
Expand Down Expand Up @@ -254,14 +281,34 @@ export class BundlerContext {
};
}

invalidate(files: Iterable<string>): boolean {
if (!this.incremental) {
return false;
}

let invalid = false;
for (const file of files) {
if (this.#loadCache?.invalidate(file)) {
invalid = true;
continue;
}

invalid ||= this.watchFiles.has(file);
}

return invalid;
}

/**
* Disposes incremental build resources present in the context.
*
* @returns A promise that resolves when disposal is complete.
*/
async dispose(): Promise<void> {
try {
return this.#esbuildContext?.dispose();
this.#esbuildOptions = undefined;
this.#loadCache = undefined;
await this.#esbuildContext?.dispose();
} finally {
this.#esbuildContext = undefined;
}
Expand Down
Expand Up @@ -12,6 +12,7 @@ import { normalize } from 'node:path';
export interface LoadResultCache {
get(path: string): OnLoadResult | undefined;
put(path: string, result: OnLoadResult): Promise<void>;
readonly watchFiles: ReadonlyArray<string>;
}

export function createCachedLoad(
Expand Down Expand Up @@ -76,4 +77,8 @@ export class MemoryLoadResultCache implements LoadResultCache {

return found;
}

get watchFiles(): string[] {
return [...this.#loadResults.keys(), ...this.#fileDependencies.keys()];
}
}

0 comments on commit ca4d163

Please sign in to comment.