GeezerVison is showing github.com — Revert "vm: add importModuleDynamically option to compileFunction" · nodejs/node@c2cf978
File treeExpand file treeCollapse file tree Open diff view settings Expand file treeCollapse file tree Open diff view settings Collapse file Copy file name to clipboardExpand all lines: doc/api/vm.md +4-11Lines changed: 4 additions & 11 deletions - Display the source diff
- Display the rich diff
| Original file line number | Diff line number | Diff line change |
|---|
@@ -88,7 +88,7 @@ changes: | 88 | 88 | This option is part of the experimental modules API, and should not be | 89 | 89 | considered stable. | 90 | 90 | * `specifier` {string} specifier passed to `import()` | 91 | | - * `script` {vm.Script} | | 91 | + * `module` {vm.Module} | 92 | 92 | * Returns: {Module Namespace Object|vm.Module} Returning a `vm.Module` is | 93 | 93 | recommended in order to take advantage of error tracking, and to avoid | 94 | 94 | issues with namespaces that contain `then` function exports. | @@ -805,6 +805,9 @@ changes: | 805 | 805 | - version: v14.1.0 | 806 | 806 | pr-url: https://github.com/nodejs/node/pull/32985 | 807 | 807 | description: The `importModuleDynamically` option is now supported. | | 808 | + - version: REPLACEME | | 809 | + pr-url: https://github.com/nodejs/node/pull/33364 | | 810 | + description: Removal of `importModuleDynamically` due to compatibility issues | 808 | 811 | --> | 809 | 812 |
| 810 | 813 | * `code` {string} The body of the function to compile. | @@ -827,16 +830,6 @@ changes: | 827 | 830 | * `contextExtensions` {Object[]} An array containing a collection of context | 828 | 831 | extensions (objects wrapping the current scope) to be applied while | 829 | 832 | compiling. **Default:** `[]`. | 830 | | - * `importModuleDynamically` {Function} Called during evaluation of this module | 831 | | - when `import()` is called. If this option is not specified, calls to | 832 | | - `import()` will reject with [`ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING`][]. | 833 | | - This option is part of the experimental modules API, and should not be | 834 | | - considered stable. | 835 | | - * `specifier` {string} specifier passed to `import()` | 836 | | - * `function` {Function} | 837 | | - * Returns: {Module Namespace Object|vm.Module} Returning a `vm.Module` is | 838 | | - recommended in order to take advantage of error tracking, and to avoid | 839 | | - issues with namespaces that contain `then` function exports. | 840 | 833 | * Returns: {Function} | 841 | 834 |
| 842 | 835 | Compiles the given code into the provided context (if no context is | |
Collapse file Copy file name to clipboardExpand all lines: lib/internal/modules/cjs/loader.js +28-12Lines changed: 28 additions & 12 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -77,6 +77,7 @@ const preserveSymlinksMain = getOptionValue('--preserve-symlinks-main'); | 77 | 77 | const manifest = getOptionValue('--experimental-policy') ? | 78 | 78 | require('internal/process/policy').manifest : | 79 | 79 | null; | | 80 | +const { compileFunction } = internalBinding('contextify'); | 80 | 81 |
| 81 | 82 | // Whether any user-provided CJS modules had been loaded (executed). | 82 | 83 | // Used for internal assertions. | @@ -1110,25 +1111,40 @@ function wrapSafe(filename, content, cjsModuleInstance) { | 1110 | 1111 | }, | 1111 | 1112 | }); | 1112 | 1113 | } | | 1114 | + let compiled; | 1113 | 1115 | try { | 1114 | | - return vm.compileFunction(content, [ | 1115 | | - 'exports', | 1116 | | - 'require', | 1117 | | - 'module', | 1118 | | - '__filename', | 1119 | | - '__dirname', | 1120 | | - ], { | | 1116 | + compiled = compileFunction( | | 1117 | + content, | 1121 | 1118 | filename, | 1122 | | - importModuleDynamically(specifier) { | 1123 | | - const loader = asyncESM.ESMLoader; | 1124 | | - return loader.import(specifier, normalizeReferrerURL(filename)); | 1125 | | - }, | 1126 | | - }); | | 1119 | + 0, | | 1120 | + 0, | | 1121 | + undefined, | | 1122 | + false, | | 1123 | + undefined, | | 1124 | + [], | | 1125 | + [ | | 1126 | + 'exports', | | 1127 | + 'require', | | 1128 | + 'module', | | 1129 | + '__filename', | | 1130 | + '__dirname', | | 1131 | + ] | | 1132 | + ); | 1127 | 1133 | } catch (err) { | 1128 | 1134 | if (process.mainModule === cjsModuleInstance) | 1129 | 1135 | enrichCJSError(err); | 1130 | 1136 | throw err; | 1131 | 1137 | } | | 1138 | + | | 1139 | + const { callbackMap } = internalBinding('module_wrap'); | | 1140 | + callbackMap.set(compiled.cacheKey, { | | 1141 | + importModuleDynamically: async (specifier) => { | | 1142 | + const loader = asyncESM.ESMLoader; | | 1143 | + return loader.import(specifier, normalizeReferrerURL(filename)); | | 1144 | + } | | 1145 | + }); | | 1146 | + | | 1147 | + return compiled.function; | 1132 | 1148 | } | 1133 | 1149 |
| 1134 | 1150 | // Run the file contents in the correct scope or sandbox. Expose | |
Collapse file Copy file name to clipboardExpand all lines: lib/vm.js -17Lines changed: 0 additions & 17 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -313,7 +313,6 @@ function compileFunction(code, params, options = {}) { | 313 | 313 | produceCachedData = false, | 314 | 314 | parsingContext = undefined, | 315 | 315 | contextExtensions = [], | 316 | | - importModuleDynamically, | 317 | 316 | } = options; | 318 | 317 |
| 319 | 318 | validateString(filename, 'options.filename'); | @@ -361,22 +360,6 @@ function compileFunction(code, params, options = {}) { | 361 | 360 | result.function.cachedData = result.cachedData; | 362 | 361 | } | 363 | 362 |
| 364 | | - if (importModuleDynamically !== undefined) { | 365 | | - if (typeof importModuleDynamically !== 'function') { | 366 | | - throw new ERR_INVALID_ARG_TYPE('options.importModuleDynamically', | 367 | | - 'function', | 368 | | - importModuleDynamically); | 369 | | - } | 370 | | - const { importModuleDynamicallyWrap } = | 371 | | - require('internal/vm/module'); | 372 | | - const { callbackMap } = internalBinding('module_wrap'); | 373 | | - const wrapped = importModuleDynamicallyWrap(importModuleDynamically); | 374 | | - const func = result.function; | 375 | | - callbackMap.set(result.cacheKey, { | 376 | | - importModuleDynamically: (s, _k) => wrapped(s, func), | 377 | | - }); | 378 | | - } | 379 | | - | 380 | 363 | return result.function; | 381 | 364 | } | 382 | 365 |
| |
Collapse file Copy file name to clipboardExpand all lines: test/parallel/test-vm-module-basic.js +1-18Lines changed: 1 addition & 18 deletions | Original file line number | Diff line number | Diff line change |
|---|
| 8 | 8 | Module, | 9 | 9 | SourceTextModule, | 10 | 10 | SyntheticModule, | 11 | | - createContext, | 12 | | - compileFunction, | | 11 | + createContext | 13 | 12 | } = require('vm'); | 14 | 13 | const util = require('util'); | 15 | 14 |
| @@ -158,19 +157,3 @@ const util = require('util'); | 158 | 157 | name: 'TypeError' | 159 | 158 | }); | 160 | 159 | } | 161 | | - | 162 | | -// Test compileFunction importModuleDynamically | 163 | | -{ | 164 | | - const module = new SyntheticModule([], () => {}); | 165 | | - module.link(() => {}); | 166 | | - const f = compileFunction('return import("x")', [], { | 167 | | - importModuleDynamically(specifier, referrer) { | 168 | | - assert.strictEqual(specifier, 'x'); | 169 | | - assert.strictEqual(referrer, f); | 170 | | - return module; | 171 | | - }, | 172 | | - }); | 173 | | - f().then((ns) => { | 174 | | - assert.strictEqual(ns, module.namespace); | 175 | | - }); | 176 | | -} |
Collapse file Copy file name to clipboardExpand all lines: tools/doc/type-parser.js -1Lines changed: 0 additions & 1 deletion | Original file line number | Diff line number | Diff line change |
|---|
@@ -154,7 +154,6 @@ const customTypesMap = { | 154 | 154 | 'URLSearchParams': 'url.html#url_class_urlsearchparams', | 155 | 155 |
| 156 | 156 | 'vm.Module': 'vm.html#vm_class_vm_module', | 157 | | - 'vm.Script': 'vm.html#vm_class_vm_script', | 158 | 157 | 'vm.SourceTextModule': 'vm.html#vm_class_vm_sourcetextmodule', | 159 | 158 |
| 160 | 159 | 'MessagePort': 'worker_threads.html#worker_threads_class_messageport', | |
|