Skip to content

Commit

Permalink
fix(@angular/build): minimize reliance on esbuild inject to prevent…
Browse files Browse the repository at this point in the history
… code reordering

Resolved an issue where the use of `esbuild`'s `inject` feature caused incorrect reordering of class structures during bundling. This reordering affected extended classes, as illustrated below:

```js
class e extends Ur {
  constructor(n, r, i) {
    super(n, r, i);
  }
  ngOnDestroy() {
    this.flush();
  }
  static ɵfac = function (r) {
    return new (r || e)(pe(Xe), pe(Ti), pe(Di));
  };
  static ɵprov = oe({ token: e, factory: e.ɵfac });
}

var Ur = class {
  // Class properties and methods omitted for brevity
};
```

By reducing the reliance on `inject`, we ensure that the ordering of class properties and methods remains consistent, preserving the expected behavior.

Closes #28941

(cherry picked from commit 8f9fc59)
  • Loading branch information
alan-agius4 authored and clydin committed Nov 25, 2024
1 parent 5ac03f4 commit f9da163
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 46 deletions.
74 changes: 28 additions & 46 deletions packages/angular/build/src/tools/esbuild/application-code-bundle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -254,9 +254,7 @@ export function createServerMainCodeBundleOptions(

return (loadResultCache) => {
const pluginOptions = createCompilerPluginOptions(options, sourceFileCache, loadResultCache);

const mainServerNamespace = 'angular:main-server';
const mainServerInjectPolyfillsNamespace = 'angular:main-server-inject-polyfills';
const mainServerInjectManifestNamespace = 'angular:main-server-inject-manifest';
const zoneless = isZonelessApp(polyfills);
const entryPoints: Record<string, string> = {
Expand All @@ -275,7 +273,9 @@ export function createServerMainCodeBundleOptions(
const buildOptions: BuildOptions = {
...getEsBuildServerCommonOptions(options),
target,
inject: [mainServerInjectPolyfillsNamespace, mainServerInjectManifestNamespace],
banner: {
js: `import './polyfills.server.mjs';`,
},
entryPoints,
supported: getFeatureSupport(target, zoneless),
plugins: [
Expand Down Expand Up @@ -311,18 +311,10 @@ export function createServerMainCodeBundleOptions(

buildOptions.plugins.push(
createServerBundleMetadata(),
createVirtualModulePlugin({
namespace: mainServerInjectPolyfillsNamespace,
cache: loadResultCache,
loadContent: () => ({
contents: `import './polyfills.server.mjs';`,
loader: 'js',
resolveDir: workspaceRoot,
}),
}),
createVirtualModulePlugin({
namespace: mainServerInjectManifestNamespace,
cache: loadResultCache,
entryPointOnly: false,
loadContent: async () => {
const contents: string[] = [
// Configure `@angular/ssr` manifest.
Expand All @@ -348,16 +340,19 @@ export function createServerMainCodeBundleOptions(
);

const contents: string[] = [
// Re-export all symbols including default export from 'main.server.ts'
`export { default } from '${mainServerEntryPointJsImport}';`,
`export * from '${mainServerEntryPointJsImport}';`,
// Inject manifest
`import '${mainServerInjectManifestNamespace}';`,

// Add @angular/ssr exports
`export {
ɵdestroyAngularServerApp,
ɵextractRoutesAndCreateRouteTree,
ɵgetOrCreateAngularServerApp,
} from '@angular/ssr';`,
ɵdestroyAngularServerApp,
ɵextractRoutesAndCreateRouteTree,
ɵgetOrCreateAngularServerApp,
} from '@angular/ssr';`,

// Re-export all symbols including default export from 'main.server.ts'
`export { default } from '${mainServerEntryPointJsImport}';`,
`export * from '${mainServerEntryPointJsImport}';`,
];

return {
Expand Down Expand Up @@ -392,22 +387,24 @@ export function createSsrEntryCodeBundleOptions(

return (loadResultCache) => {
const pluginOptions = createCompilerPluginOptions(options, sourceFileCache, loadResultCache);

const ssrEntryNamespace = 'angular:ssr-entry';
const ssrInjectManifestNamespace = 'angular:ssr-entry-inject-manifest';
const ssrInjectRequireNamespace = 'angular:ssr-entry-inject-require';
const isNodePlatform = options.ssrOptions?.platform !== ExperimentalPlatform.Neutral;

const inject: string[] = [ssrInjectManifestNamespace];
if (isNodePlatform) {
inject.unshift(ssrInjectRequireNamespace);
}

const buildOptions: BuildOptions = {
...getEsBuildServerCommonOptions(options),
target,
banner: isNodePlatform
? {
js: [
// Note: Needed as esbuild does not provide require shims / proxy from ESModules.
// See: https://github.com/evanw/esbuild/issues/1921.
`import { createRequire } from 'node:module';`,
`globalThis['require'] ??= createRequire(import.meta.url);`,
].join('\n'),
}
: undefined,
entryPoints: {
// TODO: consider renaming to index
'server': ssrEntryNamespace,
},
supported: getFeatureSupport(target, true),
Expand All @@ -420,7 +417,6 @@ export function createSsrEntryCodeBundleOptions(
stylesheetBundler,
),
],
inject,
};

buildOptions.plugins ??= [];
Expand All @@ -443,27 +439,10 @@ export function createSsrEntryCodeBundleOptions(

buildOptions.plugins.push(
createServerBundleMetadata({ ssrEntryBundle: true }),
createVirtualModulePlugin({
namespace: ssrInjectRequireNamespace,
cache: loadResultCache,
loadContent: () => {
const contents: string[] = [
// Note: Needed as esbuild does not provide require shims / proxy from ESModules.
// See: https://github.com/evanw/esbuild/issues/1921.
`import { createRequire } from 'node:module';`,
`globalThis['require'] ??= createRequire(import.meta.url);`,
];

return {
contents: contents.join('\n'),
loader: 'js',
resolveDir: workspaceRoot,
};
},
}),
createVirtualModulePlugin({
namespace: ssrInjectManifestNamespace,
cache: loadResultCache,
entryPointOnly: false,
loadContent: () => {
const contents: string[] = [
// Configure `@angular/ssr` app engine manifest.
Expand All @@ -488,6 +467,9 @@ export function createSsrEntryCodeBundleOptions(
serverEntryPoint,
);
const contents: string[] = [
// Configure `@angular/ssr` app engine manifest.
`import '${ssrInjectManifestNamespace}';`,

// Re-export all symbols including default export
`import * as server from '${serverEntryPointJsImport}';`,
`export * from '${serverEntryPointJsImport}';`,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
import { updateJsonFile, useSha } from '../../../utils/project';
import { getGlobalVariable } from '../../../utils/env';
import { findFreePort } from '../../../utils/network';
import { readFile } from 'node:fs/promises';

export default async function () {
assert(
Expand Down Expand Up @@ -68,6 +69,10 @@ export default async function () {
},
];
`,
'src/app/app.config.ts': `
import { provideAnimationsAsync } from '@angular/platform-browser/animations/async';
${(await readFile('src/app/app.config.ts', 'utf8')).replace('provideRouter(routes),', 'provideAnimationsAsync(), provideRouter(routes),')}
`,
'src/server.ts': `
import { AngularAppEngine, createRequestHandler } from '@angular/ssr';
import { createApp, createRouter, toWebHandler, defineEventHandler, toWebRequest } from 'h3';
Expand Down

0 comments on commit f9da163

Please sign in to comment.