From 7375fa029548c94eb341c73996a1cbffc06e8713 Mon Sep 17 00:00:00 2001 From: Chengzhong Wu Date: Fri, 6 Dec 2024 13:55:59 +0000 Subject: [PATCH] fixup! esm: add experimental support for addon modules --- doc/api/module.md | 1 + lib/internal/modules/esm/translators.js | 23 ++++++++++--------- .../node_modules/esm-package | 1 + .../esm-package-dependent/test-import.js | 15 ++++++++++++ .../esm-package-dependent/test-require.js | 15 ++++++++++++ .../binding-export-default.cc | 0 .../binding-export-primitive.cc | 0 test/addons/{esm => esm-package}/binding.cc | 0 test/addons/{esm => esm-package}/binding.gyp | 0 test/addons/esm-package/package.json | 11 +++++++++ test/addons/{esm => esm-package}/test-esm.mjs | 13 ++++++++--- .../{esm => esm-package}/test-import.js | 1 - .../{esm => esm-package}/test-require.js | 1 - 13 files changed, 65 insertions(+), 16 deletions(-) create mode 120000 test/addons/esm-package-dependent/node_modules/esm-package create mode 100644 test/addons/esm-package-dependent/test-import.js create mode 100644 test/addons/esm-package-dependent/test-require.js rename test/addons/{esm => esm-package}/binding-export-default.cc (100%) rename test/addons/{esm => esm-package}/binding-export-primitive.cc (100%) rename test/addons/{esm => esm-package}/binding.cc (100%) rename test/addons/{esm => esm-package}/binding.gyp (100%) create mode 100644 test/addons/esm-package/package.json rename test/addons/{esm => esm-package}/test-esm.mjs (86%) rename test/addons/{esm => esm-package}/test-import.js (99%) rename test/addons/{esm => esm-package}/test-require.js (99%) diff --git a/doc/api/module.md b/doc/api/module.md index 3d20d83c73a9b3..0a60a18e8e8af8 100644 --- a/doc/api/module.md +++ b/doc/api/module.md @@ -957,6 +957,7 @@ The final value of `format` must be one of the following: | `format` | Description | Acceptable types for `source` returned by `load` | | ------------ | ------------------------------ | -------------------------------------------------------------------------- | +| `'addon'` | Load a Node.js addon | { `null` } | | `'builtin'` | Load a Node.js builtin module | Not applicable | | `'commonjs'` | Load a Node.js CommonJS module | { [`string`][], [`ArrayBuffer`][], [`TypedArray`][], `null`, `undefined` } | | `'json'` | Load a JSON file | { [`string`][], [`ArrayBuffer`][], [`TypedArray`][] } | diff --git a/lib/internal/modules/esm/translators.js b/lib/internal/modules/esm/translators.js index bedd7135ab9b11..0ce1927cc41826 100644 --- a/lib/internal/modules/esm/translators.js +++ b/lib/internal/modules/esm/translators.js @@ -245,13 +245,14 @@ function createCJSNoSourceModuleWrap(url, isMain) { // Addon export names are not known until the addon is loaded. const exportNames = ['default', 'module.exports']; - return new ModuleWrap(url, undefined, exportNames, function() { + return new ModuleWrap(url, undefined, exportNames, function evaluationCallback() { debug(`Loading CJSModule ${url}`); if (!module.loaded) { wrapModuleLoad(filename, null, isMain); } + /** @type {import('./loader').ModuleExports} */ let exports; if (module[kModuleExport] !== undefined) { exports = module[kModuleExport]; @@ -323,18 +324,18 @@ translators.set('commonjs', function commonjsStrategy(url, source, isMain) { */ function cjsEmplaceModuleCacheEntry(filename, exportNames) { // TODO: Do we want to keep hitting the user mutable CJS loader here? - let module = CJSModule._cache[filename]; - if (module) { - return module; + let cjsMod = CJSModule._cache[filename]; + if (cjsMod) { + return cjsMod; } - module = new CJSModule(filename); - module.filename = filename; - module.paths = CJSModule._nodeModulePaths(module.path); - module[kIsCachedByESMLoader] = true; - CJSModule._cache[filename] = module; + cjsMod = new CJSModule(filename); + cjsMod.filename = filename; + cjsMod.paths = CJSModule._nodeModulePaths(cjsMod.path); + cjsMod[kIsCachedByESMLoader] = true; + CJSModule._cache[filename] = cjsMod; - return module; + return cjsMod; } /** @@ -510,7 +511,7 @@ translators.set('wasm', async function(url, source) { }); // Strategy for loading a addon -translators.set('addon', function(url, source, isMain) { +translators.set('addon', function translateAddon(url, source, isMain) { emitExperimentalWarning('Importing addons'); // The addon must be loaded from file system with dlopen. Assert diff --git a/test/addons/esm-package-dependent/node_modules/esm-package b/test/addons/esm-package-dependent/node_modules/esm-package new file mode 120000 index 00000000000000..c7c1eb7f6858ba --- /dev/null +++ b/test/addons/esm-package-dependent/node_modules/esm-package @@ -0,0 +1 @@ +../../esm-package/ \ No newline at end of file diff --git a/test/addons/esm-package-dependent/test-import.js b/test/addons/esm-package-dependent/test-import.js new file mode 100644 index 00000000000000..41c42320dd6379 --- /dev/null +++ b/test/addons/esm-package-dependent/test-import.js @@ -0,0 +1,15 @@ +// Flags: --experimental-addon-modules +'use strict'; +const common = require('../../common'); +const assert = require('node:assert'); + +/** + * Test that the export condition `node-addons` can be used + * with `*.node` files with the ESM loader. + */ + +import(`esm-package/${common.buildType}`) + .then((mod) => { + assert.strictEqual(mod.default.hello(), 'world'); + }) + .then(common.mustCall()); diff --git a/test/addons/esm-package-dependent/test-require.js b/test/addons/esm-package-dependent/test-require.js new file mode 100644 index 00000000000000..ee6aa223a092b6 --- /dev/null +++ b/test/addons/esm-package-dependent/test-require.js @@ -0,0 +1,15 @@ +// Flags: --experimental-addon-modules +'use strict'; +const common = require('../../common'); +const assert = require('node:assert'); + +/** + * Test that the export condition `node-addons` can be used + * with `*.node` files with the CJS loader. + */ + +require(`esm-package/${common.buildType}`) + .then((mod) => { + assert.strictEqual(mod.hello(), 'world'); + }) + .then(common.mustCall()); diff --git a/test/addons/esm/binding-export-default.cc b/test/addons/esm-package/binding-export-default.cc similarity index 100% rename from test/addons/esm/binding-export-default.cc rename to test/addons/esm-package/binding-export-default.cc diff --git a/test/addons/esm/binding-export-primitive.cc b/test/addons/esm-package/binding-export-primitive.cc similarity index 100% rename from test/addons/esm/binding-export-primitive.cc rename to test/addons/esm-package/binding-export-primitive.cc diff --git a/test/addons/esm/binding.cc b/test/addons/esm-package/binding.cc similarity index 100% rename from test/addons/esm/binding.cc rename to test/addons/esm-package/binding.cc diff --git a/test/addons/esm/binding.gyp b/test/addons/esm-package/binding.gyp similarity index 100% rename from test/addons/esm/binding.gyp rename to test/addons/esm-package/binding.gyp diff --git a/test/addons/esm-package/package.json b/test/addons/esm-package/package.json new file mode 100644 index 00000000000000..67843d2e7d89b9 --- /dev/null +++ b/test/addons/esm-package/package.json @@ -0,0 +1,11 @@ +{ + "name": "esm-package", + "exports": { + "./Debug": { + "node-addons": "./build/Debug/binding.node" + }, + "./Release": { + "node-addons": "./build/Release/binding.node" + } + } +} diff --git a/test/addons/esm/test-esm.mjs b/test/addons/esm-package/test-esm.mjs similarity index 86% rename from test/addons/esm/test-esm.mjs rename to test/addons/esm-package/test-esm.mjs index 7d694075aeb33f..d274cd6b5cedef 100644 --- a/test/addons/esm/test-esm.mjs +++ b/test/addons/esm-package/test-esm.mjs @@ -1,3 +1,9 @@ +/** + * This file is supposed to be loaded by `test-import.js` and `test-require.js` + * to verify that `import('*.node')` is working properly either been loaded with + * the ESM loader or the CJS loader. + */ + // eslint-disable-next-line node-core/require-common-first import { buildType } from '../../common/index.mjs'; import assert from 'node:assert'; @@ -9,6 +15,7 @@ export async function run() { // binding.node { const bindingPath = require.resolve(`./build/${buildType}/binding.node`); + // Test with order of import+require const { default: binding, 'module.exports': exports } = await import(bindingPath); assert.strictEqual(binding, exports); assert.strictEqual(binding.hello(), 'world'); @@ -28,14 +35,14 @@ export async function run() { // binding-export-default.node { const bindingPath = require.resolve(`./build/${buildType}/binding-export-default.node`); + // Test with order of require+import + const bindingRequire = require(bindingPath); const ns = await import(bindingPath); + assert.strictEqual(ns.default, bindingRequire); // As same as ESM-import-CJS, the default export is the value of `module.exports`. assert.strictEqual(ns.default, ns['module.exports']); assert.strictEqual(ns.default.default(), 'hello world'); - - const bindingRequire = require(bindingPath); - assert.strictEqual(ns.default, bindingRequire); } // binding-export-primitive.node diff --git a/test/addons/esm/test-import.js b/test/addons/esm-package/test-import.js similarity index 99% rename from test/addons/esm/test-import.js rename to test/addons/esm-package/test-import.js index 4e56ed13a3fd24..6ff6bee660c6e9 100644 --- a/test/addons/esm/test-import.js +++ b/test/addons/esm-package/test-import.js @@ -1,5 +1,4 @@ // Flags: --experimental-addon-modules - 'use strict'; const common = require('../../common'); diff --git a/test/addons/esm/test-require.js b/test/addons/esm-package/test-require.js similarity index 99% rename from test/addons/esm/test-require.js rename to test/addons/esm-package/test-require.js index 784114e8a21f5f..79a0904ebd1377 100644 --- a/test/addons/esm/test-require.js +++ b/test/addons/esm-package/test-require.js @@ -1,6 +1,5 @@ // Flags: --experimental-addon-modules 'use strict'; - const common = require('../../common'); require('./test-esm.mjs')