From 46c8f5dd8997d6906bd7b7457c8febdf0a376a52 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Mon, 8 Aug 2022 10:51:35 +0200 Subject: [PATCH] esm: do not bind loader hook functions PR-URL: https://github.com/nodejs/node/pull/44122 Reviewed-By: Geoffrey Booth Reviewed-By: Benjamin Gruenbaum --- lib/internal/modules/esm/loader.js | 17 ++++++----------- .../loader-this-value-inside-hook-functions.mjs | 14 ++++++++++++++ ...loaders-this-value-inside-hook-functions.mjs | 4 ++++ 3 files changed, 24 insertions(+), 11 deletions(-) create mode 100644 test/fixtures/es-module-loaders/loader-this-value-inside-hook-functions.mjs create mode 100644 test/parallel/test-loaders-this-value-inside-hook-functions.mjs diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js index f2d5600fba657c..71c9100a7e7ed1 100644 --- a/lib/internal/modules/esm/loader.js +++ b/lib/internal/modules/esm/loader.js @@ -8,7 +8,6 @@ const { ArrayIsArray, ArrayPrototypeJoin, ArrayPrototypePush, - FunctionPrototypeBind, FunctionPrototypeCall, ObjectAssign, ObjectCreate, @@ -306,16 +305,14 @@ class ESMLoader { 'DeprecationWarning', ); - // Use .bind() to avoid giving access to the Loader instance when called. if (globalPreload) { - acceptedHooks.globalPreloader = - FunctionPrototypeBind(globalPreload, null); + acceptedHooks.globalPreloader = globalPreload; } if (resolve) { - acceptedHooks.resolver = FunctionPrototypeBind(resolve, null); + acceptedHooks.resolver = resolve; } if (load) { - acceptedHooks.loader = FunctionPrototypeBind(load, null); + acceptedHooks.loader = load; } return acceptedHooks; @@ -346,7 +343,7 @@ class ESMLoader { ArrayPrototypePush( this.#globalPreloaders, { - fn: FunctionPrototypeBind(globalPreloader), // [1] + fn: globalPreloader, url, }, ); @@ -355,7 +352,7 @@ class ESMLoader { ArrayPrototypePush( this.#resolvers, { - fn: FunctionPrototypeBind(resolver), // [1] + fn: resolver, url, }, ); @@ -364,15 +361,13 @@ class ESMLoader { ArrayPrototypePush( this.#loaders, { - fn: FunctionPrototypeBind(loader), // [1] + fn: loader, url, }, ); } } - // [1] ensure hook function is not bound to ESMLoader instance - this.preload(); } diff --git a/test/fixtures/es-module-loaders/loader-this-value-inside-hook-functions.mjs b/test/fixtures/es-module-loaders/loader-this-value-inside-hook-functions.mjs new file mode 100644 index 00000000000000..c1c80622feea66 --- /dev/null +++ b/test/fixtures/es-module-loaders/loader-this-value-inside-hook-functions.mjs @@ -0,0 +1,14 @@ +export function resolve(url, _, next) { + if (this != null) throw new Error('hook function must not be bound to ESMLoader instance'); + return next(url); +} + +export function load(url, _, next) { + if (this != null) throw new Error('hook function must not be bound to ESMLoader instance'); + return next(url); +} + +export function globalPreload() { + if (this != null) throw new Error('hook function must not be bound to ESMLoader instance'); + return ""; +} diff --git a/test/parallel/test-loaders-this-value-inside-hook-functions.mjs b/test/parallel/test-loaders-this-value-inside-hook-functions.mjs new file mode 100644 index 00000000000000..b74b7d9e1e4c30 --- /dev/null +++ b/test/parallel/test-loaders-this-value-inside-hook-functions.mjs @@ -0,0 +1,4 @@ +// Flags: --experimental-loader ./test/fixtures/es-module-loaders/loader-this-value-inside-hook-functions.mjs +import '../common/index.mjs'; + +// Actual test is inside the loader module.