From b7695a0258ea4da05ca01cbeb00565a04caeb4f2 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Sun, 13 Jan 2019 01:01:27 +0800 Subject: [PATCH 1/2] src: pass cli options to bootstrap/loaders.js lexically Instead of using `internalBinding('config')` which should be used to carry information about build-time options, directly pass the run-time cli options into bootstrap/loaders.js lexically via function arguments. --- lib/internal/bootstrap/loaders.js | 7 +++---- src/node.cc | 13 +++++++++++-- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/lib/internal/bootstrap/loaders.js b/lib/internal/bootstrap/loaders.js index 300a362abb5f56..fa7ffe14cb98ed 100644 --- a/lib/internal/bootstrap/loaders.js +++ b/lib/internal/bootstrap/loaders.js @@ -42,7 +42,7 @@ // This file is compiled as if it's wrapped in a function with arguments // passed by node::LoadEnvironment() /* global process, getBinding, getLinkedBinding, getInternalBinding */ -/* global debugBreak */ +/* global debugBreak, experimentalModules, exposeInternals */ if (debugBreak) debugger; // eslint-disable-line no-debugger @@ -162,7 +162,6 @@ internalBinding('module_wrap').callbackMap = new WeakMap(); // written in CommonJS style. const loaderExports = { internalBinding, NativeModule }; const loaderId = 'internal/bootstrap/loaders'; -const config = internalBinding('config'); // Set up NativeModule. function NativeModule(id) { @@ -177,7 +176,7 @@ function NativeModule(id) { // Do not expose this to user land even with --expose-internals. this.canBeRequiredByUsers = false; } else if (id.startsWith('internal/')) { - this.canBeRequiredByUsers = config.exposeInternals; + this.canBeRequiredByUsers = exposeInternals; } else { this.canBeRequiredByUsers = true; } @@ -316,7 +315,7 @@ NativeModule.prototype.compile = function() { const fn = compileFunction(id); fn(this.exports, requireFn, this, process, internalBinding); - if (config.experimentalModules && this.canBeRequiredByUsers) { + if (experimentalModules && this.canBeRequiredByUsers) { this.proxifyExports(); } diff --git a/src/node.cc b/src/node.cc index d5f3f4e6aad2bf..7a2dfd2186c764 100644 --- a/src/node.cc +++ b/src/node.cc @@ -676,7 +676,12 @@ void RunBootstrapping(Environment* env) { FIXED_ONE_BYTE_STRING(isolate, "getBinding"), FIXED_ONE_BYTE_STRING(isolate, "getLinkedBinding"), FIXED_ONE_BYTE_STRING(isolate, "getInternalBinding"), - FIXED_ONE_BYTE_STRING(isolate, "debugBreak")}; + // --inspect-brk-node + FIXED_ONE_BYTE_STRING(isolate, "debugBreak"), + // --experimental-modules + FIXED_ONE_BYTE_STRING(isolate, "experimentalModules"), + // --expose-internals + FIXED_ONE_BYTE_STRING(isolate, "exposeInternals")}; std::vector> loaders_args = { process, env->NewFunctionTemplate(binding::GetBinding) @@ -689,7 +694,11 @@ void RunBootstrapping(Environment* env) { ->GetFunction(context) .ToLocalChecked(), Boolean::New(isolate, - env->options()->debug_options().break_node_first_line)}; + env->options()->debug_options().break_node_first_line), + Boolean::New(isolate, + env->options()->experimental_modules), + Boolean::New(isolate, + env->options()->expose_internals)}; MaybeLocal loader_exports; // Bootstrap internal loaders From fb0d83ac30ff042f2ef966d5a316c30dfb116c58 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Sun, 13 Jan 2019 01:03:49 +0800 Subject: [PATCH 2/2] src: remove unused `internalBinding('config')` properties Remove the following properties: - `preserveSymlinks` - `preserveSymlinksMain` - `experimentalModules` - `userLoader` - `experimentalVMModules` - `experimentalREPLAwait` - `exposeInternals` We used to use them to pass cli option values from C++ into JS, but now the values are obtained in JS land using `require('internal/options').getOptionValue` instead so they are unused. Also removes `test/parallel/test-internal-modules-expose.js` which tests `--expose-internals`. We already have hundreds of tests depending on `--expose-internals`, they are more than enough to test the functionality of the flag. --- src/node_config.cc | 22 ------------------- test/parallel/test-internal-modules-expose.js | 12 ---------- 2 files changed, 34 deletions(-) delete mode 100644 test/parallel/test-internal-modules-expose.js diff --git a/src/node_config.cc b/src/node_config.cc index c244d1d9ff1260..555685dd457ad2 100644 --- a/src/node_config.cc +++ b/src/node_config.cc @@ -69,28 +69,6 @@ static void Initialize(Local target, #endif // NODE_HAVE_I18N_SUPPORT - if (env->options()->preserve_symlinks) - READONLY_TRUE_PROPERTY(target, "preserveSymlinks"); - if (env->options()->preserve_symlinks_main) - READONLY_TRUE_PROPERTY(target, "preserveSymlinksMain"); - - if (env->options()->experimental_modules) { - READONLY_TRUE_PROPERTY(target, "experimentalModules"); - const std::string& userland_loader = env->options()->userland_loader; - if (!userland_loader.empty()) { - READONLY_STRING_PROPERTY(target, "userLoader", userland_loader); - } - } - - if (env->options()->experimental_vm_modules) - READONLY_TRUE_PROPERTY(target, "experimentalVMModules"); - - if (env->options()->experimental_repl_await) - READONLY_TRUE_PROPERTY(target, "experimentalREPLAwait"); - - if (env->options()->expose_internals) - READONLY_TRUE_PROPERTY(target, "exposeInternals"); - if (env->abort_on_uncaught_exception()) READONLY_TRUE_PROPERTY(target, "shouldAbortOnUncaughtException"); diff --git a/test/parallel/test-internal-modules-expose.js b/test/parallel/test-internal-modules-expose.js deleted file mode 100644 index 5229032573088e..00000000000000 --- a/test/parallel/test-internal-modules-expose.js +++ /dev/null @@ -1,12 +0,0 @@ -'use strict'; -// Flags: --expose_internals - -require('../common'); -const assert = require('assert'); -const { internalBinding } = require('internal/test/binding'); -const config = internalBinding('config'); - -console.log(config, process.argv); - -assert.strictEqual(typeof require('internal/freelist').FreeList, 'function'); -assert.strictEqual(config.exposeInternals, true);