Skip to content

Commit 619f090

Browse files
committed
refactor: change ngcc to only rely on ESM features (angular#48521)
Refactors ngcc to only rely on ESM features because CJS is no longer needed & tested. PR Close angular#48521
1 parent 661134f commit 619f090

File tree

12 files changed

+90
-57
lines changed

12 files changed

+90
-57
lines changed

packages/compiler-cli/ngcc/src/execution/cluster/master.ts

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ export class ClusterMaster {
4444
}
4545

4646
// Set the worker entry-point
47-
cluster.setupMaster({exec: getClusterWorkerScriptPath(fileSystem)});
47+
cluster.setupMaster({exec: ClusterWorkerScriptResolver.resolve(fileSystem)});
4848

4949
this.taskQueue = analyzeEntryPoints();
5050
this.onTaskCompleted = createTaskCompletedCallback(this.taskQueue);
@@ -343,13 +343,15 @@ export class ClusterMaster {
343343
}
344344
}
345345

346-
/** Gets the absolute file path to the cluster worker script. */
347-
export function getClusterWorkerScriptPath(fileSystem: PathManipulation): AbsoluteFsPath {
348-
// NodeJS `import.meta.resolve` is experimental. We leverage `require`.
349-
const requireFn = module.createRequire(import.meta.url);
350-
// We resolve the worker script using module resolution as in the package output,
351-
// the worker might be bundled but exposed through a subpath export mapping.
352-
const workerScriptPath =
353-
requireFn.resolve('@angular/compiler-cli/ngcc/src/execution/cluster/ngcc_cluster_worker');
354-
return fileSystem.resolve(workerScriptPath);
346+
/** Wrapper for resolving the cluster worker script. Useful for test patching. */
347+
export class ClusterWorkerScriptResolver {
348+
static resolve(fileSystem: PathManipulation): AbsoluteFsPath {
349+
// NodeJS `import.meta.resolve` is experimental. We leverage `require`.
350+
const requireFn = module.createRequire(import.meta.url);
351+
// We resolve the worker script using module resolution as in the package output,
352+
// the worker might be bundled but exposed through a subpath export mapping.
353+
const workerScriptPath =
354+
requireFn.resolve('@angular/compiler-cli/ngcc/src/execution/cluster/ngcc_cluster_worker');
355+
return fileSystem.resolve(workerScriptPath);
356+
}
355357
}

packages/compiler-cli/ngcc/src/locking/lock_file.ts

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,15 +10,18 @@ import module from 'module';
1010

1111
import {AbsoluteFsPath, PathManipulation} from '../../../src/ngtsc/file_system';
1212

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

2427
export interface LockFile {

packages/compiler-cli/ngcc/src/locking/lock_file_with_child_process/index.ts

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import module from 'module';
1010

1111
import {AbsoluteFsPath, FileSystem} from '../../../../src/ngtsc/file_system';
1212
import {Logger, LogLevel} from '../../../../src/ngtsc/logging';
13-
import {getLockFilePath, LockFile} from '../lock_file';
13+
import {LockFile, LockFilePathResolver} from '../lock_file';
1414

1515
import {removeLockFile} from './util';
1616

@@ -38,7 +38,7 @@ export class LockFileWithChildProcess implements LockFile {
3838
private unlocker: ChildProcess|null;
3939

4040
constructor(protected fs: FileSystem, protected logger: Logger) {
41-
this.path = getLockFilePath(fs);
41+
this.path = LockFilePathResolver.resolve(fs);
4242
this.unlocker = this.createUnlocker(this.path);
4343
}
4444

@@ -79,7 +79,7 @@ export class LockFileWithChildProcess implements LockFile {
7979
this.logger.level !== undefined ? this.logger.level.toString() : LogLevel.info.toString();
8080
const isWindows = process.platform === 'win32';
8181
const unlocker = fork(
82-
getLockFileUnlockerScriptPath(this.fs), [path, logLevel],
82+
LockFileUnlockerScriptResolver.resolve(this.fs), [path, logLevel],
8383
{detached: true, stdio: isWindows ? 'pipe' : 'inherit'});
8484
if (isWindows) {
8585
unlocker.stdout?.on('data', process.stdout.write.bind(process.stdout));
@@ -89,13 +89,15 @@ export class LockFileWithChildProcess implements LockFile {
8989
}
9090
}
9191

92-
/** Gets the absolute file path to the lock file unlocker script. */
93-
export function getLockFileUnlockerScriptPath(fileSystem: FileSystem): AbsoluteFsPath {
94-
// NodeJS `import.meta.resolve` is experimental. We leverage `require`.
95-
const requireFn = module.createRequire(import.meta.url);
96-
// We resolve the worker script using module resolution as in the package output,
97-
// the worker might be bundled but exposed through a subpath export mapping.
98-
const unlockerScriptPath = requireFn.resolve(
99-
'@angular/compiler-cli/ngcc/src/locking/lock_file_with_child_process/ngcc_lock_unlocker');
100-
return fileSystem.resolve(unlockerScriptPath);
92+
/** Wrapper for resolving the lock file unlocker script. Useful for test patching. */
93+
export class LockFileUnlockerScriptResolver {
94+
static resolve(fs: FileSystem): AbsoluteFsPath {
95+
// NodeJS `import.meta.resolve` is experimental. We leverage `require`.
96+
const requireFn = module.createRequire(import.meta.url);
97+
// We resolve the worker script using module resolution as in the package output,
98+
// the worker might be bundled but exposed through a subpath export mapping.
99+
const unlockerScriptPath = requireFn.resolve(
100+
'@angular/compiler-cli/ngcc/src/locking/lock_file_with_child_process/ngcc_lock_unlocker');
101+
return fs.resolve(unlockerScriptPath);
102+
}
101103
}

packages/compiler-cli/ngcc/src/ngcc_options.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
* Use of this source code is governed by an MIT-style license that can be
66
* found in the LICENSE file at https://angular.io/license
77
*/
8-
import * as os from 'os';
8+
import os from 'os';
99

1010
import {absoluteFrom, AbsoluteFsPath, FileSystem, getFileSystem, PathManipulation} from '../../src/ngtsc/file_system';
1111
import {ConsoleLogger, Logger, LogLevel} from '../../src/ngtsc/logging';

packages/compiler-cli/ngcc/test/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ ts_library(
6060
"//packages/compiler-cli/src/ngtsc/file_system/testing",
6161
"//packages/compiler-cli/src/ngtsc/logging/testing",
6262
"//packages/compiler-cli/src/ngtsc/testing",
63+
"@npm//@bazel/runfiles",
6364
"@npm//rxjs",
6465
"@npm//typescript",
6566
],

packages/compiler-cli/ngcc/test/execution/cluster/executor_spec.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import {FileWriter} from '../../../src/writing/file_writer';
2020
import {PackageJsonUpdater} from '../../../src/writing/package_json_updater';
2121
import {MockLockFile} from '../../helpers/mock_lock_file';
2222
import {mockProperty} from '../../helpers/spy_utils';
23+
import {mockRequireResolveForWorkerScript} from '../../helpers/utils';
2324

2425
runInEachFileSystem(() => {
2526
describe('ClusterExecutor', () => {
@@ -33,6 +34,8 @@ runInEachFileSystem(() => {
3334
let createTaskCompletedCallback: jasmine.Spy;
3435

3536
beforeEach(() => {
37+
mockRequireResolveForWorkerScript();
38+
3639
masterRunSpy = spyOn(ClusterMaster.prototype, 'run')
3740
.and.returnValue(Promise.resolve('ClusterMaster#run()' as any));
3841
createTaskCompletedCallback = jasmine.createSpy('createTaskCompletedCallback');

packages/compiler-cli/ngcc/test/helpers/utils.ts

Lines changed: 23 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,10 @@ import ts from 'typescript';
99

1010
import {absoluteFrom, AbsoluteFsPath, getFileSystem, NgtscCompilerHost} from '../../../src/ngtsc/file_system';
1111
import {TestFile} from '../../../src/ngtsc/file_system/testing';
12+
import {ClusterWorkerScriptResolver} from '../../src/execution/cluster/master';
1213
import {DtsProcessing} from '../../src/execution/tasks/api';
14+
import {LockFilePathResolver} from '../../src/locking/lock_file';
15+
import {LockFileUnlockerScriptResolver} from '../../src/locking/lock_file_with_child_process';
1316
import {BundleProgram, makeBundleProgram} from '../../src/packages/bundle_program';
1417
import {NgccEntryPointConfig} from '../../src/packages/configuration';
1518
import {EntryPoint, EntryPointFormat} from '../../src/packages/entry_point';
@@ -125,13 +128,24 @@ export function getRootFiles(testFiles: TestFile[]): AbsoluteFsPath[] {
125128
* Mock out the lockfile path resolution, which uses `require.resolve()`.
126129
*/
127130
export function mockRequireResolveForLockfile() {
128-
const moduleConstructor: any = module.constructor;
129-
const originalResolveFileName = moduleConstructor._resolveFilename;
130-
spyOn<any>(moduleConstructor, '_resolveFilename').and.callFake(function(request: string) {
131-
if (request === '@angular/compiler-cli/package.json') {
132-
return '/node_modules/' + request;
133-
} else {
134-
return originalResolveFileName.apply(null, arguments as any);
135-
}
136-
});
131+
spyOn(LockFilePathResolver, 'resolve')
132+
.and.returnValue(absoluteFrom('/node_modules/@angular/compiler-cli/package.json'));
133+
}
134+
135+
/**
136+
* Mock out the worker script resolution, which uses `require.resolve()`.
137+
*/
138+
export function mockRequireResolveForWorkerScript() {
139+
spyOn(ClusterWorkerScriptResolver, 'resolve')
140+
.and.returnValue(absoluteFrom(
141+
'/node_modules/@angular/compiler-cli/ngcc/src/execution/cluster/ngcc_cluster_worker'));
142+
}
143+
144+
/**
145+
* Mock out the lock file unlocker script resolution, which uses `require.resolve()`.
146+
*/
147+
export function mockRequireResolveForLockFileUnlockerScript() {
148+
spyOn(LockFileUnlockerScriptResolver, 'resolve')
149+
.and.returnValue(absoluteFrom(
150+
'/node_modules/@angular/compiler-cli/ngcc/src/locking/lock_file_with_child_process/ngcc_lock_unlocker'));
137151
}

packages/compiler-cli/ngcc/test/integration/ngcc_spec.ts

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,22 +7,23 @@
77
*/
88

99
/// <reference types="node" />
10+
import {runfiles} from '@bazel/runfiles';
1011
import realFs from 'fs';
1112
import os from 'os';
1213

1314
import {absoluteFrom, AbsoluteFsPath, FileSystem, getFileSystem} from '../../../src/ngtsc/file_system';
1415
import {Folder, MockFileSystem, runInEachFileSystem, TestFile} from '../../../src/ngtsc/file_system/testing';
1516
import {MockLogger} from '../../../src/ngtsc/logging/testing';
1617
import {loadTestFiles} from '../../../src/ngtsc/testing';
17-
import {getLockFilePath} from '../../src/locking/lock_file';
18+
import {LockFilePathResolver} from '../../src/locking/lock_file';
1819
import {mainNgcc} from '../../src/main';
1920
import {clearTsConfigCache} from '../../src/ngcc_options';
2021
import {hasBeenProcessed, markAsProcessed} from '../../src/packages/build_marker';
2122
import {EntryPointJsonProperty, EntryPointPackageJson, SUPPORTED_FORMAT_PROPERTIES} from '../../src/packages/entry_point';
2223
import {EntryPointManifestFile} from '../../src/packages/entry_point_manifest';
2324
import {Transformer} from '../../src/packages/transformer';
2425
import {DirectPackageJsonUpdater, PackageJsonUpdater} from '../../src/writing/package_json_updater';
25-
import {mockRequireResolveForLockfile} from '../helpers/utils';
26+
import {mockRequireResolveForLockfile, mockRequireResolveForLockFileUnlockerScript, mockRequireResolveForWorkerScript} from '../helpers/utils';
2627

2728
import {loadNgccIntegrationTestFiles} from './util';
2829

@@ -47,7 +48,11 @@ runInEachFileSystem(() => {
4748
_ = absoluteFrom;
4849
fs = getFileSystem();
4950
pkgJsonUpdater = new DirectPackageJsonUpdater(fs);
51+
5052
mockRequireResolveForLockfile();
53+
mockRequireResolveForWorkerScript();
54+
mockRequireResolveForLockFileUnlockerScript();
55+
5156
initMockFileSystem(fs, testFiles);
5257

5358
// Force single-process execution in unit tests by mocking available CPUs to 1.
@@ -71,7 +76,7 @@ runInEachFileSystem(() => {
7176
fs.ensureDir(fs.join(pkgPath, 'fesm5'));
7277
fs.writeFile(
7378
fs.join(pkgPath, 'fesm5/core.js'),
74-
realFs.readFileSync(require.resolve('../fesm5_angular_core.js'), 'utf8'));
79+
realFs.readFileSync(runfiles.resolvePackageRelative('./fesm5_angular_core.js'), 'utf8'));
7580

7681
pkgJson.esm5 = './fesm5/core.js';
7782
pkgJson.fesm5 = './fesm5/core.js';
@@ -3940,7 +3945,7 @@ runInEachFileSystem(() => {
39403945
function initMockFileSystem(fs: FileSystem, testFiles: Folder) {
39413946
if (fs instanceof MockFileSystem) {
39423947
fs.init(testFiles);
3943-
fs.ensureDir(fs.dirname(getLockFilePath(fs)));
3948+
fs.ensureDir(fs.dirname(LockFilePathResolver.resolve(fs)));
39443949
}
39453950

39463951
// a random test package that no metadata.json file so not compiled by Angular.

packages/compiler-cli/ngcc/test/integration/util.ts

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,21 +8,23 @@
88

99
import {AbsoluteFsPath} from '../../../src/ngtsc/file_system';
1010
import {Folder, MockFileSystemPosix} from '../../../src/ngtsc/file_system/testing';
11-
import {loadTestDirectory, loadTsLib, resolveNpmTreeArtifact} from '../../../src/ngtsc/testing';
11+
import {loadTestDirectory, loadTsLib, resolveFromRunfiles} from '../../../src/ngtsc/testing';
1212

1313
export function loadNgccIntegrationTestFiles(): Folder {
1414
const tmpFs = new MockFileSystemPosix(true);
1515
const basePath = '/' as AbsoluteFsPath;
1616

1717
loadTsLib(tmpFs, basePath);
1818
loadTestDirectory(
19-
tmpFs, resolveNpmTreeArtifact('@angular/core-12'),
19+
tmpFs, resolveFromRunfiles('npm/node_modules/@angular/core-12'),
2020
tmpFs.resolve('/node_modules/@angular/core'));
2121
loadTestDirectory(
22-
tmpFs, resolveNpmTreeArtifact('@angular/common-12'),
22+
tmpFs, resolveFromRunfiles('npm/node_modules/@angular/common-12'),
2323
tmpFs.resolve('/node_modules/@angular/common'));
2424
loadTestDirectory(
25-
tmpFs, resolveNpmTreeArtifact('typescript'), tmpFs.resolve('/node_modules/typescript'));
26-
loadTestDirectory(tmpFs, resolveNpmTreeArtifact('rxjs'), tmpFs.resolve('/node_modules/rxjs'));
25+
tmpFs, resolveFromRunfiles('npm/node_modules/typescript'),
26+
tmpFs.resolve('/node_modules/typescript'));
27+
loadTestDirectory(
28+
tmpFs, resolveFromRunfiles('npm/node_modules/rxjs'), tmpFs.resolve('/node_modules/rxjs'));
2729
return tmpFs.dump();
2830
}

packages/compiler-cli/ngcc/test/locking/lockfile_with_child_process/index_spec.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import * as process from 'process';
1111
import {FileSystem, getFileSystem} from '../../../../src/ngtsc/file_system';
1212
import {runInEachFileSystem} from '../../../../src/ngtsc/file_system/testing';
1313
import {MockLogger} from '../../../../src/ngtsc/logging/testing';
14-
import {getLockFilePath} from '../../../src/locking/lock_file';
14+
import {LockFilePathResolver} from '../../../src/locking/lock_file';
1515
import {LockFileWithChildProcess} from '../../../src/locking/lock_file_with_child_process';
1616
import {mockRequireResolveForLockfile} from '../../helpers/utils';
1717

@@ -72,10 +72,11 @@ runInEachFileSystem(() => {
7272
it('should write the lock-file to disk', () => {
7373
const fs = getFileSystem();
7474
const lockFile = new LockFileUnderTest(fs);
75-
expect(fs.exists(getLockFilePath(fs))).toBe(false);
75+
const lockPath = LockFilePathResolver.resolve(fs);
76+
expect(fs.exists(lockPath)).toBe(false);
7677
lockFile.write();
77-
expect(fs.exists(getLockFilePath(fs))).toBe(true);
78-
expect(fs.readFile(getLockFilePath(fs))).toEqual('' + process.pid);
78+
expect(fs.exists(lockPath)).toBe(true);
79+
expect(fs.readFile(lockPath)).toEqual('' + process.pid);
7980
});
8081

8182
it('should create the unlocker process if it is not already created', () => {

0 commit comments

Comments
 (0)