Skip to content

Commit

Permalink
module: explicitly initialize CJS loader
Browse files Browse the repository at this point in the history
Explicitly initialize the CJS loader with `module._initPaths()`
instead of making it a side-effect of requiring
`internal/modules/cjs/loader` - that makes it harder to reason about
when it's safe to load `internal/modules/cjs/loader`.

PR-URL: #27313
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
  • Loading branch information
joyeecheung committed Apr 22, 2019
1 parent 528d100 commit 7c816b7
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 13 deletions.
8 changes: 7 additions & 1 deletion lib/internal/bootstrap/pre_execution.js
Expand Up @@ -48,6 +48,7 @@ function prepareMainThreadExecution(expandArgv1 = false) {


initializeDeprecations(); initializeDeprecations();
initializeFrozenIntrinsics(); initializeFrozenIntrinsics();
initializeCJSLoader();
initializeESMLoader(); initializeESMLoader();
loadPreloadModules(); loadPreloadModules();
} }
Expand Down Expand Up @@ -336,6 +337,10 @@ function initializePolicy() {
} }
} }


function initializeCJSLoader() {
require('internal/modules/cjs/loader')._initPaths();
}

function initializeESMLoader() { function initializeESMLoader() {
const experimentalModules = getOptionValue('--experimental-modules'); const experimentalModules = getOptionValue('--experimental-modules');
const experimentalVMModules = getOptionValue('--experimental-vm-modules'); const experimentalVMModules = getOptionValue('--experimental-vm-modules');
Expand Down Expand Up @@ -397,5 +402,6 @@ module.exports = {
loadPreloadModules, loadPreloadModules,
setupTraceCategoryState, setupTraceCategoryState,
setupInspectorHooks, setupInspectorHooks,
initializeReport initializeReport,
initializeCJSLoader
}; };
17 changes: 7 additions & 10 deletions lib/internal/main/check_syntax.js
Expand Up @@ -18,6 +18,11 @@ const {
stripShebang, stripBOM stripShebang, stripBOM
} = require('internal/modules/cjs/helpers'); } = require('internal/modules/cjs/helpers');


const {
_resolveFilename: resolveCJSModuleName,
wrap: wrapCJSModule
} = require('internal/modules/cjs/loader');

// TODO(joyeecheung): not every one of these are necessary // TODO(joyeecheung): not every one of these are necessary
prepareMainThreadExecution(true); prepareMainThreadExecution(true);


Expand All @@ -26,12 +31,8 @@ if (process.argv[1] && process.argv[1] !== '-') {
const path = require('path'); const path = require('path');
process.argv[1] = path.resolve(process.argv[1]); process.argv[1] = path.resolve(process.argv[1]);


// This has to be done after prepareMainThreadExecution because it
// relies on process.execPath
const CJSModule = require('internal/modules/cjs/loader');

// Read the source. // Read the source.
const filename = CJSModule._resolveFilename(process.argv[1]); const filename = resolveCJSModuleName(process.argv[1]);


const fs = require('fs'); const fs = require('fs');
const source = fs.readFileSync(filename, 'utf-8'); const source = fs.readFileSync(filename, 'utf-8');
Expand All @@ -51,10 +52,6 @@ function checkSyntax(source, filename) {
// Remove Shebang. // Remove Shebang.
source = stripShebang(source); source = stripShebang(source);


// This has to be done after prepareMainThreadExecution because it
// relies on process.execPath
const CJSModule = require('internal/modules/cjs/loader');

const { getOptionValue } = require('internal/options'); const { getOptionValue } = require('internal/options');
const experimentalModules = getOptionValue('--experimental-modules'); const experimentalModules = getOptionValue('--experimental-modules');
if (experimentalModules) { if (experimentalModules) {
Expand All @@ -76,7 +73,7 @@ function checkSyntax(source, filename) {
// Remove BOM. // Remove BOM.
source = stripBOM(source); source = stripBOM(source);
// Wrap it. // Wrap it.
source = CJSModule.wrap(source); source = wrapCJSModule(source);
// Compile the script, this will throw if it fails. // Compile the script, this will throw if it fails.
new vm.Script(source, { displayErrors: true, filename }); new vm.Script(source, { displayErrors: true, filename });
} }
2 changes: 2 additions & 0 deletions lib/internal/main/worker_thread.js
Expand Up @@ -12,6 +12,7 @@ const {
setupWarningHandler, setupWarningHandler,
setupDebugEnv, setupDebugEnv,
initializeDeprecations, initializeDeprecations,
initializeCJSLoader,
initializeESMLoader, initializeESMLoader,
initializeFrozenIntrinsics, initializeFrozenIntrinsics,
initializeReport, initializeReport,
Expand Down Expand Up @@ -104,6 +105,7 @@ port.on('message', (message) => {
} }
initializeDeprecations(); initializeDeprecations();
initializeFrozenIntrinsics(); initializeFrozenIntrinsics();
initializeCJSLoader();
initializeESMLoader(); initializeESMLoader();
loadPreloadModules(); loadPreloadModules();
publicWorker.parentPort = publicPort; publicWorker.parentPort = publicPort;
Expand Down
2 changes: 0 additions & 2 deletions lib/internal/modules/cjs/loader.js
Expand Up @@ -893,8 +893,6 @@ Module._preloadModules = function(requests) {
parent.require(requests[n]); parent.require(requests[n]);
}; };


Module._initPaths();

// Backwards compatibility // Backwards compatibility
Module.Module = Module; Module.Module = Module;


Expand Down

0 comments on commit 7c816b7

Please sign in to comment.