Skip to content

Commit

Permalink
refactor(@ngtools/webpack): support an ESM-only `@angular/compiler-cl…
Browse files Browse the repository at this point in the history
…i` 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.
  • Loading branch information
clydin authored and alan-agius4 committed Sep 22, 2021
1 parent 5bb6897 commit 7d98ab3
Show file tree
Hide file tree
Showing 7 changed files with 112 additions and 28 deletions.
2 changes: 1 addition & 1 deletion goldens/public-api/ngtools/webpack/src/index.md
Expand Up @@ -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)
Expand Down
9 changes: 6 additions & 3 deletions packages/ngtools/webpack/src/ivy/diagnostics.ts
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion packages/ngtools/webpack/src/ivy/host.ts
Expand Up @@ -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';
Expand Down
86 changes: 75 additions & 11 deletions packages/ngtools/webpack/src/ivy/plugin.ts
Expand Up @@ -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';
Expand Down Expand Up @@ -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() ?? [];
Expand All @@ -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,
Expand All @@ -95,6 +99,8 @@ const compilationFileEmitters = new WeakMap<Compilation, FileEmitterCollection>(

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;
Expand All @@ -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;
}
Expand Down Expand Up @@ -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<string> | undefined;
Expand All @@ -172,6 +190,7 @@ export class AngularWebpackPlugin {
const { processor, errors, warnings } = initializeNgccProcessor(
compiler,
this.pluginOptions.tsconfig,
this.compilerNgccModule,
);

processor.process();
Expand All @@ -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
Expand Down Expand Up @@ -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;
Expand All @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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, () => []),
Expand Down Expand Up @@ -687,4 +718,37 @@ export class AngularWebpackPlugin {
return { content, map, dependencies, hash };
};
}

private async initializeCompilerCli(): Promise<void> {
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;
}
}
4 changes: 2 additions & 2 deletions packages/ngtools/webpack/src/ivy/transformation.ts
Expand Up @@ -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';
Expand Down Expand Up @@ -36,6 +35,7 @@ export function createAotTransformers(

export function createJitTransformers(
builder: ts.BuilderProgram,
compilerCli: typeof import('@angular/compiler-cli'),
options: {
directTemplateLoading?: boolean;
inlineStyleFileExtension?: string;
Expand All @@ -51,7 +51,7 @@ export function createJitTransformers(
options.directTemplateLoading,
options.inlineStyleFileExtension,
),
constructorParametersDownlevelTransform(builder.getProgram()),
compilerCli.constructorParametersDownlevelTransform(builder.getProgram()),
],
};
}
Expand Down
24 changes: 18 additions & 6 deletions packages/ngtools/webpack/src/ngcc_processor.ts
Expand Up @@ -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';
Expand Down Expand Up @@ -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)[],
Expand All @@ -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);
}

Expand Down Expand Up @@ -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.
Expand All @@ -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 */,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down
13 changes: 9 additions & 4 deletions tests/legacy-cli/e2e/tests/packages/webpack/test-app.ts
@@ -1,29 +1,34 @@
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);
await expectFileSizeToBeUnder('dist/501.app.main.js', 1 * 1024);
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();
Expand Down

0 comments on commit 7d98ab3

Please sign in to comment.