Skip to content

Commit

Permalink
refactor(compiler-cli): do not use __filename or __dirname global…
Browse files Browse the repository at this point in the history
… for ESM compatibility (#43431)

Switches the compiler-cli usage of `__filename` to `import.meta.url`
when ESM bundles are generated. Unfortunately we cannot start using
only `import.meta` yet as we still build and run all code in Angular
in CommonJS module output for devmode tests.

This commit also fixes various instances where a jasmine spy was applied on
a namespace export that will break with ES module (and the interop for
CommonJS output). We fix these spies by using a default import.

PR Close #43431
  • Loading branch information
devversion authored and dylhunn committed Oct 1, 2021
1 parent b46b3cf commit b1d6fad
Show file tree
Hide file tree
Showing 15 changed files with 113 additions and 16 deletions.
7 changes: 7 additions & 0 deletions packages/compiler-cli/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,11 @@ ts_config(
deps = ["//packages:tsconfig-build.json"],
)

ts_library(
name = "import_meta_url_types",
srcs = ["import_meta_url.d.ts"],
)

ts_library(
name = "compiler-cli",
srcs = glob(
Expand All @@ -76,11 +81,13 @@ ts_library(
"src/**/*.ts",
],
exclude = [
"import_meta_url.d.ts",
"src/integrationtest/**/*.ts",
],
),
tsconfig = ":tsconfig",
deps = [
":import_meta_url_types",
"//packages/compiler",
"//packages/compiler-cli/src/ngtsc/core",
"//packages/compiler-cli/src/ngtsc/core:api",
Expand Down
14 changes: 14 additions & 0 deletions packages/compiler-cli/import_meta_url.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
/**
* @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
*/


// Note: The `__ESM_IMPORT_META_URL__` identifier will be defined as part of bundling.
// We cannot use `import.meta.url` directly as this code currently still runs in CommonJS
// for devmode. This is a solution allowing for both ESModule and CommonJS to run this file.
// TODO(devversion): replace all of this with `import.meta.url` once devmode is using ESM.
declare const __ESM_IMPORT_META_URL__: string;
1 change: 1 addition & 0 deletions packages/compiler-cli/ngcc/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ ts_library(
"//packages:types",
"//packages/compiler",
"//packages/compiler-cli",
"//packages/compiler-cli:import_meta_url_types",
"//packages/compiler-cli/src/ngtsc/annotations",
"//packages/compiler-cli/src/ngtsc/cycles",
"//packages/compiler-cli/src/ngtsc/diagnostics",
Expand Down
15 changes: 12 additions & 3 deletions packages/compiler-cli/ngcc/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@
* found in the LICENSE file at https://angular.io/license
*/

import {join} from 'path';
import {dirname, join} from 'path';
import {fileURLToPath} from 'url';

import {NodeJSFileSystem, setFileSystem} from '../src/ngtsc/file_system';

import {mainNgcc} from './src/main';
Expand All @@ -23,10 +25,17 @@ export function process(options: AsyncNgccOptions|SyncNgccOptions): void|Promise
return mainNgcc(options);
}


// CommonJS/ESM interop for determining the current file name and containing
// directory. These path is needed for providing an absolute path to the ngcc
// command line entry-point script (for the CLI).
export const containingDirPath =
typeof __dirname !== 'undefined' ? __dirname : dirname(fileURLToPath(__ESM_IMPORT_META_URL__));

/**
* Absolute file path that points to the `ngcc` entry-point.
* Absolute file path that points to the `ngcc` command line entry-point.
*
* This can be used by the Angular CLI to spawn a process running ngcc using
* command line options.
*/
export const ngccMainFilePath = join(__dirname, './main-ngcc.js');
export const ngccMainFilePath = join(containingDirPath, './main-ngcc.js');
17 changes: 15 additions & 2 deletions packages/compiler-cli/ngcc/src/execution/cluster/master.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
/// <reference types="node" />

import cluster from 'cluster';
import module from 'module';

import {AbsoluteFsPath, PathManipulation} from '../../../../src/ngtsc/file_system';
import {Logger} from '../../../../src/ngtsc/logging';
Expand All @@ -21,7 +22,6 @@ import {stringifyTask} from '../tasks/utils';
import {MessageFromWorker, TaskCompletedMessage, TransformedFilesMessage, UpdatePackageJsonMessage} from './api';
import {Deferred, sendMessageToWorker} from './utils';


/**
* The cluster master is responsible for analyzing all entry-points, planning the work that needs to
* be done, distributing it to worker-processes and collecting/post-processing the results.
Expand All @@ -44,7 +44,7 @@ export class ClusterMaster {
}

// Set the worker entry-point
cluster.setupMaster({exec: this.fileSystem.resolve(__dirname, 'ngcc_cluster_worker.js')});
cluster.setupMaster({exec: getClusterWorkerScriptPath(fileSystem)});

this.taskQueue = analyzeEntryPoints();
this.onTaskCompleted = createTaskCompletedCallback(this.taskQueue);
Expand Down Expand Up @@ -330,3 +330,16 @@ export class ClusterMaster {
};
}
}

/** Gets the absolute file path to the cluster worker script. */
export function getClusterWorkerScriptPath(fileSystem: PathManipulation): AbsoluteFsPath {
// This is an interop allowing for the worker script to be determined in both
// a CommonJS module, or an ES module which does not come with `require` by default.
const requireFn =
typeof require !== 'undefined' ? require : module.createRequire(__ESM_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);
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/
import {ChildProcess, fork} from 'child_process';
import module from 'module';

import {AbsoluteFsPath, FileSystem} from '../../../../src/ngtsc/file_system';
import {Logger, LogLevel} from '../../../../src/ngtsc/logging';
Expand Down Expand Up @@ -78,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(
__dirname + '/ngcc_lock_unlocker.js', [path, logLevel],
getLockFileUnlockerScriptPath(this.fs), [path, logLevel],
{detached: true, stdio: isWindows ? 'pipe' : 'inherit'});
if (isWindows) {
unlocker.stdout?.on('data', process.stdout.write.bind(process.stdout));
Expand All @@ -87,3 +88,16 @@ export class LockFileWithChildProcess implements LockFile {
return unlocker;
}
}

/** Gets the absolute file path to the lock file unlocker script. */
export function getLockFileUnlockerScriptPath(fileSystem: FileSystem): AbsoluteFsPath {
// This is an interop allowing for the unlocking script to be determined in both
// a CommonJS module, or an ES module which does not come with `require` by default.
const requireFn =
typeof require !== 'undefined' ? require : module.createRequire(__ESM_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);
}
9 changes: 8 additions & 1 deletion packages/compiler-cli/ngcc/src/packages/configuration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/
import {createHash} from 'crypto';
import module from 'module';
import semver from 'semver';
import * as vm from 'vm';

Expand Down Expand Up @@ -267,6 +268,11 @@ export const DEFAULT_NGCC_CONFIG: NgccProjectConfig = {

const NGCC_CONFIG_FILENAME = 'ngcc.config.js';

// CommonJS/ESM interop for determining the current file name and containing
// directory. The path is needed for loading the user configuration.
const isCommonJS = typeof require !== 'undefined';
const currentFileUrl = isCommonJS ? null : __ESM_IMPORT_META_URL__;

/**
* The processed package level configuration as a result of processing a raw package level config.
*/
Expand Down Expand Up @@ -435,12 +441,13 @@ export class NgccConfiguration {
}

private evalSrcFile(srcPath: AbsoluteFsPath): any {
const requireFn = isCommonJS ? require : module.createRequire(currentFileUrl!);
const src = this.fs.readFile(srcPath);
const theExports = {};
const sandbox = {
module: {exports: theExports},
exports: theExports,
require,
require: requireFn,
__dirname: this.fs.dirname(srcPath),
__filename: srcPath
};
Expand Down
6 changes: 3 additions & 3 deletions packages/compiler-cli/ngcc/test/integration/ngcc_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@
*/

/// <reference types="node" />
import {readFileSync} from 'fs';
import * as os from 'os';
import realFs from 'fs';
import os from 'os';

import {absoluteFrom, AbsoluteFsPath, FileSystem, getFileSystem} from '../../../src/ngtsc/file_system';
import {Folder, MockFileSystem, runInEachFileSystem, TestFile} from '../../../src/ngtsc/file_system/testing';
Expand Down Expand Up @@ -69,7 +69,7 @@ runInEachFileSystem(() => {
fs.ensureDir(fs.join(pkgPath, 'fesm5'));
fs.writeFile(
fs.join(pkgPath, 'fesm5/core.js'),
readFileSync(require.resolve('../fesm5_angular_core.js'), 'utf8'));
realFs.readFileSync(require.resolve('../fesm5_angular_core.js'), 'utf8'));

pkgJson.esm5 = './fesm5/core.js';
pkgJson.fesm5 = './fesm5/core.js';
Expand Down
5 changes: 4 additions & 1 deletion packages/compiler-cli/ngcc/test/ngcc_options_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@
* 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';
// 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.
import os from 'os';

import {absoluteFrom, AbsoluteFsPath, FileSystem, getFileSystem} from '../../src/ngtsc/file_system';
import {runInEachFileSystem} from '../../src/ngtsc/file_system/testing';
Expand Down
6 changes: 6 additions & 0 deletions packages/compiler-cli/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,12 @@
"./private/migrations": {
"types": "./private/migrations.d.ts",
"default": "./bundles/private/migrations.js"
},
"./ngcc/src/execution/cluster/ngcc_cluster_worker": {
"default": "./bundles/ngcc/src/execution/cluster/ngcc_cluster_worker.js"
},
"./ngcc/src/locking/lock_file_with_child_process/ngcc_lock_unlocker": {
"default": "./bundles/ngcc/src/locking/lock_file_with_child_process/ngcc_lock_unlocker.js"
}
},
"dependencies": {
Expand Down
3 changes: 3 additions & 0 deletions packages/compiler-cli/src/ngtsc/file_system/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ ts_library(
"src/**/*.ts",
]),
deps = [
# Allows for the `import.meta.url` constant to be used. This is
# a temporary interop until devmode is switched to ESM.
"//packages/compiler-cli:import_meta_url_types",
"@npm//@types/node",
"@npm//typescript",
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,10 @@
*/
/// <reference types="node" />
import * as fs from 'fs';
import module from 'module';
import * as p from 'path';
import {fileURLToPath} from 'url';

import {AbsoluteFsPath, FileStats, FileSystem, PathManipulation, PathSegment, PathString, ReadonlyFileSystem} from './types';

/**
Expand Down Expand Up @@ -51,6 +54,14 @@ export class NodeJSPathManipulation implements PathManipulation {
}
}

// CommonJS/ESM interop for determining the current file name and containing
// directory. These paths are needed for detecting whether the file system
// is case sensitive, or for finding the TypeScript default libraries.
// TODO(devversion): Simplify this in the future if devmode uses ESM as well.
const isCommonJS = typeof __filename !== 'undefined';
const currentFileUrl = isCommonJS ? null : __ESM_IMPORT_META_URL__;
const currentFileName = isCommonJS ? __filename : fileURLToPath(currentFileUrl!);

/**
* A wrapper around the Node.js file-system that supports readonly operations and path manipulation.
*/
Expand All @@ -60,7 +71,7 @@ export class NodeJSReadonlyFileSystem extends NodeJSPathManipulation implements
if (this._caseSensitive === undefined) {
// Note the use of the real file-system is intentional:
// `this.exists()` relies upon `isCaseSensitive()` so that would cause an infinite recursion.
this._caseSensitive = !fs.existsSync(this.normalize(toggleCase(__filename)));
this._caseSensitive = !fs.existsSync(this.normalize(toggleCase(currentFileName)));
}
return this._caseSensitive;
}
Expand All @@ -86,7 +97,9 @@ export class NodeJSReadonlyFileSystem extends NodeJSPathManipulation implements
return this.resolve(fs.realpathSync(path));
}
getDefaultLibLocation(): AbsoluteFsPath {
return this.resolve(require.resolve('typescript'), '..');
// TODO(devversion): Once devmode output uses ESM, we can simplify this.
const requireFn = isCommonJS ? require : module.createRequire(currentFileUrl!);
return this.resolve(requireFn.resolve('typescript'), '..');
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,12 @@
* 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 realFs from 'fs';
// 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.
import realFs from 'fs';
import * as os from 'os';

import {NodeJSFileSystem, NodeJSPathManipulation, NodeJSReadonlyFileSystem} from '../src/node_js_file_system';
import {AbsoluteFsPath, PathSegment} from '../src/types';

Expand Down
2 changes: 1 addition & 1 deletion packages/language-service/test/reflector_host_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/

import * as path from 'path';
import path from 'path';
import ts from 'typescript';

import {ReflectorHost} from '../src/reflector_host';
Expand Down
5 changes: 4 additions & 1 deletion packages/language-service/test/typescript_host_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,10 @@
* found in the LICENSE file at https://angular.io/license
*/

import * as path from 'path';
// 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 `path`. Using
// the default export gives us an object where we can patch properties on.
import path from 'path';
import ts from 'typescript';

import {TypeScriptServiceHost} from '../src/typescript_host';
Expand Down

0 comments on commit b1d6fad

Please sign in to comment.