Skip to content
Closed
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
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,68 @@ describeBuilder(buildApplication, APPLICATION_BUILDER_INFO, (harness) => {
);
});

it('rebuilds PostCSS stylesheet when a file is added to a plugin directory dependency', async () => {
harness.useTarget('build', {
...BASE_OPTIONS,
watch: true,
styles: ['src/styles.css'],
});

// Emits a class for each source file, similar to Tailwind CSS v4 content scanning
await harness.writeFile(
'test-plugin.js',

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use a unique plugin filename here instead of test-plugin.js. An earlier test in this suite also uses ./test-plugin.js in .postcssrc.json, and StylesheetPluginFactory caches PostCSS processors by config and require.cache, which can cause the wrong plugin to be reused.

`
const fs = require('fs');
const path = require('path');
module.exports = () => {
return {
postcssPlugin: 'test-plugin',
Once(root, { result }) {
const dir = path.join(path.dirname(root.source.input.file), 'sources');
result.messages.push({
type: 'dir-dependency',
dir,
glob: '**/*.txt',
});
for (const file of fs.readdirSync(dir, { recursive: true })) {
if (file.endsWith('.txt')) {
const name = fs.readFileSync(path.join(dir, file), 'utf-8').trim();
root.append('.' + name + ' { color: red; }');
}
}
},
};
};
module.exports.postcss = true;
`,
);
await harness.writeFile(
'.postcssrc.json',
JSON.stringify({
plugins: {
'./test-plugin.js': {},
},
}),
);
await harness.writeFile('src/styles.css', '/* base */');
await harness.writeFile('src/sources/foo/a.txt', 'foo-class');

await harness.executeWithCases([
async ({ result }) => {
expect(result?.success).toBe(true);
harness.expectFile('dist/browser/styles.css').content.toContain('.foo-class');
harness.expectFile('dist/browser/styles.css').content.not.toContain('.bar-class');

await harness.writeFile('src/sources/bar/b.txt', 'bar-class');
},
({ result }) => {
expect(result?.success).toBe(true);
harness.expectFile('dist/browser/styles.css').content.toContain('.foo-class');
harness.expectFile('dist/browser/styles.css').content.toContain('.bar-class');
},
]);
});

it('rebuilds PostCSS stylesheet after CSS syntax error on initial build from import', async () => {
harness.useTarget('build', {
...BASE_OPTIONS,
Expand Down
27 changes: 19 additions & 8 deletions packages/angular/build/src/tools/esbuild/load-result-cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
*/

import type { OnLoadResult, PluginBuild } from 'esbuild';
import { normalize } from 'node:path';
import { dirname, normalize } from 'node:path';

export interface LoadResultCache {
get(path: string): OnLoadResult | Promise<OnLoadResult | undefined> | undefined;
Expand Down Expand Up @@ -108,16 +108,27 @@ export class MemoryLoadResultCache implements LoadResultCache {
}

invalidate(path: string): boolean {
const affectedPaths = this.#fileDependencies.get(path);
if (!affectedPaths) {
return false;
}
let invalid = false;

// Check parent directories to support directory dependencies (PostCSS `dir-dependency`)
let current = normalize(path);
while (true) {
const affectedPaths = this.#fileDependencies.get(current);
if (affectedPaths) {
for (const affected of affectedPaths) {
this.#loadResults.delete(affected);
}
invalid = true;
}

for (const affected of affectedPaths) {
this.#loadResults.delete(affected);
const parent = dirname(current);
if (parent === current) {
break;
}
current = parent;
}

return true;
return invalid;
}
Comment on lines +111 to 132

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Walking up dirname() to the filesystem root on every invalidate() call turns an O(1) lookup into an O(depth) walk for every changed file across all caches (including SourceFileCache and ComponentStylesheetBundler).

Also, after recent changes on main, BundlerContext.invalidate() checks !this.watchFiles.isDisjointFrom(candidateFiles) rather than the return value of this.#loadCache.invalidate(file). When a new file is created inside a watched directory, candidateFiles only has the new file path while this.watchFiles only has the directory path, so this.#esbuildResult is not cleared and the bundle is not rebuilt.

Lastly, normalize() in put() preserves trailing slashes on directory paths while dirname() strips them, so a dir with a trailing slash will not match.


get watchFiles(): string[] {
Expand Down
16 changes: 16 additions & 0 deletions packages/angular/build/src/tools/esbuild/load-result-cache_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,22 @@ describe('MemoryLoadResultCache', () => {
expect(cache.get('file:/test/styles.css')).toBeUndefined();
});

it('should invalidate cached results when a file inside a dependency directory changes', async () => {
const result = {
contents: 'body { color: red; }',
loader: 'css' as const,
watchFiles: ['/test/styles.css', '/test/src'],
};

await cache.put('file:/test/styles.css', result);

expect(cache.invalidate('/test/other/a.ts')).toBeFalse();
expect(cache.get('file:/test/styles.css')).toBe(result);

expect(cache.invalidate('/test/src/app/new/a.ts')).toBeTrue();
expect(cache.get('file:/test/styles.css')).toBeUndefined();
});

it('should preserve previous watch files when caching an error result', async () => {
const successResult = {
contents: 'body { color: red; }',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -401,6 +401,8 @@ async function compileString(
typeof resultMessage['glob'] === 'string'
) {
loadResult.watchFiles ??= [];
// Watch the directory itself so files added or moved into it later also cause a rebuild
loadResult.watchFiles.push(resultMessage['dir']);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding resultMessage['dir'] directly to watchFiles causes a few issues:

  1. watchFiles is intended for file paths rather than directories (for example PersistentLoadResultCache calls readFile on each entry in watchFiles, which fails with EISDIR).
  2. It ignores resultMessage['glob'] during invalidation and watching. When dir is src or the workspace root (such as in Tailwind v4), modifying any file under dir will trigger the watcher and invalidate the stylesheet cache, even for files that do not match glob.
  3. Because glob() is still executed below, all existing files and the parent directory are both added to watchFiles.

const dependencies = await glob(resultMessage['glob'], {
absolute: true,
cwd: resultMessage['dir'],
Expand Down
Loading