Skip to content
Open
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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

These appear to be general correctness fixes. Can we move those to a separate change to better track and to avoid incidental reverts.

Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ export class ComponentStylesheetBundler {
const filename = secondSemi !== -1 ? entry.slice(secondSemi + 1) : '';
if (filename && normalizedFiles.has(path.normalize(filename))) {
this.#inlineContexts.delete(entry);
void bundler.dispose();
void bundler.dispose().catch(() => {});
} else {
bundler.invalidate(normalizedFiles);
}
Expand All @@ -229,10 +229,13 @@ export class ComponentStylesheetBundler {
}

collectReferencedFiles(): string[] {
const files = [];
const files: string[] = [];
for (const context of this.#fileContexts.values()) {
files.push(...context.watchFiles);
}
for (const context of this.#inlineContexts.values()) {
files.push(...context.watchFiles);
}

return files;
}
Expand Down

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Same as above. These also look like correctness fixes

Original file line number Diff line number Diff line change
Expand Up @@ -141,8 +141,8 @@ export function createBrowserPolyfillBundleOptions(
buildOptions.plugins ??= [];
const pluginOptions = createCompilerPluginOptions(
options,

sourceFileCache,
sourceFileCache.loadResultCache,
);
buildOptions.plugins.push(
createCompilerPlugin(
Expand Down Expand Up @@ -501,7 +501,7 @@ export function createSsrEntryCodeBundleOptions(
// The below is needed to avoid
// `Import "default" will always be undefined because there is no matching export` warning when no default is present.
`const defaultExportName = 'default';`,
`export default server[defaultExportName]`,
`export default server[defaultExportName];`,

// Add @angular/ssr exports
`export { AngularAppEngine } from '@angular/ssr';`,
Expand Down Expand Up @@ -764,7 +764,7 @@ function getEsBuildCommonPolyfillsOptions(
}

function entryFileToWorkspaceRelative(workspaceRoot: string, entryFile: string): string {
return './' + toPosixPath(relative(workspaceRoot, entryFile).replace(/.[mc]?ts$/, ''));
return './' + toPosixPath(relative(workspaceRoot, entryFile).replace(/\.[mc]?ts$/, ''));
}

/**
Expand Down
18 changes: 11 additions & 7 deletions packages/angular/build/src/tools/esbuild/bundler-context.ts

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Same as above. These also look like correctness fixes.

Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ export class BundlerContext {
#optionsFactory: BundlerOptionsFactory<BuildOptions & { metafile: true; write: false }>;
#shouldCacheResult: boolean;
#loadCache?: LoadResultCache;
#invalidationEpoch = 0;
readonly watchFiles = new Set<string>();

constructor(
Expand Down Expand Up @@ -128,8 +129,8 @@ export class BundlerContext {
const externalImportsBrowser = new Set<string>();
const externalImportsServer = new Set<string>();

const outputFiles = [];
let externalConfiguration;
const outputFiles: BuildOutputFile[] = [];
let externalConfiguration: Set<string> | undefined;
for (const result of results) {
warnings.push(...result.warnings);
if (result.errors) {
Expand Down Expand Up @@ -202,6 +203,7 @@ export class BundlerContext {
return this.#activeBundlePromise;
}

const bundleEpoch = this.#invalidationEpoch;
const bundlePromise = this.#performBundle().finally(() => {
if (this.#activeBundlePromise === bundlePromise) {
this.#activeBundlePromise = undefined;
Expand All @@ -210,7 +212,7 @@ export class BundlerContext {
this.#activeBundlePromise = bundlePromise;

const result = await bundlePromise;
if (this.#shouldCacheResult) {
if (this.#shouldCacheResult && bundleEpoch === this.#invalidationEpoch) {
this.#esbuildResult = result;
}

Expand Down Expand Up @@ -286,9 +288,10 @@ export class BundlerContext {
}

if (this.#loadCache) {
const cachedLoad = await (this.#loadCache.get(input) ??
this.#loadCache.get(input.replace(';', ':')) ??
this.#loadCache.get('file:' + normalizedAbsoluteInput));
const cachedLoad =
(await this.#loadCache.get(input)) ??
(await this.#loadCache.get(input.replace(';', ':'))) ??
(await this.#loadCache.get('file:' + normalizedAbsoluteInput));
if (cachedLoad?.watchFiles) {
for (const file of cachedLoad.watchFiles) {
if (!isInternalAngularFile(file)) {
Expand Down Expand Up @@ -551,6 +554,7 @@ export class BundlerContext {
}

if (invalid) {
this.#invalidationEpoch++;
this.#esbuildResult = undefined;
}

Expand Down Expand Up @@ -582,7 +586,7 @@ function isInternalAngularFile(file: string) {

function isInternalBundlerFile(file: string) {
// Bundler virtual files such as "<define:???>" or "<runtime>"
if (file[0] === '<' && file.at(-1) === '>') {
if (file.startsWith('<') && file.endsWith('>')) {
return true;
}

Expand Down
Loading