From 59257af33c85a01cf5c34632587ae86b918cd646 Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Sun, 3 Mar 2019 16:46:43 +0200 Subject: [PATCH 1/2] esm: deprecate global.process, global.Buffer in ES Modules --- lib/internal/bootstrap/loaders.js | 3 +- lib/internal/bootstrap/node.js | 5 ++ lib/internal/bootstrap/pre_execution.js | 37 ++++++++---- lib/internal/modules/cjs/loader.js | 60 +++++++++---------- src/node_contextify.cc | 27 +++++++++ src/node_contextify.h | 2 + test/common/index.mjs | 7 +-- test/message/async_error_sync_esm.out | 1 - test/parallel/test-global-buffer-dep.mjs | 9 +++ .../parallel/test-global-process-dep-eval.mjs | 9 +++ .../test-global-process-dep-nested-eval.mjs | 9 +++ test/parallel/test-global-process-dep.mjs | 9 +++ 12 files changed, 129 insertions(+), 49 deletions(-) create mode 100644 test/parallel/test-global-buffer-dep.mjs create mode 100644 test/parallel/test-global-process-dep-eval.mjs create mode 100644 test/parallel/test-global-process-dep-nested-eval.mjs create mode 100644 test/parallel/test-global-process-dep.mjs diff --git a/lib/internal/bootstrap/loaders.js b/lib/internal/bootstrap/loaders.js index 7a98e4c96c87d4..83a3bafeba7bad 100644 --- a/lib/internal/bootstrap/loaders.js +++ b/lib/internal/bootstrap/loaders.js @@ -140,7 +140,8 @@ let internalBinding; const loaderExports = { internalBinding, NativeModule, - require: nativeModuleRequire + require: nativeModuleRequire, + cjsContext: { __proto__: null, process: undefined, Buffer: undefined } }; const loaderId = 'internal/bootstrap/loaders'; diff --git a/lib/internal/bootstrap/node.js b/lib/internal/bootstrap/node.js index 22dabf4d7ed459..3ee8bb48e8bb68 100644 --- a/lib/internal/bootstrap/node.js +++ b/lib/internal/bootstrap/node.js @@ -305,6 +305,7 @@ function setupPrepareStackTrace() { } function setupProcessObject() { + const { cjsContext } = require('internal/bootstrap/loaders'); const EventEmitter = require('events'); const origProcProto = Object.getPrototypeOf(process); Object.setPrototypeOf(origProcProto, EventEmitter.prototype); @@ -322,6 +323,7 @@ function setupProcessObject() { writable: true, configurable: true }); + cjsContext.process = process; } function setupProcessStdio(getStdout, getStdin, getStderr) { @@ -390,6 +392,7 @@ function setupGlobalProxy() { function setupBuffer() { const { Buffer } = require('buffer'); + const { cjsContext } = require('internal/bootstrap/loaders'); const bufferBinding = internalBinding('buffer'); // Only after this point can C++ use Buffer::New() @@ -397,12 +400,14 @@ function setupBuffer() { delete bufferBinding.setBufferPrototype; delete bufferBinding.zeroFill; + // Make process globally available to users by putting it on the global proxy Object.defineProperty(global, 'Buffer', { value: Buffer, enumerable: false, writable: true, configurable: true }); + cjsContext.Buffer = Buffer; } function createGlobalConsole(consoleFromVM) { diff --git a/lib/internal/bootstrap/pre_execution.js b/lib/internal/bootstrap/pre_execution.js index e9571a89a62857..6fed21fdbad687 100644 --- a/lib/internal/bootstrap/pre_execution.js +++ b/lib/internal/bootstrap/pre_execution.js @@ -3,7 +3,6 @@ const { Object, SafeWeakMap } = primordials; const { getOptionValue } = require('internal/options'); -const { Buffer } = require('buffer'); function prepareMainThreadExecution(expandArgv1 = false) { // Patch the process object with legacy properties and normalizations @@ -262,28 +261,46 @@ function initializeDeprecations() { 'Please use public APIs instead.', 'DEP0111'); } - // Create global.process and global.Buffer as getters so that we have a - // deprecation path for these in ES Modules. - // See https://github.com/nodejs/node/pull/26334. - let _process = process; + const { cjsContext } = require('internal/bootstrap/loaders'); + const { getStackSourceName } = internalBinding('contextify'); + const experimentalModules = getOptionValue('--experimental-modules'); + + // Deprecate global.process and global.Buffer access in ES Modules. + const processGetter = deprecate( + () => cjsContext.process, + 'global.process is deprecated in ECMAScript Modules. ' + + 'Please use `import process from \'process\'` instead.', 'DEP0XXX'); Object.defineProperty(global, 'process', { get() { - return _process; + if (experimentalModules) { + const sourceName = getStackSourceName(1); + if (sourceName && sourceName.startsWith('file://')) + return processGetter(); + } + return cjsContext.process; }, set(value) { - _process = value; + cjsContext.process = value; }, enumerable: false, configurable: true }); - let _Buffer = Buffer; + const bufferGetter = deprecate( + () => cjsContext.Buffer, + 'global.Buffer is deprecated in ECMAScript Modules. ' + + 'Please use `import { Buffer } from \'buffer\'` instead.', 'DEP0YYY'); Object.defineProperty(global, 'Buffer', { get() { - return _Buffer; + if (experimentalModules) { + const sourceName = getStackSourceName(1); + if (sourceName && sourceName.startsWith('file://')) + return bufferGetter(); + } + return cjsContext.Buffer; }, set(value) { - _Buffer = value; + cjsContext.Buffer = value; }, enumerable: false, configurable: true diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index 639877434e2e98..6eea68dc9fe6a0 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -23,7 +23,7 @@ const { JSON, Object, Reflect } = primordials; -const { NativeModule } = require('internal/bootstrap/loaders'); +const { NativeModule, cjsContext } = require('internal/bootstrap/loaders'); const { pathToFileURL, fileURLToPath, URL } = require('internal/url'); const { deprecate } = require('internal/util'); const vm = require('vm'); @@ -703,37 +703,35 @@ function wrapSafe(filename, content) { return loader.import(specifier, normalizeReferrerURL(filename)); } : undefined, }); + } else { + const compiledWrapper = compileFunction( + content, + filename, + 0, + 0, + undefined, + false, + undefined, + [cjsContext], + [ + 'exports', + 'require', + 'module', + '__filename', + '__dirname', + ] + ); + if (experimentalModules) { + const { callbackMap } = internalBinding('module_wrap'); + callbackMap.set(compiledWrapper, { + importModuleDynamically: async (specifier) => { + const loader = await asyncESM.loaderPromise; + return loader.import(specifier, normalizeReferrerURL(filename)); + } + }); + } + return compiledWrapper; } - - const compiledWrapper = compileFunction( - content, - filename, - 0, - 0, - undefined, - false, - undefined, - [], - [ - 'exports', - 'require', - 'module', - '__filename', - '__dirname', - ] - ); - - if (experimentalModules) { - const { callbackMap } = internalBinding('module_wrap'); - callbackMap.set(compiledWrapper, { - importModuleDynamically: async (specifier) => { - const loader = await asyncESM.loaderPromise; - return loader.import(specifier, normalizeReferrerURL(filename)); - } - }); - } - - return compiledWrapper; } // Run the file contents in the correct scope or sandbox. Expose diff --git a/src/node_contextify.cc b/src/node_contextify.cc index 70c63a0d8a3ec1..e8240d9a6dc0f9 100644 --- a/src/node_contextify.cc +++ b/src/node_contextify.cc @@ -66,6 +66,8 @@ using v8::PropertyHandlerFlags; using v8::Script; using v8::ScriptCompiler; using v8::ScriptOrigin; +using v8::StackFrame; +using v8::StackTrace; using v8::String; using v8::Symbol; using v8::Uint32; @@ -221,6 +223,7 @@ void ContextifyContext::Init(Environment* env, Local target) { env->SetMethod(target, "makeContext", MakeContext); env->SetMethod(target, "isContext", IsContext); env->SetMethod(target, "compileFunction", CompileFunction); + env->SetMethod(target, "getStackSourceName", GetStackSourceName); } @@ -285,6 +288,30 @@ void ContextifyContext::IsContext(const FunctionCallbackInfo& args) { args.GetReturnValue().Set(result.FromJust()); } +void ContextifyContext::GetStackSourceName( + const FunctionCallbackInfo& args) { + Environment* env = Environment::GetCurrent(args); + + CHECK_EQ(args.Length(), 1); + CHECK(args[0]->IsNumber()); + + int32_t stack_depth = args[0].As()->Value(); + + Local frame; + do { + Local stack = + StackTrace::CurrentStackTrace(env->isolate(), + stack_depth + 1); + if (stack_depth >= stack->GetFrameCount()) { + return; + } + frame = stack->GetFrame(env->isolate(), stack_depth++); + } while (frame->IsEval()); + + Local script_name = frame->GetScriptName(); + + args.GetReturnValue().Set(script_name); +} void ContextifyContext::WeakCallback( const WeakCallbackInfo& data) { diff --git a/src/node_contextify.h b/src/node_contextify.h index d04bf9ea28efb2..f9bea1127816f8 100644 --- a/src/node_contextify.h +++ b/src/node_contextify.h @@ -57,6 +57,8 @@ class ContextifyContext { private: static void MakeContext(const v8::FunctionCallbackInfo& args); static void IsContext(const v8::FunctionCallbackInfo& args); + static void GetStackSourceName( + const v8::FunctionCallbackInfo& args); static void CompileFunction( const v8::FunctionCallbackInfo& args); static void WeakCallback( diff --git a/test/common/index.mjs b/test/common/index.mjs index 47587044020fcc..f747ee327913a5 100644 --- a/test/common/index.mjs +++ b/test/common/index.mjs @@ -1,12 +1,7 @@ // Flags: --experimental-modules /* eslint-disable node-core/require-common-first, node-core/required-modules */ -import { createRequireFromPath } from 'module'; -import { fileURLToPath as toPath } from 'url'; - -function createRequire(metaUrl) { - return createRequireFromPath(toPath(metaUrl)); -} +import { createRequire } from 'module'; const require = createRequire(import.meta.url); const common = require('./index.js'); diff --git a/test/message/async_error_sync_esm.out b/test/message/async_error_sync_esm.out index 544916e24866e2..f34628ef44e52a 100644 --- a/test/message/async_error_sync_esm.out +++ b/test/message/async_error_sync_esm.out @@ -5,4 +5,3 @@ Error: test at async three (*fixtures*async-error.js:20:3) at async four (*fixtures*async-error.js:24:3) at async main (*message*async_error_sync_esm.mjs:7:5) -(node:*) [DEP0130] DeprecationWarning: Module.createRequireFromPath() is deprecated. Use Module.createRequire() instead. diff --git a/test/parallel/test-global-buffer-dep.mjs b/test/parallel/test-global-buffer-dep.mjs new file mode 100644 index 00000000000000..5b55402987f3e0 --- /dev/null +++ b/test/parallel/test-global-buffer-dep.mjs @@ -0,0 +1,9 @@ +// Flags: --experimental-modules +import { expectWarning } from '../common/index.mjs'; + +global.Buffer; + +expectWarning('DeprecationWarning', + 'global.Buffer is deprecated in ECMAScript Modules. ' + + 'Please use `import { Buffer } from \'buffer\'` instead.', + 'DEP0YYY'); diff --git a/test/parallel/test-global-process-dep-eval.mjs b/test/parallel/test-global-process-dep-eval.mjs new file mode 100644 index 00000000000000..192df74dcab47c --- /dev/null +++ b/test/parallel/test-global-process-dep-eval.mjs @@ -0,0 +1,9 @@ +// Flags: --experimental-modules +import { expectWarning } from '../common/index.mjs'; + +(0, eval)('process'); + +expectWarning('DeprecationWarning', + 'global.process is deprecated in ECMAScript Modules. ' + + 'Please use `import process from \'process\'` instead.', + 'DEP0XXX'); diff --git a/test/parallel/test-global-process-dep-nested-eval.mjs b/test/parallel/test-global-process-dep-nested-eval.mjs new file mode 100644 index 00000000000000..60282fdbbee9ca --- /dev/null +++ b/test/parallel/test-global-process-dep-nested-eval.mjs @@ -0,0 +1,9 @@ +// Flags: --experimental-modules +import { expectWarning } from '../common/index.mjs'; + +new Function('return new Function("eval(process)")')()(); + +expectWarning('DeprecationWarning', + 'global.process is deprecated in ECMAScript Modules. ' + + 'Please use `import process from \'process\'` instead.', + 'DEP0XXX'); diff --git a/test/parallel/test-global-process-dep.mjs b/test/parallel/test-global-process-dep.mjs new file mode 100644 index 00000000000000..f521c7d18bb96f --- /dev/null +++ b/test/parallel/test-global-process-dep.mjs @@ -0,0 +1,9 @@ +// Flags: --experimental-modules +import { expectWarning } from '../common/index.mjs'; + +process; + +expectWarning('DeprecationWarning', + 'global.process is deprecated in ECMAScript Modules. ' + + 'Please use `import process from \'process\'` instead.', + 'DEP0XXX'); From 71d434f17585f3ebb08e6a7aa700eed8c11a62c6 Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Sun, 16 Jun 2019 23:32:11 +0200 Subject: [PATCH 2/2] esm: deprecate process, Buffer only when loading an ES module --- lib/internal/bootstrap/node.js | 5 --- lib/internal/bootstrap/pre_execution.js | 26 ++--------- .../modules/esm/deprecate_process_buffer.js | 45 +++++++++++++++++++ lib/internal/modules/esm/translators.js | 7 +++ node.gyp | 1 + 5 files changed, 57 insertions(+), 27 deletions(-) create mode 100644 lib/internal/modules/esm/deprecate_process_buffer.js diff --git a/lib/internal/bootstrap/node.js b/lib/internal/bootstrap/node.js index 3ee8bb48e8bb68..22dabf4d7ed459 100644 --- a/lib/internal/bootstrap/node.js +++ b/lib/internal/bootstrap/node.js @@ -305,7 +305,6 @@ function setupPrepareStackTrace() { } function setupProcessObject() { - const { cjsContext } = require('internal/bootstrap/loaders'); const EventEmitter = require('events'); const origProcProto = Object.getPrototypeOf(process); Object.setPrototypeOf(origProcProto, EventEmitter.prototype); @@ -323,7 +322,6 @@ function setupProcessObject() { writable: true, configurable: true }); - cjsContext.process = process; } function setupProcessStdio(getStdout, getStdin, getStderr) { @@ -392,7 +390,6 @@ function setupGlobalProxy() { function setupBuffer() { const { Buffer } = require('buffer'); - const { cjsContext } = require('internal/bootstrap/loaders'); const bufferBinding = internalBinding('buffer'); // Only after this point can C++ use Buffer::New() @@ -400,14 +397,12 @@ function setupBuffer() { delete bufferBinding.setBufferPrototype; delete bufferBinding.zeroFill; - // Make process globally available to users by putting it on the global proxy Object.defineProperty(global, 'Buffer', { value: Buffer, enumerable: false, writable: true, configurable: true }); - cjsContext.Buffer = Buffer; } function createGlobalConsole(consoleFromVM) { diff --git a/lib/internal/bootstrap/pre_execution.js b/lib/internal/bootstrap/pre_execution.js index 6fed21fdbad687..3667bf2af743af 100644 --- a/lib/internal/bootstrap/pre_execution.js +++ b/lib/internal/bootstrap/pre_execution.js @@ -3,6 +3,7 @@ const { Object, SafeWeakMap } = primordials; const { getOptionValue } = require('internal/options'); +const { Buffer } = require('buffer'); function prepareMainThreadExecution(expandArgv1 = false) { // Patch the process object with legacy properties and normalizations @@ -261,22 +262,12 @@ function initializeDeprecations() { 'Please use public APIs instead.', 'DEP0111'); } + // Create global.process and global.Buffer as getters to allow deprecation. const { cjsContext } = require('internal/bootstrap/loaders'); - const { getStackSourceName } = internalBinding('contextify'); - const experimentalModules = getOptionValue('--experimental-modules'); - - // Deprecate global.process and global.Buffer access in ES Modules. - const processGetter = deprecate( - () => cjsContext.process, - 'global.process is deprecated in ECMAScript Modules. ' + - 'Please use `import process from \'process\'` instead.', 'DEP0XXX'); + cjsContext.process = process; + cjsContext.Buffer = Buffer; Object.defineProperty(global, 'process', { get() { - if (experimentalModules) { - const sourceName = getStackSourceName(1); - if (sourceName && sourceName.startsWith('file://')) - return processGetter(); - } return cjsContext.process; }, set(value) { @@ -286,17 +277,8 @@ function initializeDeprecations() { configurable: true }); - const bufferGetter = deprecate( - () => cjsContext.Buffer, - 'global.Buffer is deprecated in ECMAScript Modules. ' + - 'Please use `import { Buffer } from \'buffer\'` instead.', 'DEP0YYY'); Object.defineProperty(global, 'Buffer', { get() { - if (experimentalModules) { - const sourceName = getStackSourceName(1); - if (sourceName && sourceName.startsWith('file://')) - return bufferGetter(); - } return cjsContext.Buffer; }, set(value) { diff --git a/lib/internal/modules/esm/deprecate_process_buffer.js b/lib/internal/modules/esm/deprecate_process_buffer.js new file mode 100644 index 00000000000000..56cbd1e1c1b1bd --- /dev/null +++ b/lib/internal/modules/esm/deprecate_process_buffer.js @@ -0,0 +1,45 @@ +'use strict'; + +const { cjsContext } = require('internal/bootstrap/loaders'); +const { getStackSourceName } = internalBinding('contextify'); +const { deprecate } = require('internal/util'); +const { Object } = primordials; + +module.exports = () => { + // Deprecate global.process and global.Buffer access in ES Modules. + const processGetter = deprecate( + () => cjsContext.process, + 'global.process is deprecated in ECMAScript Modules. ' + + 'Please use `import process from \'process\'` instead.', 'DEP0XXX'); + Object.defineProperty(global, 'process', { + get() { + const sourceName = getStackSourceName(1); + if (sourceName && sourceName.startsWith('file://')) + return processGetter(); + return cjsContext.process; + }, + set(value) { + cjsContext.process = value; + }, + enumerable: false, + configurable: true + }); + + const bufferGetter = deprecate( + () => cjsContext.Buffer, + 'global.Buffer is deprecated in ECMAScript Modules. ' + + 'Please use `import { Buffer } from \'buffer\'` instead.', 'DEP0YYY'); + Object.defineProperty(global, 'Buffer', { + get() { + const sourceName = getStackSourceName(1); + if (sourceName && sourceName.startsWith('file://')) + return bufferGetter(); + return cjsContext.Buffer; + }, + set(value) { + cjsContext.Buffer = value; + }, + enumerable: false, + configurable: true + }); +}; diff --git a/lib/internal/modules/esm/translators.js b/lib/internal/modules/esm/translators.js index b1f2c9f2595fc7..b04296e64c4d93 100644 --- a/lib/internal/modules/esm/translators.js +++ b/lib/internal/modules/esm/translators.js @@ -21,6 +21,8 @@ const fs = require('fs'); const { fileURLToPath, URL } = require('url'); const { debuglog } = require('internal/util/debuglog'); const { promisify } = require('internal/util'); +const deprecateProcessBufferAccess = + require('internal/modules/esm/deprecate_process_buffer'); const esmLoader = require('internal/process/esm_loader'); const { ERR_UNKNOWN_BUILTIN_MODULE @@ -43,7 +45,12 @@ async function importModuleDynamically(specifier, { url }) { } // Strategy for loading a standard JavaScript module +let firstModule = true; translators.set('module', async function moduleStrategy(url) { + if (firstModule) { + deprecateProcessBufferAccess(); + firstModule = false; + } const source = `${await readFileAsync(new URL(url))}`; debug(`Translating StandardModule ${url}`); const { ModuleWrap, callbackMap } = internalBinding('module_wrap'); diff --git a/node.gyp b/node.gyp index 9a5556c2eff40d..f01be89fd4a1f7 100644 --- a/node.gyp +++ b/node.gyp @@ -150,6 +150,7 @@ 'lib/internal/modules/esm/loader.js', 'lib/internal/modules/esm/create_dynamic_module.js', 'lib/internal/modules/esm/default_resolve.js', + 'lib/internal/modules/esm/deprecate_process_buffer.js', 'lib/internal/modules/esm/module_job.js', 'lib/internal/modules/esm/module_map.js', 'lib/internal/modules/esm/translators.js',