From 5d33dfd7876d0f2b8e43af9424d4b12d0dc65b27 Mon Sep 17 00:00:00 2001
From: Antoine du Hamel <duhamelantoine1995@gmail.com>
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 <webadmin@geoffreybooth.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
---
 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.