Skip to content

Commit

Permalink
fix(@angular-devkit/build-angular): cache loading of component resour…
Browse files Browse the repository at this point in the history 
…ces in JIT mode

The load result caching capabilities of the Angular compiler plugin used within the
`application` and `browser-esbuild` builders is now used for both stylesheet and
template component resources when building in JIT mode. This limits the amount of
file system access required during a rebuild in JIT mode and also more accurately
captures the full set of watched files.

(cherry picked from commit 12f4433)
  • Loading branch information
@clydin @alan-agius4
clydin authored and alan-agius4 committed Dec 12, 2023
1 parent 7b8d6cd commit 385eb77
Show file tree
Hide file tree
Showing 3 changed files with 107 additions and 88 deletions.
103 changes: 55 additions & 48 deletions ...it/build_angular/src/builders/application/tests/behavior/rebuild-component_styles_spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,60 +18,67 @@ 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,
});
for (const aot of [true, false]) {
it(`updates component when imported sass changes with ${aot ? 'AOT' : 'JIT'}`, async () => {
harness.useTarget('build', {
...BASE_OPTIONS,
watch: true,
aot,
});
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; }');
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);
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');
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: 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');
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();
// Test complete - abort watch mode
builderAbort.abort();
break;
}
}),
count(),
)
.toPromise();

expect(buildCount).toBe(3);
});
expect(buildCount).toBe(3);
});
}
});
});
1 change: 1 addition & 0 deletions packages/angular_devkit/build_angular/src/tools/esbuild/angular/compiler-plugin.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -409,6 +409,7 @@ export function createCompilerPlugin(
stylesheetBundler,
additionalResults,
styleOptions.inlineStyleLanguage,
pluginOptions.loadResultCache,
);
}

Expand Down
91 changes: 51 additions & 40 deletions packages/angular_devkit/build_angular/src/tools/esbuild/angular/jit-plugin-callbacks.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@

import type { Metafile, OutputFile, PluginBuild } from 'esbuild';
import { readFile } from 'node:fs/promises';
import path from 'node:path';
import { dirname, join, relative } from 'node:path';
import { LoadResultCache, createCachedLoad } from '../load-result-cache';
import { ComponentStylesheetBundler } from './component-stylesheets';
import {
JIT_NAMESPACE_REGEXP,
Expand All @@ -34,7 +35,7 @@ async function loadEntry(
skipRead?: boolean,
): Promise<{ path: string; contents?: string }> {
if (entry.startsWith('file:')) {
const specifier = path.join(root, entry.slice(5));
const specifier = join(root, entry.slice(5));

return {
path: specifier,
Expand All @@ -44,7 +45,7 @@ async function loadEntry(
const [importer, data] = entry.slice(7).split(';', 2);

return {
path: path.join(root, importer),
path: join(root, importer),
contents: Buffer.from(data, 'base64').toString(),
};
} else {
Expand All @@ -66,6 +67,7 @@ export function setupJitPluginCallbacks(
stylesheetBundler: ComponentStylesheetBundler,
additionalResultFiles: Map<string, { outputFiles?: OutputFile[]; metafile?: Metafile }>,
inlineStyleLanguage: string,
loadCache?: LoadResultCache,
): void {
const root = build.initialOptions.absWorkingDir ?? '';

Expand All @@ -84,12 +86,12 @@ export function setupJitPluginCallbacks(
return {
// Use a relative path to prevent fully resolved paths in the metafile (JSON stats file).
// This is only necessary for custom namespaces. esbuild will handle the file namespace.
path: 'file:' + path.relative(root, path.join(path.dirname(args.importer), specifier)),
path: 'file:' + relative(root, join(dirname(args.importer), specifier)),
namespace,
};
} else {
// Inline data may need the importer to resolve imports/references within the content
const importer = path.relative(root, args.importer);
const importer = relative(root, args.importer);

return {
path: `inline:${importer};${specifier}`,
Expand All @@ -99,45 +101,54 @@ export function setupJitPluginCallbacks(
});

// Add a load callback to handle Component stylesheets (both inline and external)
build.onLoad({ filter: /./, namespace: JIT_STYLE_NAMESPACE }, async (args) => {
// skipRead is used here because the stylesheet bundling will read a file stylesheet
// directly either via a preprocessor or esbuild itself.
const entry = await loadEntry(args.path, root, true /* skipRead */);
build.onLoad(
{ filter: /./, namespace: JIT_STYLE_NAMESPACE },
createCachedLoad(loadCache, async (args) => {
// skipRead is used here because the stylesheet bundling will read a file stylesheet
// directly either via a preprocessor or esbuild itself.
const entry = await loadEntry(args.path, root, true /* skipRead */);

let stylesheetResult;

// Stylesheet contents only exist for internal stylesheets
if (entry.contents === undefined) {
stylesheetResult = await stylesheetBundler.bundleFile(entry.path);
} else {
stylesheetResult = await stylesheetBundler.bundleInline(
entry.contents,
entry.path,
inlineStyleLanguage,
);
}

const { contents, resourceFiles, errors, warnings, metafile, referencedFiles } =
stylesheetResult;

additionalResultFiles.set(entry.path, { outputFiles: resourceFiles, metafile });

let stylesheetResult;

// Stylesheet contents only exist for internal stylesheets
if (entry.contents === undefined) {
stylesheetResult = await stylesheetBundler.bundleFile(entry.path);
} else {
stylesheetResult = await stylesheetBundler.bundleInline(
entry.contents,
entry.path,
inlineStyleLanguage,
);
}

const { contents, resourceFiles, errors, warnings, metafile } = stylesheetResult;

additionalResultFiles.set(entry.path, { outputFiles: resourceFiles, metafile });

return {
errors,
warnings,
contents,
loader: 'text',
};
});
return {
errors,
warnings,
contents,
loader: 'text',
watchFiles: referencedFiles && [...referencedFiles],
};
}),
);

// Add a load callback to handle Component templates
// NOTE: While this callback supports both inline and external templates, the transformer
// currently only supports generating URIs for external templates.
build.onLoad({ filter: /./, namespace: JIT_TEMPLATE_NAMESPACE }, async (args) => {
const { contents } = await loadEntry(args.path, root);
build.onLoad(
{ filter: /./, namespace: JIT_TEMPLATE_NAMESPACE },
createCachedLoad(loadCache, async (args) => {
const { contents, path } = await loadEntry(args.path, root);

return {
contents,
loader: 'text',
};
});
return {
contents,
loader: 'text',
watchFiles: [path],
};
}),
);
}

0 comments on commit 385eb77

Please sign in to comment.