From 11a81898be7de96f5d6f976cd2c87aa8cec5886c Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Sun, 19 Sep 2021 18:07:20 +0200 Subject: [PATCH] refactor(compiler-cli): remove dynamic require in ngcc to allow for bundling (#43431) ngcc currently dynamially loads the `Transformer` code. It does this to avoid unnecessary parsing and loading of transformer-related code if there is nothing to process (so-called noop case). Unfortunately this dynamic require is not recognized by ESBuild. The import needs to be discovered as otheriwse the transformer code would not be included in the bundled package output of the CLI. The ngcc code needs to use an async runtime import as it would work in ES modules. This introduces async code into to the compililation pipeline, breaking the `ngccMain` synchronous invocation feature. To avoid this, we just move the dynamic require/async import to the file top-level so that we do not break synchronous processing which the CLI relies on. This has the downside of slowing-down the noop case a little but I believe that should be mitigated through bundling of ngcc anyway. In the future with full-ESM we won't be able to get around this anyway (unless we remove the sync variant of ngcc processing). PR Close #43431 --- packages/compiler-cli/BUILD.bazel | 2 + .../ngcc/src/execution/cluster/master.ts | 2 +- .../execution/cluster/ngcc_cluster_worker.ts | 46 +++++++++++++++++++ .../ngcc/src/execution/cluster/worker.ts | 38 --------------- .../src/execution/create_compile_function.ts | 2 +- .../lock_file_with_child_process/index.ts | 2 +- .../{unlocker.ts => ngcc_lock_unlocker.ts} | 0 .../unlocker_spec.ts | 2 +- 8 files changed, 52 insertions(+), 42 deletions(-) create mode 100644 packages/compiler-cli/ngcc/src/execution/cluster/ngcc_cluster_worker.ts rename packages/compiler-cli/ngcc/src/locking/lock_file_with_child_process/{unlocker.ts => ngcc_lock_unlocker.ts} (100%) diff --git a/packages/compiler-cli/BUILD.bazel b/packages/compiler-cli/BUILD.bazel index ddb6d53e4e3..149c9f57790 100644 --- a/packages/compiler-cli/BUILD.bazel +++ b/packages/compiler-cli/BUILD.bazel @@ -34,6 +34,8 @@ esbuild( "//packages/compiler-cli/private:bazel.ts", "//packages/compiler-cli/private:localize.ts", "//packages/compiler-cli/private:migrations.ts", + "//packages/compiler-cli/ngcc:src/locking/lock_file_with_child_process/ngcc_lock_unlocker.ts", + "//packages/compiler-cli/ngcc:src/execution/cluster/ngcc_cluster_worker.ts", ], external = [ "@angular/compiler", diff --git a/packages/compiler-cli/ngcc/src/execution/cluster/master.ts b/packages/compiler-cli/ngcc/src/execution/cluster/master.ts index 9024a96ff78..f8d8929757d 100644 --- a/packages/compiler-cli/ngcc/src/execution/cluster/master.ts +++ b/packages/compiler-cli/ngcc/src/execution/cluster/master.ts @@ -44,7 +44,7 @@ export class ClusterMaster { } // Set the worker entry-point - cluster.setupMaster({exec: this.fileSystem.resolve(__dirname, 'worker.js')}); + cluster.setupMaster({exec: this.fileSystem.resolve(__dirname, 'ngcc_cluster_worker.js')}); this.taskQueue = analyzeEntryPoints(); this.onTaskCompleted = createTaskCompletedCallback(this.taskQueue); diff --git a/packages/compiler-cli/ngcc/src/execution/cluster/ngcc_cluster_worker.ts b/packages/compiler-cli/ngcc/src/execution/cluster/ngcc_cluster_worker.ts new file mode 100644 index 00000000000..a7072fecaa8 --- /dev/null +++ b/packages/compiler-cli/ngcc/src/execution/cluster/ngcc_cluster_worker.ts @@ -0,0 +1,46 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +import {parseCommandLineOptions} from '../../command_line_options'; +import {getSharedSetup} from '../../ngcc_options'; +import {getCreateCompileFn} from '../create_compile_function'; + +import {ClusterWorkerPackageJsonUpdater} from './package_json_updater'; +import {startWorker} from './worker'; + +// Cluster worker entry point +(async () => { + process.title = 'ngcc (worker)'; + + try { + const { + logger, + pathMappings, + enableI18nLegacyMessageIdFormat, + fileSystem, + tsConfig, + getFileWriter, + } = getSharedSetup(parseCommandLineOptions(process.argv.slice(2))); + + // NOTE: To avoid file corruption, `ngcc` invocation only creates _one_ instance of + // `PackageJsonUpdater` that actually writes to disk (across all processes). + // In cluster workers we use a `PackageJsonUpdater` that delegates to the cluster master. + const pkgJsonUpdater = new ClusterWorkerPackageJsonUpdater(); + const fileWriter = getFileWriter(pkgJsonUpdater); + + // The function for creating the `compile()` function. + const createCompileFn = getCreateCompileFn( + fileSystem, logger, fileWriter, enableI18nLegacyMessageIdFormat, tsConfig, pathMappings); + + await startWorker(logger, createCompileFn); + process.exitCode = 0; + } catch (e) { + console.error(e.stack || e.message); + process.exit(1); + } +})(); diff --git a/packages/compiler-cli/ngcc/src/execution/cluster/worker.ts b/packages/compiler-cli/ngcc/src/execution/cluster/worker.ts index b74e849dbf3..32b984b2ecd 100644 --- a/packages/compiler-cli/ngcc/src/execution/cluster/worker.ts +++ b/packages/compiler-cli/ngcc/src/execution/cluster/worker.ts @@ -10,50 +10,12 @@ import * as cluster from 'cluster'; import {Logger} from '../../../../src/ngtsc/logging'; -import {parseCommandLineOptions} from '../../command_line_options'; -import {getSharedSetup} from '../../ngcc_options'; import {CreateCompileFn} from '../api'; -import {getCreateCompileFn} from '../create_compile_function'; import {stringifyTask} from '../tasks/utils'; import {MessageToWorker} from './api'; -import {ClusterWorkerPackageJsonUpdater} from './package_json_updater'; import {sendMessageToMaster} from './utils'; -// Cluster worker entry point -if (require.main === module) { - (async () => { - process.title = 'ngcc (worker)'; - - try { - const { - logger, - pathMappings, - enableI18nLegacyMessageIdFormat, - fileSystem, - tsConfig, - getFileWriter, - } = getSharedSetup(parseCommandLineOptions(process.argv.slice(2))); - - // NOTE: To avoid file corruption, `ngcc` invocation only creates _one_ instance of - // `PackageJsonUpdater` that actually writes to disk (across all processes). - // In cluster workers we use a `PackageJsonUpdater` that delegates to the cluster master. - const pkgJsonUpdater = new ClusterWorkerPackageJsonUpdater(); - const fileWriter = getFileWriter(pkgJsonUpdater); - - // The function for creating the `compile()` function. - const createCompileFn = getCreateCompileFn( - fileSystem, logger, fileWriter, enableI18nLegacyMessageIdFormat, tsConfig, pathMappings); - - await startWorker(logger, createCompileFn); - process.exitCode = 0; - } catch (e) { - console.error(e.stack || e.message); - process.exit(1); - } - })(); -} - export async function startWorker(logger: Logger, createCompileFn: CreateCompileFn): Promise { if (cluster.isMaster) { throw new Error('Tried to run cluster worker on the master process.'); diff --git a/packages/compiler-cli/ngcc/src/execution/create_compile_function.ts b/packages/compiler-cli/ngcc/src/execution/create_compile_function.ts index 3b2005d60c0..fb528a8ea02 100644 --- a/packages/compiler-cli/ngcc/src/execution/create_compile_function.ts +++ b/packages/compiler-cli/ngcc/src/execution/create_compile_function.ts @@ -15,6 +15,7 @@ import {ParsedConfiguration} from '../../../src/perform_compile'; import {getEntryPointFormat} from '../packages/entry_point'; import {makeEntryPointBundle} from '../packages/entry_point_bundle'; import {createModuleResolutionCache, SharedFileCache} from '../packages/source_file_cache'; +import {Transformer} from '../packages/transformer'; import {PathMappings} from '../path_mappings'; import {FileWriter} from '../writing/file_writer'; @@ -29,7 +30,6 @@ export function getCreateCompileFn( enableI18nLegacyMessageIdFormat: boolean, tsConfig: ParsedConfiguration|null, pathMappings: PathMappings|undefined): CreateCompileFn { return (beforeWritingFiles, onTaskCompleted) => { - const {Transformer} = require('../packages/transformer'); const transformer = new Transformer(fileSystem, logger, tsConfig); const sharedFileCache = new SharedFileCache(fileSystem); const moduleResolutionCache = createModuleResolutionCache(fileSystem); diff --git a/packages/compiler-cli/ngcc/src/locking/lock_file_with_child_process/index.ts b/packages/compiler-cli/ngcc/src/locking/lock_file_with_child_process/index.ts index feafea9d456..8a0c59a8a58 100644 --- a/packages/compiler-cli/ngcc/src/locking/lock_file_with_child_process/index.ts +++ b/packages/compiler-cli/ngcc/src/locking/lock_file_with_child_process/index.ts @@ -78,7 +78,7 @@ export class LockFileWithChildProcess implements LockFile { this.logger.level !== undefined ? this.logger.level.toString() : LogLevel.info.toString(); const isWindows = process.platform === 'win32'; const unlocker = fork( - __dirname + '/unlocker.js', [path, logLevel], + __dirname + '/ngcc_lock_unlocker.js', [path, logLevel], {detached: true, stdio: isWindows ? 'pipe' : 'inherit'}); if (isWindows) { unlocker.stdout?.on('data', process.stdout.write.bind(process.stdout)); diff --git a/packages/compiler-cli/ngcc/src/locking/lock_file_with_child_process/unlocker.ts b/packages/compiler-cli/ngcc/src/locking/lock_file_with_child_process/ngcc_lock_unlocker.ts similarity index 100% rename from packages/compiler-cli/ngcc/src/locking/lock_file_with_child_process/unlocker.ts rename to packages/compiler-cli/ngcc/src/locking/lock_file_with_child_process/ngcc_lock_unlocker.ts diff --git a/packages/compiler-cli/ngcc/test/locking/lockfile_with_child_process/unlocker_spec.ts b/packages/compiler-cli/ngcc/test/locking/lockfile_with_child_process/unlocker_spec.ts index 07ca1b59954..83b9bc5b4e4 100644 --- a/packages/compiler-cli/ngcc/test/locking/lockfile_with_child_process/unlocker_spec.ts +++ b/packages/compiler-cli/ngcc/test/locking/lockfile_with_child_process/unlocker_spec.ts @@ -11,7 +11,7 @@ describe('unlocker', () => { it('should attach a handler to the `disconnect` event', () => { spyOn(process, 'on'); - require('../../../src/locking/lock_file_with_child_process/unlocker'); + require('../../../src/locking/lock_file_with_child_process/ngcc_lock_unlocker'); // TODO: @JiaLiPassion, need to wait for @types/jasmine to handle the override case // https://github.com/DefinitelyTyped/DefinitelyTyped/issues/42455 expect(process.on).toHaveBeenCalledWith('disconnect' as any, jasmine.any(Function));