esm: remove globalPreload hook (superseded by initialize)

nodejs · Aug 13, 2023 · abdde28 · abdde28
1 parent b5da2f4
commit abdde28
Show file tree

Hide file tree

Showing 8 changed files with 10 additions and 405 deletions.
diff --git a/lib/internal/modules/esm/hooks.js b/lib/internal/modules/esm/hooks.js
@@ -73,7 +73,7 @@ let importMetaInitializer;
 
 /**
  * @typedef {object} ExportedHooks
- * @property {Function} globalPreload Global preload hook.
+ * @property {Function} initialize Customizations setup hook.
  * @property {Function} resolve Resolve hook.
  * @property {Function} load Load hook.
  */
@@ -88,13 +88,6 @@ let importMetaInitializer;
 
 class Hooks {
   #chains = {
-    /**
-     * Prior to ESM loading. These are called once before any modules are started.
-     * @private
-     * @property {KeyedHook[]} globalPreload Last-in-first-out list of preload hooks.
-     */
-    globalPreload: [],
-
     /**
      * Phase 1 of 2 in ESM loading.
      * The output of the `resolve` chain of hooks is passed into the `load` chain of hooks.
@@ -154,18 +147,11 @@ class Hooks {
    */
   addCustomLoader(url, exports, data) {
     const {
-      globalPreload,
       initialize,
       resolve,
       load,
     } = pluckHooks(exports);
 
-    if (globalPreload && !initialize) {
-      emitExperimentalWarning(
-        '`globalPreload` is planned for removal in favor of `initialize`. `globalPreload`',
-      );
-      ArrayPrototypePush(this.#chains.globalPreload, { __proto__: null, fn: globalPreload, url });
-    }
     if (resolve) {
       const next = this.#chains.resolve[this.#chains.resolve.length - 1];
       ArrayPrototypePush(this.#chains.resolve, { __proto__: null, fn: resolve, url, next });
@@ -177,49 +163,6 @@ class Hooks {
     return initialize?.(data);
   }
 
-  /**
-   * Initialize `globalPreload` hooks.
-   */
-  initializeGlobalPreload() {
-    const preloadScripts = [];
-    for (let i = this.#chains.globalPreload.length - 1; i >= 0; i--) {
-      const { MessageChannel } = require('internal/worker/io');
-      const channel = new MessageChannel();
-      const {
-        port1: insidePreload,
-        port2: insideLoader,
-      } = channel;
-
-      insidePreload.unref();
-      insideLoader.unref();
-
-      const {
-        fn: preload,
-        url: specifier,
-      } = this.#chains.globalPreload[i];
-
-      const preloaded = preload({
-        port: insideLoader,
-      });
-
-      if (preloaded == null) { continue; }
-
-      if (typeof preloaded !== 'string') { // [2]
-        throw new ERR_INVALID_RETURN_VALUE(
-          'a string',
-          `${specifier} globalPreload`,
-          preload,
-        );
-      }
-
-      ArrayPrototypePush(preloadScripts, {
-        code: preloaded,
-        port: insidePreload,
-      });
-    }
-    return preloadScripts;
-  }
-
   /**
    * Resolve the location of the module.
    *
@@ -749,9 +692,6 @@ function pluckHooks({
 }) {
   const acceptedHooks = { __proto__: null };
 
-  if (globalPreload) {
-    acceptedHooks.globalPreload = globalPreload;
-  }
   if (resolve) {
     acceptedHooks.resolve = resolve;
   }
@@ -761,6 +701,11 @@ function pluckHooks({
 
   if (initialize) {
     acceptedHooks.initialize = initialize;
+  } else if (globalPreload) { // TODO(JakobJingleheimer): Remove this when loaders go "stable".
+    process.emitWarning(
+      '`globalPreload` has been removed; use `initialize` instead.',
+      'DeprecationWarning',
+    );
   }
 
   return acceptedHooks;

diff --git a/lib/internal/modules/esm/utils.js b/lib/internal/modules/esm/utils.js
@@ -145,9 +145,7 @@ async function initializeHooks() {
     );
   }
 
-  const preloadScripts = hooks.initializeGlobalPreload();
-
-  return { __proto__: null, hooks, preloadScripts };
+  return { __proto__: null, hooks };
 }
 
 module.exports = {

diff --git a/test/es-module/test-esm-loader-hooks.mjs b/test/es-module/test-esm-loader-hooks.mjs
@@ -419,143 +419,6 @@ describe('Loader hooks', { concurrency: true }, () => {
     });
   });
 
-  describe('globalPreload', () => {
-    it('should emit deprecation warning', async () => {
-      const { stderr } = await spawnPromisified(execPath, [
-        '--experimental-loader',
-        'data:text/javascript,export function globalPreload(){}',
-        '--experimental-loader',
-        'data:text/javascript,export function globalPreload(){return""}',
-        fixtures.path('empty.js'),
-      ]);
-
-      assert.strictEqual(stderr.match(/`globalPreload` is an experimental feature/g).length, 1);
-    });
-
-    it('should not emit deprecation warning when initialize is supplied', async () => {
-      const { stderr } = await spawnPromisified(execPath, [
-        '--experimental-loader',
-        'data:text/javascript,export function globalPreload(){}export function initialize(){}',
-        fixtures.path('empty.js'),
-      ]);
-
-      assert.doesNotMatch(stderr, /`globalPreload` is an experimental feature/);
-    });
-
-    it('should handle globalPreload returning undefined', async () => {
-      const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [
-        '--no-warnings',
-        '--experimental-loader',
-        'data:text/javascript,export function globalPreload(){}',
-        fixtures.path('empty.js'),
-      ]);
-
-      assert.strictEqual(stderr, '');
-      assert.strictEqual(stdout, '');
-      assert.strictEqual(code, 0);
-      assert.strictEqual(signal, null);
-    });
-
-    it('should handle loading node:test', async () => {
-      const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [
-        '--no-warnings',
-        '--experimental-loader',
-        'data:text/javascript,export function globalPreload(){return `getBuiltin("node:test")()`}',
-        fixtures.path('empty.js'),
-      ]);
-
-      assert.strictEqual(stderr, '');
-      assert.match(stdout, /\n# pass 1\r?\n/);
-      assert.strictEqual(code, 0);
-      assert.strictEqual(signal, null);
-    });
-
-    it('should handle loading node:os with node: prefix', async () => {
-      const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [
-        '--no-warnings',
-        '--experimental-loader',
-        'data:text/javascript,export function globalPreload(){return `console.log(getBuiltin("node:os").arch())`}',
-        fixtures.path('empty.js'),
-      ]);
-
-      assert.strictEqual(stderr, '');
-      assert.strictEqual(stdout.trim(), os.arch());
-      assert.strictEqual(code, 0);
-      assert.strictEqual(signal, null);
-    });
-
-    // `os` is used here because it's simple and not mocked (the builtin module otherwise doesn't matter).
-    it('should handle loading builtin module without node: prefix', async () => {
-      const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [
-        '--no-warnings',
-        '--experimental-loader',
-        'data:text/javascript,export function globalPreload(){return `console.log(getBuiltin("os").arch())`}',
-        fixtures.path('empty.js'),
-      ]);
-
-      assert.strictEqual(stderr, '');
-      assert.strictEqual(stdout.trim(), os.arch());
-      assert.strictEqual(code, 0);
-      assert.strictEqual(signal, null);
-    });
-
-    it('should throw when loading node:test without node: prefix', async () => {
-      const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [
-        '--no-warnings',
-        '--experimental-loader',
-        'data:text/javascript,export function globalPreload(){return `getBuiltin("test")()`}',
-        fixtures.path('empty.js'),
-      ]);
-
-      assert.match(stderr, /ERR_UNKNOWN_BUILTIN_MODULE/);
-      assert.strictEqual(stdout, '');
-      assert.strictEqual(code, 1);
-      assert.strictEqual(signal, null);
-    });
-
-    it('should register globals set from globalPreload', async () => {
-      const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [
-        '--no-warnings',
-        '--experimental-loader',
-        'data:text/javascript,export function globalPreload(){return "this.myGlobal=4"}',
-        '--print', 'myGlobal',
-      ]);
-
-      assert.strictEqual(stderr, '');
-      assert.strictEqual(stdout.trim(), '4');
-      assert.strictEqual(code, 0);
-      assert.strictEqual(signal, null);
-    });
-
-    it('should log console.log calls returned from globalPreload', async () => {
-      const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [
-        '--no-warnings',
-        '--experimental-loader',
-        'data:text/javascript,export function globalPreload(){return `console.log("Hello from globalPreload")`}',
-        fixtures.path('empty.js'),
-      ]);
-
-      assert.strictEqual(stderr, '');
-      assert.strictEqual(stdout.trim(), 'Hello from globalPreload');
-      assert.strictEqual(code, 0);
-      assert.strictEqual(signal, null);
-    });
-
-    it('should crash if globalPreload returns code that throws', async () => {
-      const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [
-        '--no-warnings',
-        '--experimental-loader',
-        'data:text/javascript,export function globalPreload(){return `throw new Error("error from globalPreload")`}',
-        fixtures.path('empty.js'),
-      ]);
-
-      assert.match(stderr, /error from globalPreload/);
-      assert.strictEqual(stdout, '');
-      assert.strictEqual(code, 1);
-      assert.strictEqual(signal, null);
-    });
-  });
-
   it('should be fine to call `process.removeAllListeners("beforeExit")` from the main thread', async () => {
     const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [
       '--no-warnings',
@@ -625,7 +488,7 @@ describe('Loader hooks', { concurrency: true }, () => {
     assert.strictEqual(signal, null);
   });
 
-  it('should have `register` work with cjs', async () => {
+  it('should `register` from cjs', async () => {
     const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [
       '--no-warnings',
       '--input-type=commonjs',

diff --git a/test/fixtures/es-module-loaders/builtin-named-exports-loader.mjs b/test/fixtures/es-module-loaders/builtin-named-exports-loader.mjs
@@ -3,17 +3,6 @@ import { readFileSync } from 'node:fs';
 
 const GET_BUILTIN = `$__get_builtin_hole_${Date.now()}`;
 
-export function globalPreload() {
-  return `Object.defineProperty(globalThis, ${JSON.stringify(GET_BUILTIN)}, {
-  value: (builtinName) => {
-    return getBuiltin(builtinName);
-  },
-  enumerable: false,
-  configurable: false,
-});
-`;
-}
-
 export async function resolve(specifier, context, next) {
   const def = await next(specifier, context);
 
@@ -56,7 +45,7 @@ const $builtinInstance = ${GET_BUILTIN}(${JSON.stringify(builtinName)});
 module.exports = $builtinInstance;
 module.exports.__fromLoader = true;
 
-// We need this for CJS-module-lexer can parse the exported names. 
+// We need this for CJS-module-lexer can parse the exported names.
 ${
   builtinExports
     .map(name => `exports.${name} = $builtinInstance.${name};`)

diff --git a/test/fixtures/es-module-loaders/hook-resolve-type.mjs b/test/fixtures/es-module-loaders/hook-resolve-type.mjs
@@ -1,30 +1,6 @@
 let importedESM = 0;
 let importedCJS = 0;
 
-export function globalPreload({ port }) {
-  port.on('message', (int32) => {
-    port.postMessage({ importedESM, importedCJS });
-    Atomics.store(int32, 0, 1);
-    Atomics.notify(int32, 0);
-  });
-  port.unref();
-  return `
-    const { receiveMessageOnPort } = getBuiltin('worker_threads');
-    global.getModuleTypeStats = async function getModuleTypeStats() {
-      const sab = new SharedArrayBuffer(4);
-      const int32 = new Int32Array(sab);
-      port.postMessage(int32);
-      // Artificial timeout to keep the event loop alive.
-      // https://bugs.chromium.org/p/v8/issues/detail?id=13238
-      // TODO(targos) Remove when V8 issue is resolved.
-      const timeout = setTimeout(() => { throw new Error('timeout'); }, 1_000);
-      await Atomics.waitAsync(int32, 0, 0).value;
-      clearTimeout(timeout);
-      return receiveMessageOnPort(port).message;
-    };
-  `;
-}
-
 export async function load(url, context, next) {
   return next(url);
 }

diff --git a/test/fixtures/es-module-loaders/loader-side-effect.mjs b/test/fixtures/es-module-loaders/loader-side-effect.mjs
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
@@ -8,7 +8,7 @@ export function load(url, _, next) {
   return next(url);
 }
 
-export function globalPreload() {
+export function initialize() {
   if (this != null) throw new Error('hook function must not be bound to ESMLoader instance');
   return "";
 }