-
Notifications
You must be signed in to change notification settings - Fork 30k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
module: remove dynamicInstantiate loader hook
The dynamicInstantiate loader hook requires that the hooks run in the same global scope as the code being loaded. We don't want to commit to this being true in the future. It stops us from sharing hooks between multiple worker threads or isolating loader hook from the application code. Using `getSource` and `getGlobalPreloadCode` the same use cases should be covered. PR-URL: #33501 Reviewed-By: Geoffrey Booth <[email protected]> Reviewed-By: Guy Bedford <[email protected]> Reviewed-By: Bradley Farias <[email protected]>
- Loading branch information
1 parent
da5e970
commit 8c9e3a9
Showing
7 changed files
with
60 additions
and
111 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
8 changes: 0 additions & 8 deletions
8
test/es-module/test-esm-loader-missing-dynamic-instantiate-hook.mjs
This file was deleted.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,9 @@ | ||
| // Flags: --experimental-loader ./test/fixtures/es-module-loaders/builtin-named-exports-loader.mjs | ||
| import '../common/index.mjs'; | ||
| import { readFile } from 'fs'; | ||
| import { readFile, __fromLoader } from 'fs'; | ||
| import assert from 'assert'; | ||
| import ok from '../fixtures/es-modules/test-esm-ok.mjs'; | ||
|
|
||
| assert(ok); | ||
| assert(readFile); | ||
| assert(__fromLoader); |
67 changes: 53 additions & 14 deletions
67
test/fixtures/es-module-loaders/builtin-named-exports-loader.mjs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,23 +1,62 @@ | ||
| import module from 'module'; | ||
|
|
||
| export function getFormat(url, context, defaultGetFormat) { | ||
| if (module.builtinModules.includes(url)) { | ||
| const GET_BUILTIN = `$__get_builtin_hole_${Date.now()}`; | ||
|
|
||
| export function getGlobalPreloadCode() { | ||
| return `Object.defineProperty(globalThis, ${JSON.stringify(GET_BUILTIN)}, { | ||
| value: (builtinName) => { | ||
| return getBuiltin(builtinName); | ||
| }, | ||
| enumerable: false, | ||
| configurable: false, | ||
| }); | ||
| `; | ||
| } | ||
|
|
||
| export function resolve(specifier, context, defaultResolve) { | ||
| const def = defaultResolve(specifier, context); | ||
| if (def.url.startsWith('nodejs:')) { | ||
| return { | ||
| url: `custom-${def.url}`, | ||
| }; | ||
| } | ||
| return def; | ||
| } | ||
|
|
||
| export function getSource(url, context, defaultGetSource) { | ||
| if (url.startsWith('custom-nodejs:')) { | ||
| const urlObj = new URL(url); | ||
| return { | ||
| format: 'dynamic' | ||
| source: generateBuiltinModule(urlObj.pathname), | ||
| format: 'module', | ||
| }; | ||
| } | ||
| return defaultGetSource(url, context); | ||
| } | ||
|
|
||
| export function getFormat(url, context, defaultGetFormat) { | ||
| if (url.startsWith('custom-nodejs:')) { | ||
| return { format: 'module' }; | ||
| } | ||
| return defaultGetFormat(url, context, defaultGetFormat); | ||
| } | ||
|
|
||
| export function dynamicInstantiate(url) { | ||
| const builtinInstance = module._load(url); | ||
| const builtinExports = ['default', ...Object.keys(builtinInstance)]; | ||
| return { | ||
| exports: builtinExports, | ||
| execute: exports => { | ||
| for (let name of builtinExports) | ||
| exports[name].set(builtinInstance[name]); | ||
| exports.default.set(builtinInstance); | ||
| } | ||
| }; | ||
| function generateBuiltinModule(builtinName) { | ||
| const builtinInstance = module._load(builtinName); | ||
| const builtinExports = [ | ||
| ...Object.keys(builtinInstance), | ||
| ]; | ||
| return `\ | ||
| const $builtinInstance = ${GET_BUILTIN}(${JSON.stringify(builtinName)}); | ||
| export const __fromLoader = true; | ||
| export default $builtinInstance; | ||
| ${ | ||
| builtinExports | ||
| .map(name => `export const ${name} = $builtinInstance.${name};`) | ||
| .join('\n') | ||
| } | ||
| `; | ||
| } |