esm: do not bind loader hook functions

PR-URL: #44122 Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
nodejs · Oct 10, 2022 · bcec653 · bcec653
1 parent 21452fb
commit bcec653
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 11 deletions.
diff --git 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
@@ -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
@@ -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.