refactor: change ngcc to only rely on ESM features (#48521)

Refactors ngcc to only rely on ESM features because CJS
is no longer needed & tested.

PR Close #48521
This commit is contained in:
Paul Gschwendtner 2022-12-13 17:32:50 +00:00
parent 661134fd21
commit 619f0900a2
12 changed files with 90 additions and 57 deletions

View file

@ -44,7 +44,7 @@ export class ClusterMaster {
}
// Set the worker entry-point
cluster.setupMaster({exec: getClusterWorkerScriptPath(fileSystem)});
cluster.setupMaster({exec: ClusterWorkerScriptResolver.resolve(fileSystem)});
this.taskQueue = analyzeEntryPoints();
this.onTaskCompleted = createTaskCompletedCallback(this.taskQueue);
@ -343,13 +343,15 @@ export class ClusterMaster {
}
}
/** Gets the absolute file path to the cluster worker script. */
export function getClusterWorkerScriptPath(fileSystem: PathManipulation): AbsoluteFsPath {
// NodeJS `import.meta.resolve` is experimental. We leverage `require`.
const requireFn = module.createRequire(import.meta.url);
// We resolve the worker script using module resolution as in the package output,
// the worker might be bundled but exposed through a subpath export mapping.
const workerScriptPath =
requireFn.resolve('@angular/compiler-cli/ngcc/src/execution/cluster/ngcc_cluster_worker');
return fileSystem.resolve(workerScriptPath);
/** Wrapper for resolving the cluster worker script. Useful for test patching. */
export class ClusterWorkerScriptResolver {
static resolve(fileSystem: PathManipulation): AbsoluteFsPath {
// NodeJS `import.meta.resolve` is experimental. We leverage `require`.
const requireFn = module.createRequire(import.meta.url);
// We resolve the worker script using module resolution as in the package output,
// the worker might be bundled but exposed through a subpath export mapping.
const workerScriptPath =
requireFn.resolve('@angular/compiler-cli/ngcc/src/execution/cluster/ngcc_cluster_worker');
return fileSystem.resolve(workerScriptPath);
}
}

View file

@ -10,15 +10,18 @@ import module from 'module';
import {AbsoluteFsPath, PathManipulation} from '../../../src/ngtsc/file_system';
export function getLockFilePath(fs: PathManipulation) {
// NodeJS `import.meta.resolve` is experimental. We leverage `require`.
const requireFn = module.createRequire(import.meta.url);
// The lock file location is resolved based on the location of the `ngcc` entry-point as this
// allows us to have a consistent position for the lock file to reside. We are unable to rely
// on `__dirname` (or equivalent) as this code is being bundled and different entry-points
// will have dedicated bundles where the lock file location would differ then.
const ngccEntryPointFile = requireFn.resolve('@angular/compiler-cli/package.json');
return fs.resolve(ngccEntryPointFile, '../../../.ngcc_lock_file');
/** Wrapper for resolving the lcok file path. Useful for test patching. */
export class LockFilePathResolver {
static resolve(fs: PathManipulation): AbsoluteFsPath {
// NodeJS `import.meta.resolve` is experimental. We leverage `require`.
const requireFn = module.createRequire(import.meta.url);
// The lock file location is resolved based on the location of the `ngcc` entry-point as this
// allows us to have a consistent position for the lock file to reside. We are unable to rely
// on `__dirname` (or equivalent) as this code is being bundled and different entry-points
// will have dedicated bundles where the lock file location would differ then.
const ngccEntryPointFile = requireFn.resolve('@angular/compiler-cli/package.json');
return fs.resolve(ngccEntryPointFile, '../../../.ngcc_lock_file');
}
}
export interface LockFile {

View file

@ -10,7 +10,7 @@ import module from 'module';
import {AbsoluteFsPath, FileSystem} from '../../../../src/ngtsc/file_system';
import {Logger, LogLevel} from '../../../../src/ngtsc/logging';
import {getLockFilePath, LockFile} from '../lock_file';
import {LockFile, LockFilePathResolver} from '../lock_file';
import {removeLockFile} from './util';
@ -38,7 +38,7 @@ export class LockFileWithChildProcess implements LockFile {
private unlocker: ChildProcess|null;
constructor(protected fs: FileSystem, protected logger: Logger) {
this.path = getLockFilePath(fs);
this.path = LockFilePathResolver.resolve(fs);
this.unlocker = this.createUnlocker(this.path);
}
@ -79,7 +79,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(
getLockFileUnlockerScriptPath(this.fs), [path, logLevel],
LockFileUnlockerScriptResolver.resolve(this.fs), [path, logLevel],
{detached: true, stdio: isWindows ? 'pipe' : 'inherit'});
if (isWindows) {
unlocker.stdout?.on('data', process.stdout.write.bind(process.stdout));
@ -89,13 +89,15 @@ export class LockFileWithChildProcess implements LockFile {
}
}
/** Gets the absolute file path to the lock file unlocker script. */
export function getLockFileUnlockerScriptPath(fileSystem: FileSystem): AbsoluteFsPath {
// NodeJS `import.meta.resolve` is experimental. We leverage `require`.
const requireFn = module.createRequire(import.meta.url);
// We resolve the worker script using module resolution as in the package output,
// the worker might be bundled but exposed through a subpath export mapping.
const unlockerScriptPath = requireFn.resolve(
'@angular/compiler-cli/ngcc/src/locking/lock_file_with_child_process/ngcc_lock_unlocker');
return fileSystem.resolve(unlockerScriptPath);
/** Wrapper for resolving the lock file unlocker script. Useful for test patching. */
export class LockFileUnlockerScriptResolver {
static resolve(fs: FileSystem): AbsoluteFsPath {
// NodeJS `import.meta.resolve` is experimental. We leverage `require`.
const requireFn = module.createRequire(import.meta.url);
// We resolve the worker script using module resolution as in the package output,
// the worker might be bundled but exposed through a subpath export mapping.
const unlockerScriptPath = requireFn.resolve(
'@angular/compiler-cli/ngcc/src/locking/lock_file_with_child_process/ngcc_lock_unlocker');
return fs.resolve(unlockerScriptPath);
}
}

View file

@ -5,7 +5,7 @@
* 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 * as os from 'os';
import os from 'os';
import {absoluteFrom, AbsoluteFsPath, FileSystem, getFileSystem, PathManipulation} from '../../src/ngtsc/file_system';
import {ConsoleLogger, Logger, LogLevel} from '../../src/ngtsc/logging';

View file

@ -60,6 +60,7 @@ ts_library(
"//packages/compiler-cli/src/ngtsc/file_system/testing",
"//packages/compiler-cli/src/ngtsc/logging/testing",
"//packages/compiler-cli/src/ngtsc/testing",
"@npm//@bazel/runfiles",
"@npm//rxjs",
"@npm//typescript",
],

View file

@ -20,6 +20,7 @@ import {FileWriter} from '../../../src/writing/file_writer';
import {PackageJsonUpdater} from '../../../src/writing/package_json_updater';
import {MockLockFile} from '../../helpers/mock_lock_file';
import {mockProperty} from '../../helpers/spy_utils';
import {mockRequireResolveForWorkerScript} from '../../helpers/utils';
runInEachFileSystem(() => {
describe('ClusterExecutor', () => {
@ -33,6 +34,8 @@ runInEachFileSystem(() => {
let createTaskCompletedCallback: jasmine.Spy;
beforeEach(() => {
mockRequireResolveForWorkerScript();
masterRunSpy = spyOn(ClusterMaster.prototype, 'run')
.and.returnValue(Promise.resolve('ClusterMaster#run()' as any));
createTaskCompletedCallback = jasmine.createSpy('createTaskCompletedCallback');

View file

@ -9,7 +9,10 @@ import ts from 'typescript';
import {absoluteFrom, AbsoluteFsPath, getFileSystem, NgtscCompilerHost} from '../../../src/ngtsc/file_system';
import {TestFile} from '../../../src/ngtsc/file_system/testing';
import {ClusterWorkerScriptResolver} from '../../src/execution/cluster/master';
import {DtsProcessing} from '../../src/execution/tasks/api';
import {LockFilePathResolver} from '../../src/locking/lock_file';
import {LockFileUnlockerScriptResolver} from '../../src/locking/lock_file_with_child_process';
import {BundleProgram, makeBundleProgram} from '../../src/packages/bundle_program';
import {NgccEntryPointConfig} from '../../src/packages/configuration';
import {EntryPoint, EntryPointFormat} from '../../src/packages/entry_point';
@ -125,13 +128,24 @@ export function getRootFiles(testFiles: TestFile[]): AbsoluteFsPath[] {
* Mock out the lockfile path resolution, which uses `require.resolve()`.
*/
export function mockRequireResolveForLockfile() {
const moduleConstructor: any = module.constructor;
const originalResolveFileName = moduleConstructor._resolveFilename;
spyOn<any>(moduleConstructor, '_resolveFilename').and.callFake(function(request: string) {
if (request === '@angular/compiler-cli/package.json') {
return '/node_modules/' + request;
} else {
return originalResolveFileName.apply(null, arguments as any);
}
});
spyOn(LockFilePathResolver, 'resolve')
.and.returnValue(absoluteFrom('/node_modules/@angular/compiler-cli/package.json'));
}
/**
* Mock out the worker script resolution, which uses `require.resolve()`.
*/
export function mockRequireResolveForWorkerScript() {
spyOn(ClusterWorkerScriptResolver, 'resolve')
.and.returnValue(absoluteFrom(
'/node_modules/@angular/compiler-cli/ngcc/src/execution/cluster/ngcc_cluster_worker'));
}
/**
* Mock out the lock file unlocker script resolution, which uses `require.resolve()`.
*/
export function mockRequireResolveForLockFileUnlockerScript() {
spyOn(LockFileUnlockerScriptResolver, 'resolve')
.and.returnValue(absoluteFrom(
'/node_modules/@angular/compiler-cli/ngcc/src/locking/lock_file_with_child_process/ngcc_lock_unlocker'));
}

View file

@ -7,6 +7,7 @@
*/
/// <reference types="node" />
import {runfiles} from '@bazel/runfiles';
import realFs from 'fs';
import os from 'os';
@ -14,7 +15,7 @@ import {absoluteFrom, AbsoluteFsPath, FileSystem, getFileSystem} from '../../../
import {Folder, MockFileSystem, runInEachFileSystem, TestFile} from '../../../src/ngtsc/file_system/testing';
import {MockLogger} from '../../../src/ngtsc/logging/testing';
import {loadTestFiles} from '../../../src/ngtsc/testing';
import {getLockFilePath} from '../../src/locking/lock_file';
import {LockFilePathResolver} from '../../src/locking/lock_file';
import {mainNgcc} from '../../src/main';
import {clearTsConfigCache} from '../../src/ngcc_options';
import {hasBeenProcessed, markAsProcessed} from '../../src/packages/build_marker';
@ -22,7 +23,7 @@ import {EntryPointJsonProperty, EntryPointPackageJson, SUPPORTED_FORMAT_PROPERTI
import {EntryPointManifestFile} from '../../src/packages/entry_point_manifest';
import {Transformer} from '../../src/packages/transformer';
import {DirectPackageJsonUpdater, PackageJsonUpdater} from '../../src/writing/package_json_updater';
import {mockRequireResolveForLockfile} from '../helpers/utils';
import {mockRequireResolveForLockfile, mockRequireResolveForLockFileUnlockerScript, mockRequireResolveForWorkerScript} from '../helpers/utils';
import {loadNgccIntegrationTestFiles} from './util';
@ -47,7 +48,11 @@ runInEachFileSystem(() => {
_ = absoluteFrom;
fs = getFileSystem();
pkgJsonUpdater = new DirectPackageJsonUpdater(fs);
mockRequireResolveForLockfile();
mockRequireResolveForWorkerScript();
mockRequireResolveForLockFileUnlockerScript();
initMockFileSystem(fs, testFiles);
// Force single-process execution in unit tests by mocking available CPUs to 1.
@ -71,7 +76,7 @@ runInEachFileSystem(() => {
fs.ensureDir(fs.join(pkgPath, 'fesm5'));
fs.writeFile(
fs.join(pkgPath, 'fesm5/core.js'),
realFs.readFileSync(require.resolve('../fesm5_angular_core.js'), 'utf8'));
realFs.readFileSync(runfiles.resolvePackageRelative('./fesm5_angular_core.js'), 'utf8'));
pkgJson.esm5 = './fesm5/core.js';
pkgJson.fesm5 = './fesm5/core.js';
@ -3940,7 +3945,7 @@ runInEachFileSystem(() => {
function initMockFileSystem(fs: FileSystem, testFiles: Folder) {
if (fs instanceof MockFileSystem) {
fs.init(testFiles);
fs.ensureDir(fs.dirname(getLockFilePath(fs)));
fs.ensureDir(fs.dirname(LockFilePathResolver.resolve(fs)));
}
// a random test package that no metadata.json file so not compiled by Angular.

View file

@ -8,7 +8,7 @@
import {AbsoluteFsPath} from '../../../src/ngtsc/file_system';
import {Folder, MockFileSystemPosix} from '../../../src/ngtsc/file_system/testing';
import {loadTestDirectory, loadTsLib, resolveNpmTreeArtifact} from '../../../src/ngtsc/testing';
import {loadTestDirectory, loadTsLib, resolveFromRunfiles} from '../../../src/ngtsc/testing';
export function loadNgccIntegrationTestFiles(): Folder {
const tmpFs = new MockFileSystemPosix(true);
@ -16,13 +16,15 @@ export function loadNgccIntegrationTestFiles(): Folder {
loadTsLib(tmpFs, basePath);
loadTestDirectory(
tmpFs, resolveNpmTreeArtifact('@angular/core-12'),
tmpFs, resolveFromRunfiles('npm/node_modules/@angular/core-12'),
tmpFs.resolve('/node_modules/@angular/core'));
loadTestDirectory(
tmpFs, resolveNpmTreeArtifact('@angular/common-12'),
tmpFs, resolveFromRunfiles('npm/node_modules/@angular/common-12'),
tmpFs.resolve('/node_modules/@angular/common'));
loadTestDirectory(
tmpFs, resolveNpmTreeArtifact('typescript'), tmpFs.resolve('/node_modules/typescript'));
loadTestDirectory(tmpFs, resolveNpmTreeArtifact('rxjs'), tmpFs.resolve('/node_modules/rxjs'));
tmpFs, resolveFromRunfiles('npm/node_modules/typescript'),
tmpFs.resolve('/node_modules/typescript'));
loadTestDirectory(
tmpFs, resolveFromRunfiles('npm/node_modules/rxjs'), tmpFs.resolve('/node_modules/rxjs'));
return tmpFs.dump();
}

View file

@ -11,7 +11,7 @@ import * as process from 'process';
import {FileSystem, getFileSystem} from '../../../../src/ngtsc/file_system';
import {runInEachFileSystem} from '../../../../src/ngtsc/file_system/testing';
import {MockLogger} from '../../../../src/ngtsc/logging/testing';
import {getLockFilePath} from '../../../src/locking/lock_file';
import {LockFilePathResolver} from '../../../src/locking/lock_file';
import {LockFileWithChildProcess} from '../../../src/locking/lock_file_with_child_process';
import {mockRequireResolveForLockfile} from '../../helpers/utils';
@ -72,10 +72,11 @@ runInEachFileSystem(() => {
it('should write the lock-file to disk', () => {
const fs = getFileSystem();
const lockFile = new LockFileUnderTest(fs);
expect(fs.exists(getLockFilePath(fs))).toBe(false);
const lockPath = LockFilePathResolver.resolve(fs);
expect(fs.exists(lockPath)).toBe(false);
lockFile.write();
expect(fs.exists(getLockFilePath(fs))).toBe(true);
expect(fs.readFile(getLockFilePath(fs))).toEqual('' + process.pid);
expect(fs.exists(lockPath)).toBe(true);
expect(fs.readFile(lockPath)).toEqual('' + process.pid);
});
it('should create the unlocker process if it is not already created', () => {

View file

@ -9,9 +9,9 @@
/// <reference types="node" />
describe('unlocker', () => {
it('should attach a handler to the `disconnect` event', () => {
it('should attach a handler to the `disconnect` event', async () => {
spyOn(process, 'on');
require('../../../src/locking/lock_file_with_child_process/ngcc_lock_unlocker');
await import('../../../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));

View file

@ -5,6 +5,7 @@
* 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
*/
// Note: We do not use a namespace import here because this will result in the
// named exports being modified if we apply jasmine spies on `realFs`. Using
// the default export gives us an object where we can patch properties on.
@ -13,7 +14,6 @@ import os from 'os';
import {absoluteFrom, AbsoluteFsPath, FileSystem, getFileSystem} from '../../src/ngtsc/file_system';
import {runInEachFileSystem} from '../../src/ngtsc/file_system/testing';
import {MockLogger} from '../../src/ngtsc/logging/testing';
import {clearTsConfigCache, getMaxNumberOfWorkers, getSharedSetup, NgccOptions} from '../src/ngcc_options';