Skip to content

Commit

Permalink
module: reduce circular dependency of internal/modules/cjs/loader
Browse files Browse the repository at this point in the history
Previously `internal/bootstrap/pre_execution.js` requires
`internal/modules/cjs/loader.js` which in turn requires
`internal/bootstrap/pre_execution.js`. This patch moves the
entry point execution logic out of `pre_execution.js` and
puts it into `internal/modules/run_main.js`. It also tests
that `Module.runMain` can be monkey-patched before further
deprecation/refactoring can be done.

Also added an internal assertion `hasLoadedAnyUserCJSModule`
for documentation purposes.

PR-URL: #30349
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
  • Loading branch information
joyeecheung authored and addaleax committed Nov 19, 2019
1 parent 0f78dcc commit efce655
Show file tree
Hide file tree
Showing 25 changed files with 152 additions and 101 deletions.
67 changes: 9 additions & 58 deletions lib/internal/bootstrap/pre_execution.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ const { Object, SafeWeakMap } = primordials;
const { getOptionValue } = require('internal/options');
const { Buffer } = require('buffer');
const { ERR_MANIFEST_ASSERT_INTEGRITY } = require('internal/errors').codes;
const path = require('path');
const assert = require('internal/assert');

function prepareMainThreadExecution(expandArgv1 = false) {
// Patch the process object with legacy properties and normalizations
Expand Down Expand Up @@ -60,6 +60,9 @@ function prepareMainThreadExecution(expandArgv1 = false) {
initializeDeprecations();
initializeCJSLoader();
initializeESMLoader();

const CJSLoader = require('internal/modules/cjs/loader');
assert(!CJSLoader.hasLoadedAnyUserCJSModule);
loadPreloadModules();
initializeFrozenIntrinsics();
}
Expand Down Expand Up @@ -394,7 +397,11 @@ function initializePolicy() {
}

function initializeCJSLoader() {
require('internal/modules/cjs/loader').Module._initPaths();
const CJSLoader = require('internal/modules/cjs/loader');
CJSLoader.Module._initPaths();
// TODO(joyeecheung): deprecate this in favor of a proper hook?
CJSLoader.Module.runMain =
require('internal/modules/run_main').executeUserEntryPoint;
}

function initializeESMLoader() {
Expand Down Expand Up @@ -433,67 +440,11 @@ function loadPreloadModules() {
}
}

function resolveMainPath(main) {
const { toRealPath, Module: CJSModule } =
require('internal/modules/cjs/loader');

// Note extension resolution for the main entry point can be deprecated in a
// future major.
let mainPath = CJSModule._findPath(path.resolve(main), null, true);
if (!mainPath)
return;

const preserveSymlinksMain = getOptionValue('--preserve-symlinks-main');
if (!preserveSymlinksMain)
mainPath = toRealPath(mainPath);

return mainPath;
}

function shouldUseESMLoader(mainPath) {
const userLoader = getOptionValue('--experimental-loader');
if (userLoader)
return true;
// Determine the module format of the main
if (mainPath && mainPath.endsWith('.mjs'))
return true;
if (!mainPath || mainPath.endsWith('.cjs'))
return false;
const { readPackageScope } = require('internal/modules/cjs/loader');
const pkg = readPackageScope(mainPath);
return pkg && pkg.data.type === 'module';
}

function runMainESM(mainPath) {
const esmLoader = require('internal/process/esm_loader');
const { pathToFileURL } = require('internal/url');
const { hasUncaughtExceptionCaptureCallback } =
require('internal/process/execution');
return esmLoader.initializeLoader().then(() => {
const main = path.isAbsolute(mainPath) ?
pathToFileURL(mainPath).href : mainPath;
return esmLoader.ESMLoader.import(main);
}).catch((e) => {
if (hasUncaughtExceptionCaptureCallback()) {
process._fatalException(e);
return;
}
internalBinding('errors').triggerUncaughtException(
e,
true /* fromPromise */
);
});
}


module.exports = {
patchProcessObject,
resolveMainPath,
runMainESM,
setupCoverageHooks,
setupWarningHandler,
setupDebugEnv,
shouldUseESMLoader,
prepareMainThreadExecution,
initializeDeprecations,
initializeESMLoader,
Expand Down
10 changes: 6 additions & 4 deletions lib/internal/main/run_main_module.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,12 @@ const {

prepareMainThreadExecution(true);

const CJSModule = require('internal/modules/cjs/loader').Module;

markBootstrapComplete();

// Note: this loads the module through the ESM loader if the module is
// determined to be an ES module
CJSModule.runMain(process.argv[1]);
// determined to be an ES module. This hangs from the CJS module loader
// because we currently allow monkey-patching of the module loaders
// in the preloaded scripts through require('module').
// runMain here might be monkey-patched by users in --require.
// XXX: the monkey-patchability here should probably be deprecated.
require('internal/modules/cjs/loader').Module.runMain(process.argv[1]);
8 changes: 6 additions & 2 deletions lib/internal/main/worker_thread.js
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,9 @@ port.on('message', (message) => {
initializeDeprecations();
initializeCJSLoader();
initializeESMLoader();

const CJSLoader = require('internal/modules/cjs/loader');
assert(!CJSLoader.hasLoadedAnyUserCJSModule);
loadPreloadModules();
initializeFrozenIntrinsics();
publicWorker.parentPort = publicPort;
Expand Down Expand Up @@ -141,8 +144,9 @@ port.on('message', (message) => {
evalScript('[worker eval]', filename);
} else {
// script filename
const CJSModule = require('internal/modules/cjs/loader').Module;
CJSModule.runMain(process.argv[1] = filename);
// runMain here might be monkey-patched by users in --require.
// XXX: the monkey-patchability here should probably be deprecated.
CJSLoader.Module.runMain(process.argv[1] = filename);
}
} else if (message.type === STDIO_PAYLOAD) {
const { stream, chunk, encoding } = message;
Expand Down
27 changes: 10 additions & 17 deletions lib/internal/modules/cjs/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,21 +64,24 @@ const manifest = getOptionValue('--experimental-policy') ?
require('internal/process/policy').manifest :
null;
const { compileFunction } = internalBinding('contextify');

// Whether any user-provided CJS modules had been loaded (executed).
// Used for internal assertions.
let hasLoadedAnyUserCJSModule = false;

const {
ERR_INVALID_ARG_VALUE,
ERR_INVALID_OPT_VALUE,
ERR_INVALID_PACKAGE_CONFIG,
ERR_REQUIRE_ESM
} = require('internal/errors').codes;
const { validateString } = require('internal/validators');
const {
resolveMainPath,
shouldUseESMLoader,
runMainESM
} = require('internal/bootstrap/pre_execution');
const pendingDeprecation = getOptionValue('--pending-deprecation');

module.exports = { wrapSafe, Module, toRealPath, readPackageScope };
module.exports = {
wrapSafe, Module, toRealPath, readPackageScope,
get hasLoadedAnyUserCJSModule() { return hasLoadedAnyUserCJSModule; }
};

let asyncESM, ModuleJob, ModuleWrap, kInstantiated;

Expand Down Expand Up @@ -1172,6 +1175,7 @@ Module.prototype._compile = function(content, filename) {
result = compiledWrapper.call(thisValue, exports, require, module,
filename, dirname);
}
hasLoadedAnyUserCJSModule = true;
if (requireDepth === 0) statCache = null;
return result;
};
Expand Down Expand Up @@ -1240,17 +1244,6 @@ Module._extensions['.node'] = function(module, filename) {
return process.dlopen(module, path.toNamespacedPath(filename));
};

// Bootstrap main module.
Module.runMain = function(main = process.argv[1]) {
const resolvedMain = resolveMainPath(main);
const useESMLoader = shouldUseESMLoader(resolvedMain);
if (useESMLoader) {
runMainESM(resolvedMain || main);
} else {
Module._load(main, null, true);
}
};

function createRequireFromPath(filename) {
// Allow a directory to be passed as the filename
const trailingSlash =
Expand Down
73 changes: 73 additions & 0 deletions lib/internal/modules/run_main.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
'use strict';

const CJSLoader = require('internal/modules/cjs/loader');
const { Module, toRealPath, readPackageScope } = CJSLoader;
const { getOptionValue } = require('internal/options');
const path = require('path');

function resolveMainPath(main) {
// Note extension resolution for the main entry point can be deprecated in a
// future major.
// Module._findPath is monkey-patchable here.
let mainPath = Module._findPath(path.resolve(main), null, true);
if (!mainPath)
return;

const preserveSymlinksMain = getOptionValue('--preserve-symlinks-main');
if (!preserveSymlinksMain)
mainPath = toRealPath(mainPath);

return mainPath;
}

function shouldUseESMLoader(mainPath) {
const userLoader = getOptionValue('--experimental-loader');
if (userLoader)
return true;
// Determine the module format of the main
if (mainPath && mainPath.endsWith('.mjs'))
return true;
if (!mainPath || mainPath.endsWith('.cjs'))
return false;
const pkg = readPackageScope(mainPath);
return pkg && pkg.data.type === 'module';
}

function runMainESM(mainPath) {
const esmLoader = require('internal/process/esm_loader');
const { pathToFileURL } = require('internal/url');
const { hasUncaughtExceptionCaptureCallback } =
require('internal/process/execution');
return esmLoader.initializeLoader().then(() => {
const main = path.isAbsolute(mainPath) ?
pathToFileURL(mainPath).href : mainPath;
return esmLoader.ESMLoader.import(main);
}).catch((e) => {
if (hasUncaughtExceptionCaptureCallback()) {
process._fatalException(e);
return;
}
internalBinding('errors').triggerUncaughtException(
e,
true /* fromPromise */
);
});
}

// For backwards compatibility, we have to run a bunch of
// monkey-patchable code that belongs to the CJS loader (exposed by
// `require('module')`) even when the entry point is ESM.
function executeUserEntryPoint(main = process.argv[1]) {
const resolvedMain = resolveMainPath(main);
const useESMLoader = shouldUseESMLoader(resolvedMain);
if (useESMLoader) {
runMainESM(resolvedMain || main);
} else {
// Module._load is the monkey-patchable CJS module loader.
Module._load(main, null, true);
}
}

module.exports = {
executeUserEntryPoint
};
1 change: 1 addition & 0 deletions node.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@
'lib/internal/main/run_main_module.js',
'lib/internal/main/run_third_party_main.js',
'lib/internal/main/worker_thread.js',
'lib/internal/modules/run_main.js',
'lib/internal/modules/cjs/helpers.js',
'lib/internal/modules/cjs/loader.js',
'lib/internal/modules/esm/loader.js',
Expand Down
8 changes: 8 additions & 0 deletions test/fixtures/monkey-patch-run-main.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
'use strict';

const oldRunMain = require('module').runMain;

require('module').runMain = function(...args) {
console.log('runMain is monkey patched!');
oldRunMain(...args);
};
2 changes: 1 addition & 1 deletion test/message/core_line_numbers.out
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,5 @@ RangeError: Invalid input
at Object.Module._extensions..js (internal/modules/cjs/loader.js:*:*)
at Module.load (internal/modules/cjs/loader.js:*:*)
at Function.Module._load (internal/modules/cjs/loader.js:*:*)
at Function.Module.runMain (internal/modules/cjs/loader.js:*:*)
at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:*:*)
at internal/main/run_main_module.js:*:*
2 changes: 1 addition & 1 deletion test/message/error_exit.out
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:
at Object.Module._extensions..js (internal/modules/cjs/loader.js:*:*)
at Module.load (internal/modules/cjs/loader.js:*:*)
at Function.Module._load (internal/modules/cjs/loader.js:*:*)
at Function.Module.runMain (internal/modules/cjs/loader.js:*:*)
at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:*:*)
at internal/main/run_main_module.js:*:* {
generatedMessage: true,
code: 'ERR_ASSERTION',
Expand Down
4 changes: 2 additions & 2 deletions test/message/esm_loader_not_found.out
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ Error: Cannot find package 'i-dont-exist' imported from *
at Loader.import (internal/modules/esm/loader.js:*:*)
at internal/process/esm_loader.js:*:*
at Object.initializeLoader (internal/process/esm_loader.js:*:*)
at runMainESM (internal/bootstrap/pre_execution.js:*:*)
at Function.Module.runMain (internal/modules/cjs/loader.js:*:*)
at runMainESM (internal/modules/run_main.js:*:*)
at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:*:*)
at internal/main/run_main_module.js:*:* {
code: 'ERR_MODULE_NOT_FOUND'
}
2 changes: 1 addition & 1 deletion test/message/events_unhandled_error_common_trace.out
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ Error: foo:bar
at Object.Module._extensions..js (internal/modules/cjs/loader.js:*:*)
at Module.load (internal/modules/cjs/loader.js:*:*)
at Function.Module._load (internal/modules/cjs/loader.js:*:*)
at Function.Module.runMain (internal/modules/cjs/loader.js:*:*)
at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:*:*)
at internal/main/run_main_module.js:*:*
Emitted 'error' event at:
at quux (*events_unhandled_error_common_trace.js:*:*)
Expand Down
2 changes: 1 addition & 1 deletion test/message/events_unhandled_error_nexttick.out
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ Error
at Object.Module._extensions..js (internal/modules/cjs/loader.js:*:*)
at Module.load (internal/modules/cjs/loader.js:*:*)
at Function.Module._load (internal/modules/cjs/loader.js:*:*)
at Function.Module.runMain (internal/modules/cjs/loader.js:*:*)
at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:*:*)
at internal/main/run_main_module.js:*:*
Emitted 'error' event at:
at *events_unhandled_error_nexttick.js:*:*
Expand Down
2 changes: 1 addition & 1 deletion test/message/events_unhandled_error_sameline.out
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ Error
at Object.Module._extensions..js (internal/modules/cjs/loader.js:*:*)
at Module.load (internal/modules/cjs/loader.js:*:*)
at Function.Module._load (internal/modules/cjs/loader.js:*:*)
at Function.Module.runMain (internal/modules/cjs/loader.js:*:*)
at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:*:*)
at internal/main/run_main_module.js:*:*
Emitted 'error' event at:
at Object.<anonymous> (*events_unhandled_error_sameline.js:*:*)
Expand Down
2 changes: 1 addition & 1 deletion test/message/events_unhandled_error_subclass.out
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ Error
at Object.Module._extensions..js (internal/modules/cjs/loader.js:*:*)
at Module.load (internal/modules/cjs/loader.js:*:*)
at Function.Module._load (internal/modules/cjs/loader.js:*:*)
at Function.Module.runMain (internal/modules/cjs/loader.js:*:*)
at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:*:*)
at internal/main/run_main_module.js:*:*
Emitted 'error' event on Foo instance at:
at Object.<anonymous> (*events_unhandled_error_subclass.js:*:*)
Expand Down
4 changes: 2 additions & 2 deletions test/message/if-error-has-good-stack.out
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ AssertionError [ERR_ASSERTION]: ifError got unwanted exception: test error
at Object.Module._extensions..js (internal/modules/cjs/loader.js:*:*)
at Module.load (internal/modules/cjs/loader.js:*:*)
at Function.Module._load (internal/modules/cjs/loader.js:*:*)
at Function.Module.runMain (internal/modules/cjs/loader.js:*:*)
at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:*:*)
at internal/main/run_main_module.js:*:* {
generatedMessage: false,
code: 'ERR_ASSERTION',
Expand All @@ -28,7 +28,7 @@ AssertionError [ERR_ASSERTION]: ifError got unwanted exception: test error
at Object.Module._extensions..js (internal/modules/cjs/loader.js:*:*)
at Module.load (internal/modules/cjs/loader.js:*:*)
at Function.Module._load (internal/modules/cjs/loader.js:*:*)
at Function.Module.runMain (internal/modules/cjs/loader.js:*:*)
at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:*:*)
at internal/main/run_main_module.js:*:*
expected: null,
operator: 'ifError'
Expand Down
2 changes: 1 addition & 1 deletion test/message/throw_error_with_getter_throw_traced.out
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,4 @@ Thrown at:
at Module._extensions..js (internal/modules/cjs/loader.js:*:*)
at Module.load (internal/modules/cjs/loader.js:*:*)
at Module._load (internal/modules/cjs/loader.js:*:*)
at Module.runMain (internal/modules/cjs/loader.js:*:*)
at executeUserEntryPoint (internal/modules/run_main.js:*:*)
2 changes: 1 addition & 1 deletion test/message/throw_null_traced.out
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,4 @@ Thrown at:
at Module._extensions..js (internal/modules/cjs/loader.js:*:*)
at Module.load (internal/modules/cjs/loader.js:*:*)
at Module._load (internal/modules/cjs/loader.js:*:*)
at Module.runMain (internal/modules/cjs/loader.js:*:*)
at executeUserEntryPoint (internal/modules/run_main.js:*:*)
2 changes: 1 addition & 1 deletion test/message/throw_undefined_traced.out
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,4 @@ Thrown at:
at Module._extensions..js (internal/modules/cjs/loader.js:*:*)
at Module.load (internal/modules/cjs/loader.js:*:*)
at Module._load (internal/modules/cjs/loader.js:*:*)
at Module.runMain (internal/modules/cjs/loader.js:*:*)
at executeUserEntryPoint (internal/modules/run_main.js:*:*)
2 changes: 1 addition & 1 deletion test/message/undefined_reference_in_new_context.out
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,4 @@ ReferenceError: foo is not defined
at *..js (internal/modules/cjs/loader.js:*)
at Module.load (internal/modules/cjs/loader.js:*)
at Function.Module._load (internal/modules/cjs/loader.js:*:*)
at Function.Module.runMain (internal/modules/cjs/loader.js:*:*)
at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:*:*)
Loading

0 comments on commit efce655

Please sign in to comment.