From 7d98ab3df9f7c15612c69cedca5a01a535301508 Mon Sep 17 00:00:00 2001 From: Charles Lyding <19598772+clydin@users.noreply.github.com> Date: Mon, 20 Sep 2021 09:07:16 -0400 Subject: [PATCH] refactor(@ngtools/webpack): support an ESM-only `@angular/compiler-cli` package With the Angular CLI currently being a CommonJS package, this change uses a dynamic import to load @angular/compiler-cli which may be ESM. CommonJS code can load ESM code via a dynamic import. Unfortunately, TypeScript will currently, unconditionally downlevel dynamic import into a require call. require calls cannot load ESM code and will result in a runtime error. To workaround this, a Function constructor is used to prevent TypeScript from changing the dynamic import. Once TypeScript provides support for keeping the dynamic import this workaround can be dropped and replaced with a standard dynamic import. Type only static import statements for `@angular/compiler-cli` are also now used since the `@angular/compiler-cli` is used as a peer dependency and has the potential to not be present. As a result static imports should only be used for types and value imports should be dynamic so that they can be guarded in the event of package absence. BREAKING CHANGE: Applications directly using the `webpack-cli` and not the Angular CLI to build must set the environment variable `DISABLE_V8_COMPILE_CACHE=1`. The `@ngtools/webpack` package now uses dynamic imports to provide support for the ESM `@angular/compiler-cli` package. The `v8-compile-cache` package used by the `webpack-cli` does not currently support dynamic import expressions and will cause builds to fail if the environment variable is not specified. Applications using the Angular CLI are not affected by this limitation. --- .../public-api/ngtools/webpack/src/index.md | 2 +- .../ngtools/webpack/src/ivy/diagnostics.ts | 9 +- packages/ngtools/webpack/src/ivy/host.ts | 2 +- packages/ngtools/webpack/src/ivy/plugin.ts | 86 ++++++++++++++++--- .../ngtools/webpack/src/ivy/transformation.ts | 4 +- .../ngtools/webpack/src/ngcc_processor.ts | 24 ++++-- .../e2e/tests/packages/webpack/test-app.ts | 13 ++- 7 files changed, 112 insertions(+), 28 deletions(-) diff --git a/goldens/public-api/ngtools/webpack/src/index.md b/goldens/public-api/ngtools/webpack/src/index.md index 4f3b212401c4..ed0f9099d782 100644 --- a/goldens/public-api/ngtools/webpack/src/index.md +++ b/goldens/public-api/ngtools/webpack/src/index.md @@ -5,7 +5,7 @@ ```ts import type { Compiler } from 'webpack'; -import { CompilerOptions } from '@angular/compiler-cli'; +import type { CompilerOptions } from '@angular/compiler-cli'; import type { LoaderContext } from 'webpack'; // @public (undocumented) diff --git a/packages/ngtools/webpack/src/ivy/diagnostics.ts b/packages/ngtools/webpack/src/ivy/diagnostics.ts index 76a322c91d23..d344e1133473 100644 --- a/packages/ngtools/webpack/src/ivy/diagnostics.ts +++ b/packages/ngtools/webpack/src/ivy/diagnostics.ts @@ -6,16 +6,19 @@ * found in the LICENSE file at https://angular.io/license */ -import { Diagnostics, formatDiagnostics } from '@angular/compiler-cli'; +import type { Diagnostics } from '@angular/compiler-cli'; import { DiagnosticCategory } from 'typescript'; import type { Compilation } from 'webpack'; export type DiagnosticsReporter = (diagnostics: Diagnostics) => void; -export function createDiagnosticsReporter(compilation: Compilation): DiagnosticsReporter { +export function createDiagnosticsReporter( + compilation: Compilation, + formatter: (diagnostic: Diagnostics[number]) => string, +): DiagnosticsReporter { return (diagnostics) => { for (const diagnostic of diagnostics) { - const text = formatDiagnostics([diagnostic]); + const text = formatter(diagnostic); if (diagnostic.category === DiagnosticCategory.Error) { addError(compilation, text); } else { diff --git a/packages/ngtools/webpack/src/ivy/host.ts b/packages/ngtools/webpack/src/ivy/host.ts index 774807dc24bd..7f52cea16de1 100644 --- a/packages/ngtools/webpack/src/ivy/host.ts +++ b/packages/ngtools/webpack/src/ivy/host.ts @@ -7,7 +7,7 @@ */ /* eslint-disable @typescript-eslint/unbound-method */ -import { CompilerHost } from '@angular/compiler-cli'; +import type { CompilerHost } from '@angular/compiler-cli'; import { createHash } from 'crypto'; import * as path from 'path'; import * as ts from 'typescript'; diff --git a/packages/ngtools/webpack/src/ivy/plugin.ts b/packages/ngtools/webpack/src/ivy/plugin.ts index d31c2073902a..ad3ab58722be 100644 --- a/packages/ngtools/webpack/src/ivy/plugin.ts +++ b/packages/ngtools/webpack/src/ivy/plugin.ts @@ -6,12 +6,8 @@ * found in the LICENSE file at https://angular.io/license */ -import { - CompilerHost, - CompilerOptions, - NgtscProgram, - readConfiguration, -} from '@angular/compiler-cli'; +import type { CompilerHost, CompilerOptions, NgtscProgram } from '@angular/compiler-cli'; +import { strict as assert } from 'assert'; import { createHash } from 'crypto'; import * as ts from 'typescript'; import type { Compilation, Compiler, Module, NormalModule } from 'webpack'; @@ -61,6 +57,7 @@ export interface AngularWebpackPluginOptions { function initializeNgccProcessor( compiler: Compiler, tsconfig: string, + compilerNgccModule: typeof import('@angular/compiler-cli/ngcc') | undefined, ): { processor: NgccProcessor; errors: string[]; warnings: string[] } { const { inputFileSystem, options: webpackOptions } = compiler; const mainFields = webpackOptions.resolve?.mainFields?.flat() ?? []; @@ -73,7 +70,14 @@ function initializeNgccProcessor( extensions: ['.json'], useSyncFileSystemCalls: true, }); + + // The compilerNgccModule field is guaranteed to be defined during a compilation + // due to the `beforeCompile` hook. Usage of this property accessor prior to the + // hook execution is an implementation error. + assert.ok(compilerNgccModule, `'@angular/compiler-cli/ngcc' used prior to Webpack compilation.`); + const processor = new NgccProcessor( + compilerNgccModule, mainFields, warnings, errors, @@ -95,6 +99,8 @@ const compilationFileEmitters = new WeakMap( export class AngularWebpackPlugin { private readonly pluginOptions: AngularWebpackPluginOptions; + private compilerCliModule?: typeof import('@angular/compiler-cli'); + private compilerNgccModule?: typeof import('@angular/compiler-cli/ngcc'); private watchMode?: boolean; private ngtscNextProgram?: NgtscProgram; private builder?: ts.EmitAndSemanticDiagnosticsBuilderProgram; @@ -117,6 +123,15 @@ export class AngularWebpackPlugin { }; } + private get compilerCli(): typeof import('@angular/compiler-cli') { + // The compilerCliModule field is guaranteed to be defined during a compilation + // due to the `beforeCompile` hook. Usage of this property accessor prior to the + // hook execution is an implementation error. + assert.ok(this.compilerCliModule, `'@angular/compiler-cli' used prior to Webpack compilation.`); + + return this.compilerCliModule; + } + get options(): AngularWebpackPluginOptions { return this.pluginOptions; } @@ -153,6 +168,9 @@ export class AngularWebpackPlugin { }); }); + // Load the compiler-cli if not already available + compiler.hooks.beforeCompile.tapPromise(PLUGIN_NAME, () => this.initializeCompilerCli()); + let ngccProcessor: NgccProcessor | undefined; let resourceLoader: WebpackResourceLoader | undefined; let previousUnused: Set | undefined; @@ -172,6 +190,7 @@ export class AngularWebpackPlugin { const { processor, errors, warnings } = initializeNgccProcessor( compiler, this.pluginOptions.tsconfig, + this.compilerNgccModule, ); processor.process(); @@ -185,7 +204,9 @@ export class AngularWebpackPlugin { const { compilerOptions, rootNames, errors } = this.loadConfiguration(); // Create diagnostics reporter and report configuration file errors - const diagnosticsReporter = createDiagnosticsReporter(compilation); + const diagnosticsReporter = createDiagnosticsReporter(compilation, (diagnostic) => + this.compilerCli.formatDiagnostics([diagnostic]), + ); diagnosticsReporter(errors); // Update TypeScript path mapping plugin with new configuration @@ -398,7 +419,10 @@ export class AngularWebpackPlugin { options: compilerOptions, rootNames, errors, - } = readConfiguration(this.pluginOptions.tsconfig, this.pluginOptions.compilerOptions); + } = this.compilerCli.readConfiguration( + this.pluginOptions.tsconfig, + this.pluginOptions.compilerOptions, + ); compilerOptions.enableIvy = true; compilerOptions.noEmitOnError = false; compilerOptions.suppressOutputPathCheck = true; @@ -422,7 +446,7 @@ export class AngularWebpackPlugin { resourceLoader: WebpackResourceLoader, ) { // Create the Angular specific program that contains the Angular compiler - const angularProgram = new NgtscProgram( + const angularProgram = new this.compilerCli.NgtscProgram( rootNames, compilerOptions, host, @@ -561,8 +585,15 @@ export class AngularWebpackPlugin { } } + // Temporary workaround during transition to ESM-only @angular/compiler-cli + // TODO_ESM: This workaround should be removed prior to the final release of v13 + // and replaced with only `this.compilerCli.OptimizeFor`. + const OptimizeFor = + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (this.compilerCli as any).OptimizeFor ?? + require('@angular/compiler-cli/src/ngtsc/typecheck/api').OptimizeFor; + // Collect new Angular diagnostics for files affected by changes - const { OptimizeFor } = require('@angular/compiler-cli/src/ngtsc/typecheck/api'); const optimizeDiagnosticsFor = affectedFiles.size <= DIAGNOSTICS_AFFECTED_THRESHOLD ? OptimizeFor.SingleFile @@ -628,7 +659,7 @@ export class AngularWebpackPlugin { ]; diagnosticsReporter(diagnostics); - const transformers = createJitTransformers(builder, this.pluginOptions); + const transformers = createJitTransformers(builder, this.compilerCli, this.pluginOptions); return { fileEmitter: this.createFileEmitter(builder, transformers, () => []), @@ -687,4 +718,37 @@ export class AngularWebpackPlugin { return { content, map, dependencies, hash }; }; } + + private async initializeCompilerCli(): Promise { + if (this.compilerCliModule) { + return; + } + + // This uses a dynamic import to load `@angular/compiler-cli` which may be ESM. + // CommonJS code can load ESM code via a dynamic import. Unfortunately, TypeScript + // will currently, unconditionally downlevel dynamic import into a require call. + // require calls cannot load ESM code and will result in a runtime error. To workaround + // this, a Function constructor is used to prevent TypeScript from changing the dynamic import. + // Once TypeScript provides support for keeping the dynamic import this workaround can + // be dropped. + const compilerCliModule = await new Function(`return import('@angular/compiler-cli');`)(); + let compilerNgccModule; + try { + compilerNgccModule = await new Function(`return import('@angular/compiler-cli/ngcc');`)(); + } catch { + // If the `exports` field entry is not present then try the file directly. + // TODO_ESM: This try/catch can be removed once the `exports` field is present in `@angular/compiler-cli` + compilerNgccModule = await new Function( + `return import('@angular/compiler-cli/ngcc/index.js');`, + )(); + } + // If it is not ESM then the functions needed will be stored in the `default` property. + // TODO_ESM: This conditional can be removed when `@angular/compiler-cli` is ESM only. + this.compilerCliModule = compilerCliModule.readConfiguration + ? compilerCliModule + : compilerCliModule.default; + this.compilerNgccModule = compilerNgccModule.process + ? compilerNgccModule + : compilerNgccModule.default; + } } diff --git a/packages/ngtools/webpack/src/ivy/transformation.ts b/packages/ngtools/webpack/src/ivy/transformation.ts index 12d7bfd5084f..5240956306d7 100644 --- a/packages/ngtools/webpack/src/ivy/transformation.ts +++ b/packages/ngtools/webpack/src/ivy/transformation.ts @@ -6,7 +6,6 @@ * found in the LICENSE file at https://angular.io/license */ -import { constructorParametersDownlevelTransform } from '@angular/compiler-cli'; import * as ts from 'typescript'; import { elideImports } from '../transformers/elide_imports'; import { removeIvyJitSupportCalls } from '../transformers/remove-ivy-jit-support-calls'; @@ -36,6 +35,7 @@ export function createAotTransformers( export function createJitTransformers( builder: ts.BuilderProgram, + compilerCli: typeof import('@angular/compiler-cli'), options: { directTemplateLoading?: boolean; inlineStyleFileExtension?: string; @@ -51,7 +51,7 @@ export function createJitTransformers( options.directTemplateLoading, options.inlineStyleFileExtension, ), - constructorParametersDownlevelTransform(builder.getProgram()), + compilerCli.constructorParametersDownlevelTransform(builder.getProgram()), ], }; } diff --git a/packages/ngtools/webpack/src/ngcc_processor.ts b/packages/ngtools/webpack/src/ngcc_processor.ts index 7648d3248df9..9c299ced47b3 100644 --- a/packages/ngtools/webpack/src/ngcc_processor.ts +++ b/packages/ngtools/webpack/src/ngcc_processor.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import { LogLevel, Logger, process as mainNgcc } from '@angular/compiler-cli/ngcc'; +import type { LogLevel, Logger } from '@angular/compiler-cli/ngcc'; import { spawnSync } from 'child_process'; import { createHash } from 'crypto'; import { accessSync, constants, existsSync, mkdirSync, readFileSync, writeFileSync } from 'fs'; @@ -35,6 +35,7 @@ export class NgccProcessor { private _nodeModulesDirectory: string; constructor( + private readonly compilerNgcc: typeof import('@angular/compiler-cli/ngcc'), private readonly propertiesToConsider: string[], private readonly compilationWarnings: (Error | string)[], private readonly compilationErrors: (Error | string)[], @@ -43,7 +44,11 @@ export class NgccProcessor { private readonly inputFileSystem: InputFileSystem, private readonly resolver: ResolverWithOptions, ) { - this._logger = new NgccLogger(this.compilationWarnings, this.compilationErrors); + this._logger = new NgccLogger( + this.compilationWarnings, + this.compilationErrors, + compilerNgcc.LogLevel.info, + ); this._nodeModulesDirectory = this.findNodeModulesDirectory(this.basePath); } @@ -115,6 +120,14 @@ export class NgccProcessor { const timeLabel = 'NgccProcessor.process'; time(timeLabel); + // Temporary workaround during transition to ESM-only @angular/compiler-cli + // TODO_ESM: This workaround should be removed prior to the final release of v13 + // and replaced with only `this.compilerNgcc.ngccMainFilePath`. + const ngccExecutablePath = + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (this.compilerNgcc as any).ngccMainFilePath ?? + require.resolve('@angular/compiler-cli/ngcc/main-ngcc.js'); + // We spawn instead of using the API because: // - NGCC Async uses clustering which is problematic when used via the API which means // that we cannot setup multiple cluster masters with different options. @@ -123,7 +136,7 @@ export class NgccProcessor { const { status, error } = spawnSync( process.execPath, [ - require.resolve('@angular/compiler-cli/ngcc/main-ngcc.js'), + ngccExecutablePath, '--source' /** basePath */, this._nodeModulesDirectory, '--properties' /** propertiesToConsider */, @@ -187,7 +200,7 @@ export class NgccProcessor { const timeLabel = `NgccProcessor.processModule.ngcc.process+${moduleName}`; time(timeLabel); - mainNgcc({ + this.compilerNgcc.process({ basePath: this._nodeModulesDirectory, targetEntryPointPath: path.dirname(packageJsonPath), propertiesToConsider: this.propertiesToConsider, @@ -246,11 +259,10 @@ export class NgccProcessor { } class NgccLogger implements Logger { - level = LogLevel.info; - constructor( private readonly compilationWarnings: (Error | string)[], private readonly compilationErrors: (Error | string)[], + public level: LogLevel, ) {} // eslint-disable-next-line @typescript-eslint/no-empty-function diff --git a/tests/legacy-cli/e2e/tests/packages/webpack/test-app.ts b/tests/legacy-cli/e2e/tests/packages/webpack/test-app.ts index 1ee5f70b03ed..91825375d048 100644 --- a/tests/legacy-cli/e2e/tests/packages/webpack/test-app.ts +++ b/tests/legacy-cli/e2e/tests/packages/webpack/test-app.ts @@ -1,14 +1,17 @@ import { normalize } from 'path'; import { createProjectFromAsset } from '../../../utils/assets'; import { expectFileSizeToBeUnder, expectFileToMatch, replaceInFile } from '../../../utils/fs'; -import { exec } from '../../../utils/process'; +import { execWithEnv } from '../../../utils/process'; export default async function (skipCleaning: () => void) { const webpackCLIBin = normalize('node_modules/.bin/webpack-cli'); await createProjectFromAsset('webpack/test-app'); - await exec(webpackCLIBin); + // DISABLE_V8_COMPILE_CACHE=1 is required to disable the `v8-compile-cache` package. + // It currently does not support dynamic import expressions which are now required by the + // CLI to support ESM. ref: https://github.com/zertosh/v8-compile-cache/issues/30 + await execWithEnv(webpackCLIBin, [], { ...process.env, 'DISABLE_V8_COMPILE_CACHE': '1' }); // Note: these sizes are without Build Optimizer or any advanced optimizations in the CLI. await expectFileSizeToBeUnder('dist/app.main.js', 656 * 1024); @@ -16,14 +19,16 @@ export default async function (skipCleaning: () => void) { await expectFileSizeToBeUnder('dist/888.app.main.js', 2 * 1024); await expectFileSizeToBeUnder('dist/972.app.main.js', 2 * 1024); - // test resource urls without ./ await replaceInFile('app/app.component.ts', './app.component.html', 'app.component.html'); await replaceInFile('app/app.component.ts', './app.component.scss', 'app.component.scss'); // test the inclusion of metadata // This build also test resource URLs without ./ - await exec(webpackCLIBin, '--mode=development'); + await execWithEnv(webpackCLIBin, ['--mode=development'], { + ...process.env, + 'DISABLE_V8_COMPILE_CACHE': '1', + }); await expectFileToMatch('dist/app.main.js', 'AppModule'); skipCleaning();