From 72336233f2f8f282d61bf841859f11c6b559c056 Mon Sep 17 00:00:00 2001 From: Moshe Atlow Date: Thu, 19 Dec 2024 00:30:00 +0200 Subject: [PATCH 01/61] meta: move MoLow to TSC regular member MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-URL: https://github.com/nodejs/node/pull/56276 Reviewed-By: Marco Ippolito Reviewed-By: Richard Lau Reviewed-By: Michaël Zasso Reviewed-By: Paolo Insogna Reviewed-By: Benjamin Gruenbaum Reviewed-By: Luigi Pinca Reviewed-By: Matteo Collina Reviewed-By: Joyee Cheung Reviewed-By: Ruben Bridgewater Reviewed-By: Chengzhong Wu --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 35a30716eb668c..40bcca0ed591cd 100644 --- a/README.md +++ b/README.md @@ -180,8 +180,6 @@ For information about the governance of the Node.js project, see **Matteo Collina** <> (he/him) * [mhdawson](https://github.com/mhdawson) - **Michael Dawson** <> (he/him) -* [MoLow](https://github.com/MoLow) - - **Moshe Atlow** <> (he/him) * [RafaelGSS](https://github.com/RafaelGSS) - **Rafael Gonzaga** <> (he/him) * [richardlau](https://github.com/richardlau) - @@ -211,6 +209,8 @@ For information about the governance of the Node.js project, see **Shelley Vohr** <> (she/her) * [GeoffreyBooth](https://github.com/GeoffreyBooth) - **Geoffrey Booth** <> (he/him) +* [MoLow](https://github.com/MoLow) - + **Moshe Atlow** <> (he/him) * [Trott](https://github.com/Trott) - **Rich Trott** <> (he/him) From 46fb69daca841828460f1123a4acc5f390920564 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Thu, 19 Dec 2024 04:24:51 +0100 Subject: [PATCH 02/61] build: build v8 with -fvisibility=hidden on macOS MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit V8 should be built with -fvisibility=hidden, otherwise the resulting binary would contain unnecessary symbols. In particular, on macOS, this leads to 5000+ weak symbols resolved at runtime, leading to a startup regression. On macOS this also reduces the binary size about ~10MB. It's only enabled on macOS in this patch as gcc can time out or run out of memory on some machines in the CI with -fvisibility=hidden. PR-URL: https://github.com/nodejs/node/pull/56275 Fixes: https://github.com/nodejs/performance/issues/180 Reviewed-By: Juan José Arboleda Reviewed-By: Daniel Lemire Reviewed-By: Rafael Gonzaga Reviewed-By: Yagiz Nizipli Reviewed-By: Geoffrey Booth Reviewed-By: Luigi Pinca Reviewed-By: Michaël Zasso Reviewed-By: Matteo Collina Reviewed-By: Chengzhong Wu Reviewed-By: Marco Ippolito --- tools/v8_gypfiles/v8.gyp | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tools/v8_gypfiles/v8.gyp b/tools/v8_gypfiles/v8.gyp index 9acad07d966a35..88c1297b9a09ec 100644 --- a/tools/v8_gypfiles/v8.gyp +++ b/tools/v8_gypfiles/v8.gyp @@ -41,6 +41,19 @@ 'AdditionalOptions': ['/utf-8'] } }, + 'conditions': [ + ['OS=="mac"', { + # Hide symbols that are not explicitly exported with V8_EXPORT. + # TODO(joyeecheung): enable it on other platforms. Currently gcc times out + # or run out of memory with -fvisibility=hidden on some machines in the CI. + 'xcode_settings': { + 'GCC_SYMBOLS_PRIVATE_EXTERN': 'YES', # -fvisibility=hidden + }, + 'defines': [ + 'BUILDING_V8_SHARED', # Make V8_EXPORT visible. + ], + }], + ], }, 'targets': [ { From 4c978b4d777f8b6b783463a38fb55f38777eafde Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Thu, 19 Dec 2024 11:34:42 +0100 Subject: [PATCH 03/61] doc: fix links in `module.md` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-URL: https://github.com/nodejs/node/pull/56283 Reviewed-By: Michaël Zasso Reviewed-By: Jacob Smith Reviewed-By: Chengzhong Wu Reviewed-By: Luigi Pinca --- doc/api/module.md | 23 +++++++++-------------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/doc/api/module.md b/doc/api/module.md index 40615c39728cab..221f53055da77c 100644 --- a/doc/api/module.md +++ b/doc/api/module.md @@ -1158,13 +1158,13 @@ validating the import attributes. The final value of `format` must be one of the following: -| `format` | Description | Acceptable types for `source` returned by `load` | -| ------------ | ------------------------------ | -------------------------------------------------------------------------- | -| `'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`][] } | -| `'module'` | Load an ES module | { [`string`][], [`ArrayBuffer`][], [`TypedArray`][] } | -| `'wasm'` | Load a WebAssembly module | { [`ArrayBuffer`][], [`TypedArray`][] } | +| `format` | Description | Acceptable types for `source` returned by `load` | +| ------------ | ------------------------------ | -------------------------------------------------- | +| `'builtin'` | Load a Node.js builtin module | {null} | +| `'commonjs'` | Load a Node.js CommonJS module | {string\|ArrayBuffer\|TypedArray\|null\|undefined} | +| `'json'` | Load a JSON file | {string\|ArrayBuffer\|TypedArray} | +| `'module'` | Load an ES module | {string\|ArrayBuffer\|TypedArray} | +| `'wasm'` | Load a WebAssembly module | {ArrayBuffer\|TypedArray} | The value of `source` is ignored for type `'builtin'` because currently it is not possible to replace the value of a Node.js builtin (core) module. @@ -1221,8 +1221,8 @@ of module format. > These types all correspond to classes defined in ECMAScript. -* The specific [`ArrayBuffer`][] object is a [`SharedArrayBuffer`][]. -* The specific [`TypedArray`][] object is a [`Uint8Array`][]. +* The specific {ArrayBuffer} object is a {SharedArrayBuffer}. +* The specific {TypedArray} object is a {Uint8Array}. If the source value of a text-based format (i.e., `'json'`, `'module'`) is not a string, it is converted to a string using [`util.TextDecoder`][]. @@ -1701,14 +1701,10 @@ returned object contains the following keys: [`--enable-source-maps`]: cli.md#--enable-source-maps [`--import`]: cli.md#--importmodule [`--require`]: cli.md#-r---require-module -[`ArrayBuffer`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/ArrayBuffer [`NODE_COMPILE_CACHE=dir`]: cli.md#node_compile_cachedir [`NODE_DISABLE_COMPILE_CACHE=1`]: cli.md#node_disable_compile_cache1 [`NODE_V8_COVERAGE=dir`]: cli.md#node_v8_coveragedir -[`SharedArrayBuffer`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/SharedArrayBuffer [`SourceMap`]: #class-modulesourcemap -[`TypedArray`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/TypedArray -[`Uint8Array`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Uint8Array [`initialize`]: #initialize [`module.constants.compileCacheStatus`]: #moduleconstantscompilecachestatus [`module.enableCompileCache()`]: #moduleenablecompilecachecachedir @@ -1718,7 +1714,6 @@ returned object contains the following keys: [`os.tmpdir()`]: os.md#ostmpdir [`registerHooks`]: #moduleregisterhooksoptions [`register`]: #moduleregisterspecifier-parenturl-options -[`string`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String [`util.TextDecoder`]: util.md#class-utiltextdecoder [chain]: #chaining [hooks]: #customization-hooks From c1627e9d190e539b91810ade0e252a50244ec542 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Thu, 19 Dec 2024 18:11:19 +0100 Subject: [PATCH 04/61] test: make `test-permission-sqlite-load-extension` more robust MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-URL: https://github.com/nodejs/node/pull/56295 Reviewed-By: Richard Lau Reviewed-By: Luigi Pinca Reviewed-By: Michaël Zasso Reviewed-By: Xuguang Mei --- .../test-permission-sqlite-load-extension.js | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/test/parallel/test-permission-sqlite-load-extension.js b/test/parallel/test-permission-sqlite-load-extension.js index 28d750d0cd06b6..1e6f7426ed9400 100644 --- a/test/parallel/test-permission-sqlite-load-extension.js +++ b/test/parallel/test-permission-sqlite-load-extension.js @@ -1,18 +1,16 @@ 'use strict'; const common = require('../common'); const assert = require('node:assert'); -const childProcess = require('child_process'); const code = `const sqlite = require('node:sqlite'); const db = new sqlite.DatabaseSync(':memory:', { allowExtension: true }); db.loadExtension('nonexistent');`.replace(/\n/g, ' '); -childProcess.exec( - `${process.execPath} --permission -e "${code}"`, - {}, - common.mustCall((err, _, stderr) => { - assert.strictEqual(err.code, 1); - assert.match(stderr, /Error: Cannot load SQLite extensions when the permission model is enabled/); - assert.match(stderr, /code: 'ERR_LOAD_SQLITE_EXTENSION'/); - }) -); +common.spawnPromisified( + process.execPath, + ['--permission', '--eval', code], +).then(common.mustCall(({ code, stderr }) => { + assert.match(stderr, /Error: Cannot load SQLite extensions when the permission model is enabled/); + assert.match(stderr, /code: 'ERR_LOAD_SQLITE_EXTENSION'/); + assert.strictEqual(code, 1); +})); From db83d2f0eee31de8f5b8b87e4fe2788f136587c5 Mon Sep 17 00:00:00 2001 From: origranot Date: Tue, 17 Dec 2024 10:25:04 +0200 Subject: [PATCH 05/61] Revert "events: add hasEventListener util for validate" This reverts commit https://github.com/nodejs/node/commit/bdb6d12. PR-URL: https://github.com/nodejs/node/pull/56282 Reviewed-By: Luigi Pinca Reviewed-By: Jason Zhang Reviewed-By: Jake Yuesong Li --- lib/events.js | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/lib/events.js b/lib/events.js index e8fd3bbb79259e..50cc720b1247ba 100644 --- a/lib/events.js +++ b/lib/events.js @@ -276,12 +276,6 @@ ObjectDefineProperty(EventEmitter, 'defaultMaxListeners', { }, }); -function hasEventListener(self, type) { - if (type === undefined) - return self._events !== undefined; - return self._events !== undefined && self._events[type] !== undefined; -}; - ObjectDefineProperties(EventEmitter, { kMaxEventTargetListeners: { __proto__: null, @@ -675,11 +669,13 @@ EventEmitter.prototype.removeListener = function removeListener(type, listener) { checkListener(listener); - if (!hasEventListener(this, type)) + const events = this._events; + if (events === undefined) return this; - const events = this._events; const list = events[type]; + if (list === undefined) + return this; if (list === listener || list.listener === listener) { this._eventsCount -= 1; @@ -733,9 +729,9 @@ EventEmitter.prototype.off = EventEmitter.prototype.removeListener; */ EventEmitter.prototype.removeAllListeners = function removeAllListeners(type) { - if (!hasEventListener(this)) - return this; const events = this._events; + if (events === undefined) + return this; // Not listening for removeListener, no need to emit if (events.removeListener === undefined) { @@ -780,10 +776,14 @@ EventEmitter.prototype.removeAllListeners = }; function _listeners(target, type, unwrap) { - if (!hasEventListener(target, type)) + const events = target._events; + + if (events === undefined) return []; - const evlistener = target._events[type]; + const evlistener = events[type]; + if (evlistener === undefined) + return []; if (typeof evlistener === 'function') return unwrap ? [evlistener.listener || evlistener] : [evlistener]; From e9762bf005ecf40e484a0fbbca4b362b1fd97ea8 Mon Sep 17 00:00:00 2001 From: origranot Date: Tue, 17 Dec 2024 10:27:59 +0200 Subject: [PATCH 06/61] test: add test case for listeners PR-URL: https://github.com/nodejs/node/pull/56282 Reviewed-By: Luigi Pinca Reviewed-By: Jason Zhang Reviewed-By: Jake Yuesong Li --- test/parallel/test-event-emitter-listeners.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/parallel/test-event-emitter-listeners.js b/test/parallel/test-event-emitter-listeners.js index eb1da829c95f21..4a08ad34c273cd 100644 --- a/test/parallel/test-event-emitter-listeners.js +++ b/test/parallel/test-event-emitter-listeners.js @@ -86,6 +86,11 @@ function listener4() { assert.deepStrictEqual(ee.listeners('foo'), []); } +{ + const ee = new events.EventEmitter(); + assert.deepStrictEqual(ee.listeners(), []); +} + { class TestStream extends events.EventEmitter {} const s = new TestStream(); From 7819bfec69b5dd3556454fc3b10024237a3846ca Mon Sep 17 00:00:00 2001 From: Luigi Pinca Date: Thu, 19 Dec 2024 23:30:28 +0100 Subject: [PATCH 07/61] test: deflake test-esm-loader-hooks-inspect-brk Refs: https://github.com/nodejs/node/pull/54827 Refs: https://github.com/nodejs/node/pull/51560 PR-URL: https://github.com/nodejs/node/pull/56050 Reviewed-By: Antoine du Hamel --- .../test-esm-loader-hooks-inspect-brk.js | 21 ++++++++----------- 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/test/parallel/test-esm-loader-hooks-inspect-brk.js b/test/parallel/test-esm-loader-hooks-inspect-brk.js index 881bdfd2dd101a..251ebb230dcd31 100644 --- a/test/parallel/test-esm-loader-hooks-inspect-brk.js +++ b/test/parallel/test-esm-loader-hooks-inspect-brk.js @@ -10,23 +10,20 @@ const assert = require('assert'); const fixtures = require('../common/fixtures'); const { NodeInstance } = require('../common/inspector-helper.js'); -async function runIfWaitingForDebugger(session) { - const commands = [ - { 'method': 'Runtime.enable' }, - { 'method': 'Debugger.enable' }, - { 'method': 'Runtime.runIfWaitingForDebugger' }, - ]; - - await session.send(commands); - await session.waitForNotification('Debugger.paused'); -} - async function runTest() { const main = fixtures.path('es-module-loaders', 'register-loader.mjs'); const child = new NodeInstance(['--inspect-brk=0'], '', main); const session = await child.connectInspectorSession(); - await runIfWaitingForDebugger(session); + await session.send({ method: 'NodeRuntime.enable' }); + await session.waitForNotification('NodeRuntime.waitingForDebugger'); + await session.send([ + { 'method': 'Runtime.enable' }, + { 'method': 'Debugger.enable' }, + { 'method': 'Runtime.runIfWaitingForDebugger' }, + ]); + await session.send({ method: 'NodeRuntime.disable' }); + await session.waitForNotification('Debugger.paused'); await session.runToCompletion(); assert.strictEqual((await child.expectShutdown()).exitCode, 0); } From d1b009b623b2110a75a6ab9e53be4f87da55c154 Mon Sep 17 00:00:00 2001 From: Chengzhong Wu Date: Tue, 17 Dec 2024 23:12:00 +0000 Subject: [PATCH 08/61] lib: suppress source map lookup exceptions When the source map data are invalid json strings, skip construct `SourceMap` on it. Additionally, suppress exceptions on source map lookups and fix test runners crash on invalid source maps. PR-URL: https://github.com/nodejs/node/pull/56299 Refs: https://github.com/nodejs/node/issues/56296 Reviewed-By: Matteo Collina Reviewed-By: Xuguang Mei Reviewed-By: Yagiz Nizipli Reviewed-By: Colin Ihrig Reviewed-By: Chemi Atlow Reviewed-By: Pietro Marchini --- lib/internal/source_map/source_map_cache.js | 39 ++++++++++++++----- .../test-runner-source-maps-invalid-json.js | 12 ++++++ 2 files changed, 41 insertions(+), 10 deletions(-) create mode 100644 test/parallel/test-runner-source-maps-invalid-json.js diff --git a/lib/internal/source_map/source_map_cache.js b/lib/internal/source_map/source_map_cache.js index dfb42f83f6f1b1..ea87a579636671 100644 --- a/lib/internal/source_map/source_map_cache.js +++ b/lib/internal/source_map/source_map_cache.js @@ -155,6 +155,9 @@ function maybeCacheSourceMap(filename, content, moduleInstance, isGeneratedSourc } const data = dataFromUrl(filename, sourceMapURL); + // `data` could be null if the source map is invalid. + // In this case, create a cache entry with null data with source url for test coverage. + const entry = { __proto__: null, lineLengths: lineLengths(content), @@ -277,6 +280,8 @@ function sourceMapFromDataUrl(sourceURL, url) { const parsedData = JSONParse(decodedData); return sourcesToAbsolute(sourceURL, parsedData); } catch (err) { + // TODO(legendecas): warn about invalid source map JSON string. + // But it could be verbose. debug(err); return null; } @@ -331,24 +336,38 @@ function sourceMapCacheToObject() { /** * Find a source map for a given actual source URL or path. + * + * This function may be invoked from user code or test runner, this must not throw + * any exceptions. * @param {string} sourceURL - actual source URL or path * @returns {import('internal/source_map/source_map').SourceMap | undefined} a source map or undefined if not found */ function findSourceMap(sourceURL) { - if (RegExpPrototypeExec(kLeadingProtocol, sourceURL) === null) { - sourceURL = pathToFileURL(sourceURL).href; + if (typeof sourceURL !== 'string') { + return undefined; } + SourceMap ??= require('internal/source_map/source_map').SourceMap; - const entry = getModuleSourceMapCache().get(sourceURL) ?? generatedSourceMapCache.get(sourceURL); - if (entry === undefined) { + try { + if (RegExpPrototypeExec(kLeadingProtocol, sourceURL) === null) { + // If the sourceURL is an invalid path, this will throw an error. + sourceURL = pathToFileURL(sourceURL).href; + } + const entry = getModuleSourceMapCache().get(sourceURL) ?? generatedSourceMapCache.get(sourceURL); + if (entry?.data == null) { + return undefined; + } + + let sourceMap = entry.sourceMap; + if (sourceMap === undefined) { + sourceMap = new SourceMap(entry.data, { lineLengths: entry.lineLengths }); + entry.sourceMap = sourceMap; + } + return sourceMap; + } catch (err) { + debug(err); return undefined; } - let sourceMap = entry.sourceMap; - if (sourceMap === undefined) { - sourceMap = new SourceMap(entry.data, { lineLengths: entry.lineLengths }); - entry.sourceMap = sourceMap; - } - return sourceMap; } module.exports = { diff --git a/test/parallel/test-runner-source-maps-invalid-json.js b/test/parallel/test-runner-source-maps-invalid-json.js new file mode 100644 index 00000000000000..508e2d432117f7 --- /dev/null +++ b/test/parallel/test-runner-source-maps-invalid-json.js @@ -0,0 +1,12 @@ +// Flags: --enable-source-maps +'use strict'; + +require('../common'); +const test = require('node:test'); + +// Verify that test runner can handle invalid source maps. + +test('ok', () => {}); + +// eslint-disable-next-line @stylistic/js/spaced-comment +//# sourceMappingURL=data:application/json;base64,-1 From e4b795ec4a1842626fed7740e006488538851b76 Mon Sep 17 00:00:00 2001 From: Chengzhong Wu Date: Tue, 17 Dec 2024 23:38:54 +0000 Subject: [PATCH 09/61] lib: optimize `prepareStackTrace` on builtin frames Only invalidates source map lookup cache when a new source map is found. This improves when user codes interleave with builtin functions, like `array.map`. PR-URL: https://github.com/nodejs/node/pull/56299 Refs: https://github.com/nodejs/node/issues/56296 Reviewed-By: Matteo Collina Reviewed-By: Xuguang Mei Reviewed-By: Yagiz Nizipli Reviewed-By: Colin Ihrig Reviewed-By: Chemi Atlow Reviewed-By: Pietro Marchini --- benchmark/fixtures/simple-error-stack.js | 21 ++++++++++--------- benchmark/fixtures/simple-error-stack.ts | 12 ++++++----- .../source_map/prepare_stack_trace.js | 10 +++++++-- lib/internal/source_map/source_map_cache.js | 5 +++++ 4 files changed, 31 insertions(+), 17 deletions(-) diff --git a/benchmark/fixtures/simple-error-stack.js b/benchmark/fixtures/simple-error-stack.js index 0057807795b072..74aae191800778 100644 --- a/benchmark/fixtures/simple-error-stack.js +++ b/benchmark/fixtures/simple-error-stack.js @@ -1,15 +1,16 @@ 'use strict'; -exports.__esModule = true; -exports.simpleErrorStack = void 0; +Object.defineProperty(exports, "__esModule", { value: true }); +exports.simpleErrorStack = simpleErrorStack; // Compile with `tsc --inlineSourceMap benchmark/fixtures/simple-error-stack.ts`. var lorem = 'Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.'; function simpleErrorStack() { - try { - lorem.BANG(); - } - catch (e) { - return e.stack; - } + [1].map(function () { + try { + lorem.BANG(); + } + catch (e) { + return e.stack; + } + }); } -exports.simpleErrorStack = simpleErrorStack; -//# sourceMappingURL=data:application/json;base64,eyJ2ZXJzaW9uIjozLCJmaWxlIjoic2ltcGxlLWVycm9yLXN0YWNrLmpzIiwic291cmNlUm9vdCI6IiIsInNvdXJjZXMiOlsic2ltcGxlLWVycm9yLXN0YWNrLnRzIl0sIm5hbWVzIjpbXSwibWFwcGluZ3MiOiJBQUFBLFlBQVksQ0FBQzs7O0FBRWIsaUZBQWlGO0FBRWpGLElBQU0sS0FBSyxHQUFHLCtiQUErYixDQUFDO0FBRTljLFNBQVMsZ0JBQWdCO0lBQ3ZCLElBQUk7UUFDRCxLQUFhLENBQUMsSUFBSSxFQUFFLENBQUM7S0FDdkI7SUFBQyxPQUFPLENBQUMsRUFBRTtRQUNWLE9BQU8sQ0FBQyxDQUFDLEtBQUssQ0FBQztLQUNoQjtBQUNILENBQUM7QUFHQyw0Q0FBZ0IifQ== +//# sourceMappingURL=data:application/json;base64,eyJ2ZXJzaW9uIjozLCJmaWxlIjoic2ltcGxlLWVycm9yLXN0YWNrLmpzIiwic291cmNlUm9vdCI6IiIsInNvdXJjZXMiOlsic2ltcGxlLWVycm9yLXN0YWNrLnRzIl0sIm5hbWVzIjpbXSwibWFwcGluZ3MiOiJBQUFBLFlBQVksQ0FBQzs7QUFpQlgsNENBQWdCO0FBZmxCLGlGQUFpRjtBQUVqRixJQUFNLEtBQUssR0FBRywrYkFBK2IsQ0FBQztBQUU5YyxTQUFTLGdCQUFnQjtJQUN2QixDQUFDLENBQUMsQ0FBQyxDQUFDLEdBQUcsQ0FBQztRQUNOLElBQUksQ0FBQztZQUNGLEtBQWEsQ0FBQyxJQUFJLEVBQUUsQ0FBQztRQUN4QixDQUFDO1FBQUMsT0FBTyxDQUFDLEVBQUUsQ0FBQztZQUNYLE9BQU8sQ0FBQyxDQUFDLEtBQUssQ0FBQztRQUNqQixDQUFDO0lBQ0gsQ0FBQyxDQUFDLENBQUE7QUFDSixDQUFDIn0= \ No newline at end of file diff --git a/benchmark/fixtures/simple-error-stack.ts b/benchmark/fixtures/simple-error-stack.ts index 58034e92f24b98..1335df3478b99b 100644 --- a/benchmark/fixtures/simple-error-stack.ts +++ b/benchmark/fixtures/simple-error-stack.ts @@ -5,11 +5,13 @@ const lorem = 'Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.'; function simpleErrorStack() { - try { - (lorem as any).BANG(); - } catch (e) { - return e.stack; - } + [1].map(() => { + try { + (lorem as any).BANG(); + } catch (e) { + return e.stack; + } + }) } export { diff --git a/lib/internal/source_map/prepare_stack_trace.js b/lib/internal/source_map/prepare_stack_trace.js index 60c9d1ed3316ff..3e4b0825e7b3a5 100644 --- a/lib/internal/source_map/prepare_stack_trace.js +++ b/lib/internal/source_map/prepare_stack_trace.js @@ -53,9 +53,15 @@ function prepareStackTraceWithSourceMaps(error, trace) { const sm = fileName === lastFileName ? lastSourceMap : findSourceMap(fileName); - lastSourceMap = sm; - lastFileName = fileName; + // Only when a source map is found, cache it for the next iteration. + // This is a performance optimization to avoid interleaving with JS builtin function + // invalidating the cache. + // - at myFunc (file:///path/to/file.js:1:2) + // - at Array.map () + // - at myFunc (file:///path/to/file.js:3:4) if (sm) { + lastSourceMap = sm; + lastFileName = fileName; return `${kStackLineAt}${serializeJSStackFrame(sm, callSite, trace[i + 1])}`; } } catch (err) { diff --git a/lib/internal/source_map/source_map_cache.js b/lib/internal/source_map/source_map_cache.js index ea87a579636671..aaca27136e66a0 100644 --- a/lib/internal/source_map/source_map_cache.js +++ b/lib/internal/source_map/source_map_cache.js @@ -347,6 +347,11 @@ function findSourceMap(sourceURL) { return undefined; } + // No source maps for builtin modules. + if (sourceURL.startsWith('node:')) { + return undefined; + } + SourceMap ??= require('internal/source_map/source_map').SourceMap; try { if (RegExpPrototypeExec(kLeadingProtocol, sourceURL) === null) { From 97d854e1d5d56ed0d77a0b146f7cba7811802448 Mon Sep 17 00:00:00 2001 From: Colin Ihrig Date: Thu, 19 Dec 2024 21:10:26 -0500 Subject: [PATCH 10/61] test_runner,cli: mark test isolation as stable This commit stabilizes test isolation configuration in the test runner. PR-URL: https://github.com/nodejs/node/pull/56298 Reviewed-By: Yagiz Nizipli Reviewed-By: Matteo Collina Reviewed-By: Moshe Atlow Reviewed-By: Pietro Marchini Reviewed-By: Chemi Atlow Reviewed-By: Jake Yuesong Li --- doc/api/cli.md | 37 +++++++++++-------- doc/node.1 | 6 +-- lib/internal/test_runner/utils.js | 2 +- src/node_options.cc | 10 +++-- test/parallel/test-runner-cli-concurrency.js | 4 +- test/parallel/test-runner-cli-timeout.js | 2 +- test/parallel/test-runner-cli.js | 18 ++++----- test/parallel/test-runner-coverage.js | 2 +- .../test-runner-extraneous-async-activity.js | 2 +- .../test-runner-force-exit-failure.js | 2 +- .../test-runner-no-isolation-filtering.js | 8 ++-- .../test-runner-no-isolation-hooks.mjs | 2 +- test/parallel/test-runner-snapshot-tests.js | 6 +-- test/parallel/test-runner-watch-mode.mjs | 2 +- 14 files changed, 55 insertions(+), 48 deletions(-) diff --git a/doc/api/cli.md b/doc/api/cli.md index 271ede0755cefb..d54b56aff9c891 100644 --- a/doc/api/cli.md +++ b/doc/api/cli.md @@ -1037,20 +1037,6 @@ generated as part of the test runner output. If no tests are run, a coverage report is not generated. See the documentation on [collecting code coverage from tests][] for more details. -### `--experimental-test-isolation=mode` - - - -> Stability: 1.0 - Early development - -Configures the type of test isolation used in the test runner. When `mode` is -`'process'`, each test file is run in a separate child process. When `mode` is -`'none'`, all test files run in the same process as the test runner. The default -isolation mode is `'process'`. This flag is ignored if the `--test` flag is not -present. See the [test runner execution model][] section for more information. - ### `--experimental-test-module-mocks` The maximum number of test files that the test runner CLI will execute -concurrently. If `--experimental-test-isolation` is set to `'none'`, this flag -is ignored and concurrency is one. Otherwise, concurrency defaults to +concurrently. If `--test-isolation` is set to `'none'`, this flag is ignored and +concurrency is one. Otherwise, concurrency defaults to `os.availableParallelism() - 1`. ### `--test-coverage-branches=threshold` @@ -2320,6 +2306,23 @@ added: Configures the test runner to exit the process once all known tests have finished executing even if the event loop would otherwise remain active. +### `--test-isolation=mode` + + + +Configures the type of test isolation used in the test runner. When `mode` is +`'process'`, each test file is run in a separate child process. When `mode` is +`'none'`, all test files run in the same process as the test runner. The default +isolation mode is `'process'`. This flag is ignored if the `--test` flag is not +present. See the [test runner execution model][] section for more information. + ### `--test-name-pattern` + +> Stability: 1.0 - Early development + +Enable experimental import support for `.node` addons. + ### `--experimental-async-context-frame` This configures Node.js to interpret `--eval` or `STDIN` input as CommonJS or -as an ES module. Valid values are `"commonjs"` or `"module"`. The default is -`"commonjs"`. +as an ES module. Valid values are `"commonjs"`, `"module"`, `"module-typescript"` and `"commonjs-typescript"`. +The `"-typescript"` values are available only in combination with the flag `--experimental-strip-types`. +The default is `"commonjs"`. + +If `--experimental-strip-types` is enabled and `--input-type` is not provided, +Node.js will try to detect the syntax with the following steps: + +1. Run the input as CommonJS. +2. If step 1 fails, run the input as an ES module. +3. If step 2 fails with a SyntaxError, strip the types. +4. If step 3 fails with an error code [`ERR_INVALID_TYPESCRIPT_SYNTAX`][], + throw the error from step 2, including the TypeScript error in the message, + else run as CommonJS. +5. If step 4 fails, run the input as an ES module. + +To avoid the delay of multiple syntax detection passes, the `--input-type=type` flag can be used to specify +how the `--eval` input should be interpreted. The REPL does not support this option. Usage of `--input-type=module` with [`--print`][] will throw an error, as `--print` does not support ES module @@ -3645,6 +3660,7 @@ node --stack-trace-limit=12 -p -e "Error.stackTraceLimit" # prints 12 [`AsyncLocalStorage`]: async_context.md#class-asynclocalstorage [`Buffer`]: buffer.md#class-buffer [`CRYPTO_secure_malloc_init`]: https://www.openssl.org/docs/man3.0/man3/CRYPTO_secure_malloc_init.html +[`ERR_INVALID_TYPESCRIPT_SYNTAX`]: errors.md#err_invalid_typescript_syntax [`NODE_OPTIONS`]: #node_optionsoptions [`NO_COLOR`]: https://no-color.org [`SlowBuffer`]: buffer.md#class-slowbuffer diff --git a/lib/internal/main/eval_string.js b/lib/internal/main/eval_string.js index 6518c93430a28e..ee402f50fbdd2b 100644 --- a/lib/internal/main/eval_string.js +++ b/lib/internal/main/eval_string.js @@ -13,9 +13,14 @@ const { prepareMainThreadExecution, markBootstrapComplete, } = require('internal/process/pre_execution'); -const { evalModuleEntryPoint, evalScript } = require('internal/process/execution'); +const { + evalModuleEntryPoint, + evalTypeScript, + parseAndEvalCommonjsTypeScript, + parseAndEvalModuleTypeScript, + evalScript, +} = require('internal/process/execution'); const { addBuiltinLibsToObject } = require('internal/modules/helpers'); -const { stripTypeScriptModuleTypes } = require('internal/modules/typescript'); const { getOptionValue } = require('internal/options'); prepareMainThreadExecution(); @@ -23,18 +28,19 @@ addBuiltinLibsToObject(globalThis, ''); markBootstrapComplete(); const code = getOptionValue('--eval'); -const source = getOptionValue('--experimental-strip-types') ? - stripTypeScriptModuleTypes(code) : - code; const print = getOptionValue('--print'); const shouldLoadESM = getOptionValue('--import').length > 0 || getOptionValue('--experimental-loader').length > 0; -if (getOptionValue('--input-type') === 'module') { - evalModuleEntryPoint(source, print); +const inputType = getOptionValue('--input-type'); +const tsEnabled = getOptionValue('--experimental-strip-types'); +if (inputType === 'module') { + evalModuleEntryPoint(code, print); +} else if (inputType === 'module-typescript' && tsEnabled) { + parseAndEvalModuleTypeScript(code, print); } else { // For backward compatibility, we want the identifier crypto to be the // `node:crypto` module rather than WebCrypto. - const isUsingCryptoIdentifier = RegExpPrototypeExec(/\bcrypto\b/, source) !== null; + const isUsingCryptoIdentifier = RegExpPrototypeExec(/\bcrypto\b/, code) !== null; const shouldDefineCrypto = isUsingCryptoIdentifier && internalBinding('config').hasOpenSSL; if (isUsingCryptoIdentifier && !shouldDefineCrypto) { @@ -49,11 +55,24 @@ if (getOptionValue('--input-type') === 'module') { }; ObjectDefineProperty(object, name, { __proto__: null, set: setReal }); } - evalScript('[eval]', - shouldDefineCrypto ? ( - print ? `let crypto=require("node:crypto");{${source}}` : `(crypto=>{{${source}}})(require('node:crypto'))` - ) : source, - getOptionValue('--inspect-brk'), - print, - shouldLoadESM); + + let evalFunction; + if (inputType === 'commonjs') { + evalFunction = evalScript; + } else if (inputType === 'commonjs-typescript' && tsEnabled) { + evalFunction = parseAndEvalCommonjsTypeScript; + } else if (tsEnabled) { + evalFunction = evalTypeScript; + } else { + // Default to commonjs. + evalFunction = evalScript; + } + + evalFunction('[eval]', + shouldDefineCrypto ? ( + print ? `let crypto=require("node:crypto");{${code}}` : `(crypto=>{{${code}}})(require('node:crypto'))` + ) : code, + getOptionValue('--inspect-brk'), + print, + shouldLoadESM); } diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index 0779190e1c9070..c453f2b403e89d 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -449,7 +449,6 @@ function initializeCJS() { const tsEnabled = getOptionValue('--experimental-strip-types'); if (tsEnabled) { - emitExperimentalWarning('Type Stripping'); Module._extensions['.cts'] = loadCTS; Module._extensions['.ts'] = loadTS; } diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js index c52f388754d5f1..19eac728623939 100644 --- a/lib/internal/modules/esm/loader.js +++ b/lib/internal/modules/esm/loader.js @@ -213,9 +213,25 @@ class ModuleLoader { } } - async eval(source, url, isEntryPoint = false) { + /** + * + * @param {string} source Source code of the module. + * @param {string} url URL of the module. + * @returns {object} The module wrap object. + */ + createModuleWrap(source, url) { + return compileSourceTextModule(url, source, this); + } + + /** + * + * @param {string} url URL of the module. + * @param {object} wrap Module wrap object. + * @param {boolean} isEntryPoint Whether the module is the entry point. + * @returns {Promise} The module object. + */ + async executeModuleJob(url, wrap, isEntryPoint = false) { const { ModuleJob } = require('internal/modules/esm/module_job'); - const wrap = compileSourceTextModule(url, source, this); const module = await onImport.tracePromise(async () => { const job = new ModuleJob( this, url, undefined, wrap, false, false); @@ -235,6 +251,18 @@ class ModuleLoader { }; } + /** + * + * @param {string} source Source code of the module. + * @param {string} url URL of the module. + * @param {boolean} isEntryPoint Whether the module is the entry point. + * @returns {Promise} The module object. + */ + eval(source, url, isEntryPoint = false) { + const wrap = this.createModuleWrap(source, url); + return this.executeModuleJob(url, wrap, isEntryPoint); + } + /** * Get a (possibly not yet fully linked) module job from the cache, or create one and return its Promise. * @param {string} specifier The module request of the module to be resolved. Typically, what's diff --git a/lib/internal/modules/esm/translators.js b/lib/internal/modules/esm/translators.js index 37611a4332c541..678659aacaad3e 100644 --- a/lib/internal/modules/esm/translators.js +++ b/lib/internal/modules/esm/translators.js @@ -291,7 +291,6 @@ translators.set('require-commonjs', (url, source, isMain) => { // Handle CommonJS modules referenced by `require` calls. // This translator function must be sync, as `require` is sync. translators.set('require-commonjs-typescript', (url, source, isMain) => { - emitExperimentalWarning('Type Stripping'); assert(cjsParse); const code = stripTypeScriptModuleTypes(stringify(source), url); return createCJSModuleWrap(url, code, isMain, 'commonjs-typescript'); @@ -536,7 +535,6 @@ translators.set('addon', function translateAddon(url, source, isMain) { // Strategy for loading a commonjs TypeScript module translators.set('commonjs-typescript', function(url, source) { - emitExperimentalWarning('Type Stripping'); assertBufferSource(source, true, 'load'); const code = stripTypeScriptModuleTypes(stringify(source), url); debug(`Translating TypeScript ${url}`); @@ -545,7 +543,6 @@ translators.set('commonjs-typescript', function(url, source) { // Strategy for loading an esm TypeScript module translators.set('module-typescript', function(url, source) { - emitExperimentalWarning('Type Stripping'); assertBufferSource(source, true, 'load'); const code = stripTypeScriptModuleTypes(stringify(source), url); debug(`Translating TypeScript ${url}`); diff --git a/lib/internal/modules/typescript.js b/lib/internal/modules/typescript.js index 2f8f61266b5d03..5a240a6a5403b7 100644 --- a/lib/internal/modules/typescript.js +++ b/lib/internal/modules/typescript.js @@ -113,9 +113,13 @@ function processTypeScriptCode(code, options) { * It is used by internal loaders. * @param {string} source TypeScript code to parse. * @param {string} filename The filename of the source code. + * @param {boolean} emitWarning Whether to emit a warning. * @returns {TransformOutput} The stripped TypeScript code. */ -function stripTypeScriptModuleTypes(source, filename) { +function stripTypeScriptModuleTypes(source, filename, emitWarning = true) { + if (emitWarning) { + emitExperimentalWarning('Type Stripping'); + } assert(typeof source === 'string'); if (isUnderNodeModules(filename)) { throw new ERR_UNSUPPORTED_NODE_MODULES_TYPE_STRIPPING(filename); diff --git a/lib/internal/process/execution.js b/lib/internal/process/execution.js index a5afd44ca9ca06..e8a1a29efcc8a6 100644 --- a/lib/internal/process/execution.js +++ b/lib/internal/process/execution.js @@ -2,6 +2,8 @@ const { RegExpPrototypeExec, + StringPrototypeIndexOf, + StringPrototypeSlice, Symbol, globalThis, } = primordials; @@ -17,6 +19,7 @@ const { } = require('internal/errors'); const { pathToFileURL } = require('internal/url'); const { exitCodes: { kGenericUserError } } = internalBinding('errors'); +const { stripTypeScriptModuleTypes } = require('internal/modules/typescript'); const { executionAsyncId, @@ -32,6 +35,7 @@ const { getOptionValue } = require('internal/options'); const { makeContextifyScript, runScriptInThisContext, } = require('internal/vm'); +const { emitExperimentalWarning, isError } = require('internal/util'); // shouldAbortOnUncaughtToggle is a typed array for faster // communication with JS. const { shouldAbortOnUncaughtToggle } = internalBinding('util'); @@ -70,21 +74,14 @@ function evalModuleEntryPoint(source, print) { } function evalScript(name, body, breakFirstLine, print, shouldLoadESM = false) { - const CJSModule = require('internal/modules/cjs/loader').Module; - - const cwd = tryGetCwd(); const origModule = globalThis.module; // Set e.g. when called from the REPL. - - const module = new CJSModule(name); - module.filename = path.join(cwd, name); - module.paths = CJSModule._nodeModulePaths(cwd); - + const module = createModule(name); const baseUrl = pathToFileURL(module.filename).href; - if (getOptionValue('--experimental-detect-module') && - getOptionValue('--input-type') === '' && - containsModuleSyntax(body, name, null, 'no CJS variables')) { - return evalModuleEntryPoint(body, print); + if (shouldUseModuleEntryPoint(name, body)) { + return getOptionValue('--experimental-strip-types') ? + evalTypeScriptModuleEntryPoint(body, print) : + evalModuleEntryPoint(body, print); } const runScript = () => { @@ -99,23 +96,8 @@ function evalScript(name, body, breakFirstLine, print, shouldLoadESM = false) { globalThis.__filename = name; RegExpPrototypeExec(/^/, ''); // Necessary to reset RegExp statics before user code runs. const result = module._compile(script, `${name}-wrapper`)(() => { - const hostDefinedOptionId = Symbol(name); - async function importModuleDynamically(specifier, _, importAttributes) { - const cascadedLoader = require('internal/modules/esm/loader').getOrInitializeCascadedLoader(); - return cascadedLoader.import(specifier, baseUrl, importAttributes); - } - const script = makeContextifyScript( - body, // code - name, // filename, - 0, // lineOffset - 0, // columnOffset, - undefined, // cachedData - false, // produceCachedData - undefined, // parsingContext - hostDefinedOptionId, // hostDefinedOptionId - importModuleDynamically, // importModuleDynamically - ); - return runScriptInThisContext(script, true, !!breakFirstLine); + const compiledScript = compileScript(name, body, baseUrl); + return runScriptInThisContext(compiledScript, true, !!breakFirstLine); }); if (print) { const { log } = require('internal/console/global'); @@ -238,10 +220,283 @@ function readStdin(callback) { }); } +/** + * Adds the TS message to the error stack. + * + * At the 3rd line of the stack, the message is added. + * @param {string} originalStack The stack to decorate + * @param {string} newMessage the message to add to the error stack + * @returns {void} + */ +function decorateCJSErrorWithTSMessage(originalStack, newMessage) { + let index; + for (let i = 0; i < 3; i++) { + index = StringPrototypeIndexOf(originalStack, '\n', index + 1); + } + return StringPrototypeSlice(originalStack, 0, index) + + '\n' + newMessage + + StringPrototypeSlice(originalStack, index); +} + +/** + * + * Wrapper of evalScript + * + * This function wraps the evaluation of the source code in a try-catch block. + * If the source code fails to be evaluated, it will retry evaluating the source code + * with the TypeScript parser. + * + * If the source code fails to be evaluated with the TypeScript parser, + * it will rethrow the original error, adding the TypeScript error message to the stack. + * + * This way we don't change the behavior of the code, but we provide a better error message + * in case of a typescript error. + * @param {string} name The name of the file + * @param {string} source The source code to evaluate + * @param {boolean} breakFirstLine Whether to break on the first line + * @param {boolean} print If the result should be printed + * @param {boolean} shouldLoadESM If the code should be loaded as an ESM module + * @returns {void} + */ +function evalTypeScript(name, source, breakFirstLine, print, shouldLoadESM = false) { + const origModule = globalThis.module; // Set e.g. when called from the REPL. + const module = createModule(name); + const baseUrl = pathToFileURL(module.filename).href; + + if (shouldUseModuleEntryPoint(name, source)) { + return evalTypeScriptModuleEntryPoint(source, print); + } + + let compiledScript; + // This variable can be modified if the source code is stripped. + let sourceToRun = source; + try { + compiledScript = compileScript(name, source, baseUrl); + } catch (originalError) { + // If it's not a SyntaxError, rethrow it. + if (!isError(originalError) || originalError.name !== 'SyntaxError') { + throw originalError; + } + try { + sourceToRun = stripTypeScriptModuleTypes(source, name, false); + // Retry the CJS/ESM syntax detection after stripping the types. + if (shouldUseModuleEntryPoint(name, sourceToRun)) { + return evalTypeScriptModuleEntryPoint(source, print); + } + // If the ContextifiedScript was successfully created, execute it. + // outside the try-catch block to avoid catching runtime errors. + compiledScript = compileScript(name, sourceToRun, baseUrl); + // Emit the experimental warning after the code was successfully evaluated. + emitExperimentalWarning('Type Stripping'); + } catch (tsError) { + // If its not an error, or it's not an invalid typescript syntax error, rethrow it. + if (!isError(tsError) || tsError?.code !== 'ERR_INVALID_TYPESCRIPT_SYNTAX') { + throw tsError; + } + + try { + originalError.stack = decorateCJSErrorWithTSMessage(originalError.stack, tsError.message); + } catch { /* Ignore potential errors coming from `stack` getter/setter */ } + throw originalError; + } + } + + if (shouldLoadESM) { + return require('internal/modules/run_main').runEntryPointWithESMLoader( + () => runScriptInContext(name, + sourceToRun, + breakFirstLine, + print, + module, + baseUrl, + compiledScript, + origModule)); + } + + runScriptInContext(name, sourceToRun, breakFirstLine, print, module, baseUrl, compiledScript, origModule); +} + +/** + * Wrapper of evalModuleEntryPoint + * + * This function wraps the compilation of the source code in a try-catch block. + * If the source code fails to be compiled, it will retry transpiling the source code + * with the TypeScript parser. + * @param {string} source The source code to evaluate + * @param {boolean} print If the result should be printed + * @returns {Promise} The module evaluation promise + */ +function evalTypeScriptModuleEntryPoint(source, print) { + if (print) { + throw new ERR_EVAL_ESM_CANNOT_PRINT(); + } + + RegExpPrototypeExec(/^/, ''); // Necessary to reset RegExp statics before user code runs. + + return require('internal/modules/run_main').runEntryPointWithESMLoader( + async (loader) => { + const url = getEvalModuleUrl(); + let moduleWrap; + try { + // Compile the module to check for syntax errors. + moduleWrap = loader.createModuleWrap(source, url); + } catch (originalError) { + // If it's not a SyntaxError, rethrow it. + if (!isError(originalError) || originalError.name !== 'SyntaxError') { + throw originalError; + } + let strippedSource; + try { + strippedSource = stripTypeScriptModuleTypes(source, url, false); + // If the moduleWrap was successfully created, execute the module job. + // outside the try-catch block to avoid catching runtime errors. + moduleWrap = loader.createModuleWrap(strippedSource, url); + // Emit the experimental warning after the code was successfully compiled. + emitExperimentalWarning('Type Stripping'); + } catch (tsError) { + // If its not an error, or it's not an invalid typescript syntax error, rethrow it. + if (!isError(tsError) || tsError?.code !== 'ERR_INVALID_TYPESCRIPT_SYNTAX') { + throw tsError; + } + try { + originalError.stack = `${tsError.message}\n\n${originalError.stack}`; + } catch { /* Ignore potential errors coming from `stack` getter/setter */ } + + throw originalError; + } + } + // If the moduleWrap was successfully created either with by just compiling + // or after transpilation, execute the module job. + return loader.executeModuleJob(url, moduleWrap, true); + }, + ); +}; + +/** + * + * Function used to shortcut when `--input-type=module-typescript` is set. + * @param {string} source + * @param {boolean} print + */ +function parseAndEvalModuleTypeScript(source, print) { + // We know its a TypeScript module, we can safely emit the experimental warning. + const strippedSource = stripTypeScriptModuleTypes(source, getEvalModuleUrl()); + evalModuleEntryPoint(strippedSource, print); +} + +/** + * Function used to shortcut when `--input-type=commonjs-typescript` is set + * @param {string} name The name of the file + * @param {string} source The source code to evaluate + * @param {boolean} breakFirstLine Whether to break on the first line + * @param {boolean} print If the result should be printed + * @param {boolean} shouldLoadESM If the code should be loaded as an ESM module + * @returns {void} + */ +function parseAndEvalCommonjsTypeScript(name, source, breakFirstLine, print, shouldLoadESM = false) { + // We know its a TypeScript module, we can safely emit the experimental warning. + const strippedSource = stripTypeScriptModuleTypes(source, getEvalModuleUrl()); + evalScript(name, strippedSource, breakFirstLine, print, shouldLoadESM); +} + +/** + * + * @param {string} name - The filename of the script. + * @param {string} body - The code of the script. + * @param {string} baseUrl Path of the parent importing the module. + * @returns {ContextifyScript} The created contextify script. + */ +function compileScript(name, body, baseUrl) { + const hostDefinedOptionId = Symbol(name); + async function importModuleDynamically(specifier, _, importAttributes) { + const cascadedLoader = require('internal/modules/esm/loader').getOrInitializeCascadedLoader(); + return cascadedLoader.import(specifier, baseUrl, importAttributes); + } + return makeContextifyScript( + body, // code + name, // filename, + 0, // lineOffset + 0, // columnOffset, + undefined, // cachedData + false, // produceCachedData + undefined, // parsingContext + hostDefinedOptionId, // hostDefinedOptionId + importModuleDynamically, // importModuleDynamically + ); +} + +/** + * @param {string} name - The filename of the script. + * @param {string} body - The code of the script. + * @returns {boolean} Whether the module entry point should be evaluated as a module. + */ +function shouldUseModuleEntryPoint(name, body) { + return getOptionValue('--experimental-detect-module') && + getOptionValue('--input-type') === '' && + containsModuleSyntax(body, name, null, 'no CJS variables'); +} + +/** + * + * @param {string} name - The filename of the script. + * @returns {import('internal/modules/esm/loader').CJSModule} The created module. + */ +function createModule(name) { + const CJSModule = require('internal/modules/cjs/loader').Module; + const cwd = tryGetCwd(); + const module = new CJSModule(name); + module.filename = path.join(cwd, name); + module.paths = CJSModule._nodeModulePaths(cwd); + return module; +} + +/** + * + * @param {string} name - The filename of the script. + * @param {string} body - The code of the script. + * @param {boolean} breakFirstLine Whether to break on the first line + * @param {boolean} print If the result should be printed + * @param {import('internal/modules/esm/loader').CJSModule} module The module + * @param {string} baseUrl Path of the parent importing the module. + * @param {object} compiledScript The compiled script. + * @param {any} origModule The original module. + * @returns {void} + */ +function runScriptInContext(name, body, breakFirstLine, print, module, baseUrl, compiledScript, origModule) { + // Create wrapper for cache entry + const script = ` + globalThis.module = module; + globalThis.exports = exports; + globalThis.__dirname = __dirname; + globalThis.require = require; + return (main) => main(); + `; + globalThis.__filename = name; + RegExpPrototypeExec(/^/, ''); // Necessary to reset RegExp statics before user code runs. + const result = module._compile(script, `${name}-wrapper`)(() => { + // If the script was already compiled, use it. + return runScriptInThisContext( + compiledScript, + true, !!breakFirstLine); + }); + if (print) { + const { log } = require('internal/console/global'); + + process.on('exit', () => { + log(result); + }); + } + if (origModule !== undefined) + globalThis.module = origModule; +} + module.exports = { + parseAndEvalCommonjsTypeScript, + parseAndEvalModuleTypeScript, readStdin, tryGetCwd, evalModuleEntryPoint, + evalTypeScript, evalScript, onGlobalUncaughtException: createOnGlobalUncaughtException(), setUncaughtExceptionCaptureCallback, diff --git a/src/node_options.cc b/src/node_options.cc index 96212d8fbee1c5..e9a6143e1ba6e4 100644 --- a/src/node_options.cc +++ b/src/node_options.cc @@ -108,8 +108,12 @@ void PerIsolateOptions::CheckOptions(std::vector* errors, void EnvironmentOptions::CheckOptions(std::vector* errors, std::vector* argv) { if (!input_type.empty()) { - if (input_type != "commonjs" && input_type != "module") { - errors->push_back("--input-type must be \"module\" or \"commonjs\""); + if (input_type != "commonjs" && input_type != "module" && + input_type != "commonjs-typescript" && + input_type != "module-typescript") { + errors->push_back( + "--input-type must be \"module\"," + "\"commonjs\", \"module-typescript\" or \"commonjs-typescript\""); } } diff --git a/test/es-module/test-typescript-eval.mjs b/test/es-module/test-typescript-eval.mjs index e6d841ffa07f7e..64e334e001beac 100644 --- a/test/es-module/test-typescript-eval.mjs +++ b/test/es-module/test-typescript-eval.mjs @@ -1,5 +1,5 @@ import { skip, spawnPromisified } from '../common/index.mjs'; -import { match, strictEqual } from 'node:assert'; +import { doesNotMatch, match, strictEqual } from 'node:assert'; import { test } from 'node:test'; if (!process.config.variables.node_use_amaro) skip('Requires Amaro'); @@ -20,7 +20,7 @@ test('eval TypeScript ESM syntax', async () => { test('eval TypeScript ESM syntax with input-type module', async () => { const result = await spawnPromisified(process.execPath, [ '--experimental-strip-types', - '--input-type=module', + '--input-type=module-typescript', '--eval', `import util from 'node:util' const text: string = 'Hello, TypeScript!' @@ -37,17 +37,16 @@ test('eval TypeScript CommonJS syntax', async () => { '--eval', `const util = require('node:util'); const text: string = 'Hello, TypeScript!' - console.log(util.styleText('red', text));`, - '--no-warnings']); + console.log(util.styleText('red', text));`]); match(result.stdout, /Hello, TypeScript!/); - strictEqual(result.stderr, ''); + match(result.stderr, /ExperimentalWarning: Type Stripping is an experimental/); strictEqual(result.code, 0); }); -test('eval TypeScript CommonJS syntax with input-type commonjs', async () => { +test('eval TypeScript CommonJS syntax with input-type commonjs-typescript', async () => { const result = await spawnPromisified(process.execPath, [ '--experimental-strip-types', - '--input-type=commonjs', + '--input-type=commonjs-typescript', '--eval', `const util = require('node:util'); const text: string = 'Hello, TypeScript!' @@ -84,10 +83,10 @@ test('TypeScript ESM syntax not specified', async () => { strictEqual(result.code, 0); }); -test('expect fail eval TypeScript CommonJS syntax with input-type module', async () => { +test('expect fail eval TypeScript CommonJS syntax with input-type module-typescript', async () => { const result = await spawnPromisified(process.execPath, [ '--experimental-strip-types', - '--input-type=module', + '--input-type=module-typescript', '--eval', `const util = require('node:util'); const text: string = 'Hello, TypeScript!' @@ -98,10 +97,10 @@ test('expect fail eval TypeScript CommonJS syntax with input-type module', async strictEqual(result.code, 1); }); -test('expect fail eval TypeScript ESM syntax with input-type commonjs', async () => { +test('expect fail eval TypeScript ESM syntax with input-type commonjs-typescript', async () => { const result = await spawnPromisified(process.execPath, [ '--experimental-strip-types', - '--input-type=commonjs', + '--input-type=commonjs-typescript', '--eval', `import util from 'node:util' const text: string = 'Hello, TypeScript!' @@ -117,6 +116,128 @@ test('check syntax error is thrown when passing invalid syntax', async () => { '--eval', 'enum Foo { A, B, C }']); strictEqual(result.stdout, ''); + match(result.stderr, /SyntaxError/); + doesNotMatch(result.stderr, /ERR_INVALID_TYPESCRIPT_SYNTAX/); + strictEqual(result.code, 1); +}); + +test('check syntax error is thrown when passing invalid syntax with --input-type=module-typescript', async () => { + const result = await spawnPromisified(process.execPath, [ + '--experimental-strip-types', + '--input-type=module-typescript', + '--eval', + 'enum Foo { A, B, C }']); + strictEqual(result.stdout, ''); + match(result.stderr, /ERR_INVALID_TYPESCRIPT_SYNTAX/); + strictEqual(result.code, 1); +}); + +test('check syntax error is thrown when passing invalid syntax with --input-type=commonjs-typescript', async () => { + const result = await spawnPromisified(process.execPath, [ + '--experimental-strip-types', + '--input-type=commonjs-typescript', + '--eval', + 'enum Foo { A, B, C }']); + strictEqual(result.stdout, ''); match(result.stderr, /ERR_INVALID_TYPESCRIPT_SYNTAX/); strictEqual(result.code, 1); }); + +test('should not parse TypeScript with --type-module=commonjs', async () => { + const result = await spawnPromisified(process.execPath, [ + '--experimental-strip-types', + '--input-type=commonjs', + '--eval', + `enum Foo {}`]); + + strictEqual(result.stdout, ''); + match(result.stderr, /SyntaxError/); + doesNotMatch(result.stderr, /ERR_INVALID_TYPESCRIPT_SYNTAX/); + strictEqual(result.code, 1); +}); + +test('should not parse TypeScript with --type-module=module', async () => { + const result = await spawnPromisified(process.execPath, [ + '--experimental-strip-types', + '--input-type=module', + '--eval', + `enum Foo {}`]); + + strictEqual(result.stdout, ''); + match(result.stderr, /SyntaxError/); + doesNotMatch(result.stderr, /ERR_INVALID_TYPESCRIPT_SYNTAX/); + strictEqual(result.code, 1); +}); + +test('check warning is emitted when eval TypeScript CommonJS syntax', async () => { + const result = await spawnPromisified(process.execPath, [ + '--experimental-strip-types', + '--eval', + `const util = require('node:util'); + const text: string = 'Hello, TypeScript!' + console.log(util.styleText('red', text));`]); + match(result.stderr, /ExperimentalWarning: Type Stripping is an experimental/); + match(result.stdout, /Hello, TypeScript!/); + strictEqual(result.code, 0); +}); + +test('code is throwing a non Error is rethrown', async () => { + const result = await spawnPromisified(process.execPath, [ + '--experimental-strip-types', + '--eval', + `throw null;`]); + doesNotMatch(result.stderr, /node:internal\/process\/execution/); + strictEqual(result.stdout, ''); + strictEqual(result.code, 1); +}); + +test('code is throwing an error with customized accessors', async () => { + const result = await spawnPromisified(process.execPath, [ + '--experimental-strip-types', + '--eval', + `throw Object.defineProperty(new Error, "stack", { set() {throw this} });`]); + + match(result.stderr, /Error/); + match(result.stderr, /at \[eval\]:1:29/); + strictEqual(result.stdout, ''); + strictEqual(result.code, 1); +}); + +test('typescript code is throwing an error', async () => { + const result = await spawnPromisified(process.execPath, [ + '--experimental-strip-types', + '--eval', + `const foo: string = 'Hello, TypeScript!'; throw new Error(foo);`]); + + match(result.stderr, /Hello, TypeScript!/); + strictEqual(result.stdout, ''); + strictEqual(result.code, 1); +}); + +test('typescript ESM code is throwing a syntax error at runtime', async () => { + const result = await spawnPromisified(process.execPath, [ + '--experimental-strip-types', + '--eval', + 'import util from "node:util"; function foo(){}; throw new SyntaxError(foo(1));']); + // Trick by passing ambiguous syntax to see if evaluated in TypeScript or JavaScript + // If evaluated in JavaScript `foo(1)` is evaluated as `foo < Number > (1)` + // result in false + // If evaluated in TypeScript `foo(1)` is evaluated as `foo(1)` + match(result.stderr, /SyntaxError: false/); + strictEqual(result.stdout, ''); + strictEqual(result.code, 1); +}); + +test('typescript CJS code is throwing a syntax error at runtime', async () => { + const result = await spawnPromisified(process.execPath, [ + '--experimental-strip-types', + '--eval', + 'const util = require("node:util"); function foo(){}; throw new SyntaxError(foo(1));']); + // Trick by passing ambiguous syntax to see if evaluated in TypeScript or JavaScript + // If evaluated in JavaScript `foo(1)` is evaluated as `foo < Number > (1)` + // result in false + // If evaluated in TypeScript `foo(1)` is evaluated as `foo(1)` + match(result.stderr, /SyntaxError: false/); + strictEqual(result.stdout, ''); + strictEqual(result.code, 1); +}); diff --git a/test/fixtures/eval/eval_messages.snapshot b/test/fixtures/eval/eval_messages.snapshot index 6a37ad22634617..4b4069219f03fb 100644 --- a/test/fixtures/eval/eval_messages.snapshot +++ b/test/fixtures/eval/eval_messages.snapshot @@ -11,6 +11,7 @@ SyntaxError: Strict mode code may not include a with statement + Node.js * 42 42 From 079cee060933212b6d6871f85d4a89b43eb2f4a2 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Tue, 24 Dec 2024 20:19:12 +0100 Subject: [PATCH 28/61] test: skip `test-sqlite-extensions` when SQLite is not built by us MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-URL: https://github.com/nodejs/node/pull/56341 Reviewed-By: Luigi Pinca Reviewed-By: Michaël Zasso --- test/sqlite/test-sqlite-extensions.mjs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/sqlite/test-sqlite-extensions.mjs b/test/sqlite/test-sqlite-extensions.mjs index 0e0acf2dc33d30..141f9c9627002c 100644 --- a/test/sqlite/test-sqlite-extensions.mjs +++ b/test/sqlite/test-sqlite-extensions.mjs @@ -7,6 +7,10 @@ import test from 'node:test'; import fs from 'node:fs'; import childProcess from 'child_process'; +if (process.config.variables.node_shared_sqlite) { + common.skip('Missing libsqlite_extension binary'); +} + // Lib extension binary is named differently on different platforms function resolveBuiltBinary(binary) { const targetFile = fs.readdirSync(path.dirname(process.execPath)).find((file) => file.startsWith(binary)); From 3943986e884bae344f835668aaea00ca043ed4ea Mon Sep 17 00:00:00 2001 From: Kevin Toshihiro Uehara Date: Tue, 24 Dec 2024 17:53:53 -0300 Subject: [PATCH 29/61] doc: fix the `crc32` documentation PR-URL: https://github.com/nodejs/node/pull/55898 Fixes: https://github.com/nodejs/node/issues/55800 Reviewed-By: Luigi Pinca Reviewed-By: Antoine du Hamel --- doc/api/zlib.md | 122 ++++++++++++++++++++++++------------------------ 1 file changed, 61 insertions(+), 61 deletions(-) diff --git a/doc/api/zlib.md b/doc/api/zlib.md index a175ded844d5c0..a12e9c3fc680f0 100644 --- a/doc/api/zlib.md +++ b/doc/api/zlib.md @@ -910,7 +910,67 @@ The `zlib.bytesWritten` property specifies the number of bytes written to the engine, before the bytes are processed (compressed or decompressed, as appropriate for the derived class). -### `zlib.crc32(data[, value])` +### `zlib.close([callback])` + + + +* `callback` {Function} + +Close the underlying handle. + +### `zlib.flush([kind, ]callback)` + + + +* `kind` **Default:** `zlib.constants.Z_FULL_FLUSH` for zlib-based streams, + `zlib.constants.BROTLI_OPERATION_FLUSH` for Brotli-based streams. +* `callback` {Function} + +Flush pending data. Don't call this frivolously, premature flushes negatively +impact the effectiveness of the compression algorithm. + +Calling this only flushes data from the internal `zlib` state, and does not +perform flushing of any kind on the streams level. Rather, it behaves like a +normal call to `.write()`, i.e. it will be queued up behind other pending +writes and will only produce output when data is being read from the stream. + +### `zlib.params(level, strategy, callback)` + + + +* `level` {integer} +* `strategy` {integer} +* `callback` {Function} + +This function is only available for zlib-based streams, i.e. not Brotli. + +Dynamically update the compression level and compression strategy. +Only applicable to deflate algorithm. + +### `zlib.reset()` + + + +Reset the compressor/decompressor to factory defaults. Only applicable to +the inflate and deflate algorithms. + +## `zlib.constants` + + + +Provides an object enumerating Zlib-related constants. + +## `zlib.crc32(data[, value])` - -* `callback` {Function} - -Close the underlying handle. - -### `zlib.flush([kind, ]callback)` - - - -* `kind` **Default:** `zlib.constants.Z_FULL_FLUSH` for zlib-based streams, - `zlib.constants.BROTLI_OPERATION_FLUSH` for Brotli-based streams. -* `callback` {Function} - -Flush pending data. Don't call this frivolously, premature flushes negatively -impact the effectiveness of the compression algorithm. - -Calling this only flushes data from the internal `zlib` state, and does not -perform flushing of any kind on the streams level. Rather, it behaves like a -normal call to `.write()`, i.e. it will be queued up behind other pending -writes and will only produce output when data is being read from the stream. - -### `zlib.params(level, strategy, callback)` - - - -* `level` {integer} -* `strategy` {integer} -* `callback` {Function} - -This function is only available for zlib-based streams, i.e. not Brotli. - -Dynamically update the compression level and compression strategy. -Only applicable to deflate algorithm. - -### `zlib.reset()` - - - -Reset the compressor/decompressor to factory defaults. Only applicable to -the inflate and deflate algorithms. - -## `zlib.constants` - - - -Provides an object enumerating Zlib-related constants. - ## `zlib.createBrotliCompress([options])` - -> Stability: 1.1 - Active development - -Enable experimental type-stripping for TypeScript files. -For more information, see the [TypeScript type-stripping][] documentation. - ### `--experimental-test-coverage` + +> Stability: 1.1 - Active development + +Disable experimental type-stripping for TypeScript files. +For more information, see the [TypeScript type-stripping][] documentation. + ### `--no-experimental-websocket` -> Stability: 1 - Experimental - ```c napi_status NAPI_CDECL node_api_create_buffer_from_arraybuffer(napi_env env, napi_value arraybuffer, @@ -2965,10 +2964,9 @@ The JavaScript `string` type is described in added: - v20.4.0 - v18.18.0 +napiVersion: 10 --> -> Stability: 1 - Experimental - ```c napi_status node_api_create_external_string_latin1(napi_env env, @@ -3045,10 +3043,9 @@ The JavaScript `string` type is described in added: - v20.4.0 - v18.18.0 +napiVersion: 10 --> -> Stability: 1 - Experimental - ```c napi_status node_api_create_external_string_utf16(napi_env env, @@ -3140,10 +3137,9 @@ creation methods. added: - v22.9.0 - v20.18.0 +napiVersion: 10 --> -> Stability: 1 - Experimental - ```c napi_status NAPI_CDECL node_api_create_property_key_latin1(napi_env env, const char* str, @@ -3175,10 +3171,9 @@ The JavaScript `string` type is described in added: - v21.7.0 - v20.12.0 +napiVersion: 10 --> -> Stability: 1 - Experimental - ```c napi_status NAPI_CDECL node_api_create_property_key_utf16(napi_env env, const char16_t* str, @@ -3208,10 +3203,9 @@ The JavaScript `string` type is described in added: - v22.9.0 - v20.18.0 +napiVersion: 10 --> -> Stability: 1 - Experimental - ```c napi_status NAPI_CDECL node_api_create_property_key_utf8(napi_env env, const char* str, @@ -6531,7 +6525,7 @@ napi_create_threadsafe_function(napi_env env, **Change History:** -* Experimental (`NAPI_EXPERIMENTAL` is defined): +* Version 10 (`NAPI_VERSION` is defined as `10` or higher): Uncaught exceptions thrown in `call_js_cb` are handled with the [`'uncaughtException'`][] event, instead of being ignored. diff --git a/doc/contributing/releases-node-api.md b/doc/contributing/releases-node-api.md index 088900f88c91e0..f8bcd273e39e70 100644 --- a/doc/contributing/releases-node-api.md +++ b/doc/contributing/releases-node-api.md @@ -85,7 +85,7 @@ with: ```bash grep \ - -E \ + -nHE \ 'N(ODE_)?API_EXPERIMENTAL' \ src/js_native_api{_types,}.h \ src/node_api{_types,}.h @@ -95,13 +95,13 @@ and update the define version guards with the release version: ```diff - #ifdef NAPI_EXPERIMENTAL -+ #if NAPI_VERSION >= 10 ++ #if NAPI_VERSION >= 11 NAPI_EXTERN napi_status NAPI_CDECL node_api_function(napi_env env); - #endif // NAPI_EXPERIMENTAL -+ #endif // NAPI_VERSION >= 10 ++ #endif // NAPI_VERSION >= 11 ``` Remove any feature flags of the form `NODE_API_EXPERIMENTAL_HAS_`. @@ -121,11 +121,11 @@ Also, update the Node-API version value of the `napi_get_version` test in #### Step 2. Update runtime version guards If this release includes runtime behavior version guards, the relevant commits -should already include `NAPI_VERSION_EXPERIMENTAL` guard for the change. Check -for these guards with: +should already include the `NAPI_VERSION_EXPERIMENTAL` guard for the change. +Check for these guards with: ```bash -grep NAPI_VERSION_EXPERIMENTAL src/js_native_api_v8* src/node_api.cc +grep -nH NAPI_VERSION_EXPERIMENTAL src/js_native_api_v8* src/node_api.cc ``` and substitute this guard version with the release version `x`. @@ -138,7 +138,7 @@ Check for these definitions with: ```bash grep \ - -E \ + -nHE \ 'N(ODE_)?API_EXPERIMENTAL' \ test/node-api/*/{*.{h,c},binding.gyp} \ test/js-native-api/*/{*.{h,c},binding.gyp} @@ -170,7 +170,7 @@ stability banner: - > Stability: 1 - Experimental @@ -186,7 +186,7 @@ For all runtime version guards updated in Step 2, check for these definitions with: ```bash -grep NAPI_EXPERIMENTAL doc/api/n-api.md +grep -nH NAPI_EXPERIMENTAL doc/api/n-api.md ``` In `doc/api/n-api.md`, update the `experimental` change history item to be the diff --git a/src/js_native_api.h b/src/js_native_api.h index 07e3df13407030..8ef079b5158249 100644 --- a/src/js_native_api.h +++ b/src/js_native_api.h @@ -92,8 +92,7 @@ NAPI_EXTERN napi_status NAPI_CDECL napi_create_string_utf16(napi_env env, const char16_t* str, size_t length, napi_value* result); -#ifdef NAPI_EXPERIMENTAL -#define NODE_API_EXPERIMENTAL_HAS_EXTERNAL_STRINGS +#if NAPI_VERSION >= 10 NAPI_EXTERN napi_status NAPI_CDECL node_api_create_external_string_latin1( napi_env env, char* str, @@ -110,17 +109,14 @@ node_api_create_external_string_utf16(napi_env env, void* finalize_hint, napi_value* result, bool* copied); -#endif // NAPI_EXPERIMENTAL -#ifdef NAPI_EXPERIMENTAL -#define NODE_API_EXPERIMENTAL_HAS_PROPERTY_KEYS NAPI_EXTERN napi_status NAPI_CDECL node_api_create_property_key_latin1( napi_env env, const char* str, size_t length, napi_value* result); NAPI_EXTERN napi_status NAPI_CDECL node_api_create_property_key_utf8( napi_env env, const char* str, size_t length, napi_value* result); NAPI_EXTERN napi_status NAPI_CDECL node_api_create_property_key_utf16( napi_env env, const char16_t* str, size_t length, napi_value* result); -#endif // NAPI_EXPERIMENTAL +#endif // NAPI_VERSION >= 10 NAPI_EXTERN napi_status NAPI_CDECL napi_create_symbol(napi_env env, napi_value description, diff --git a/src/js_native_api_v8.cc b/src/js_native_api_v8.cc index 3159cd7f69b6f4..6e1680a74e2124 100644 --- a/src/js_native_api_v8.cc +++ b/src/js_native_api_v8.cc @@ -2753,7 +2753,7 @@ napi_status NAPI_CDECL napi_create_reference(napi_env env, CHECK_ARG(env, result); v8::Local v8_value = v8impl::V8LocalValueFromJsValue(value); - if (env->module_api_version != NAPI_VERSION_EXPERIMENTAL) { + if (env->module_api_version < 10) { if (!(v8_value->IsObject() || v8_value->IsFunction() || v8_value->IsSymbol())) { return napi_set_last_error(env, napi_invalid_arg); diff --git a/src/js_native_api_v8.h b/src/js_native_api_v8.h index 99bb30cfbe9a9d..27aeac589b19cd 100644 --- a/src/js_native_api_v8.h +++ b/src/js_native_api_v8.h @@ -234,11 +234,11 @@ inline napi_status napi_set_last_error(node_api_basic_env basic_env, CHECK_ENV_NOT_IN_GC((env)); \ RETURN_STATUS_IF_FALSE( \ (env), (env)->last_exception.IsEmpty(), napi_pending_exception); \ - RETURN_STATUS_IF_FALSE((env), \ - (env)->can_call_into_js(), \ - (env->module_api_version == NAPI_VERSION_EXPERIMENTAL \ - ? napi_cannot_run_js \ - : napi_pending_exception)); \ + RETURN_STATUS_IF_FALSE( \ + (env), \ + (env)->can_call_into_js(), \ + (env->module_api_version >= 10 ? napi_cannot_run_js \ + : napi_pending_exception)); \ napi_clear_last_error((env)); \ v8impl::TryCatch try_catch((env)) diff --git a/src/node_api.cc b/src/node_api.cc index cccb2fd0a17f3a..1638d096969826 100644 --- a/src/node_api.cc +++ b/src/node_api.cc @@ -93,11 +93,11 @@ void node_napi_env__::CallbackIntoModule(T&& call) { return; } node::Environment* node_env = env->node_env(); - // If the module api version is less than NAPI_VERSION_EXPERIMENTAL, - // and the option --force-node-api-uncaught-exceptions-policy is not - // specified, emit a warning about the uncaught exception instead of - // triggering uncaught exception event. - if (env->module_api_version < NAPI_VERSION_EXPERIMENTAL && + // If the module api version is less than 10, and the option + // --force-node-api-uncaught-exceptions-policy is not specified, emit a + // warning about the uncaught exception instead of triggering the uncaught + // exception event. + if (env->module_api_version < 10 && !node_env->options()->force_node_api_uncaught_exceptions_policy && !enforceUncaughtExceptionPolicy) { ProcessEmitDeprecationWarning( @@ -678,11 +678,13 @@ node::addon_context_register_func get_node_api_context_register_func( const char* module_name, int32_t module_api_version) { static_assert( - NODE_API_SUPPORTED_VERSION_MAX == 9, + NODE_API_SUPPORTED_VERSION_MAX == 10, "New version of Node-API requires adding another else-if statement below " "for the new version and updating this assert condition."); if (module_api_version == 9) { return node_api_context_register_func<9>; + } else if (module_api_version == 10) { + return node_api_context_register_func<10>; } else if (module_api_version == NAPI_VERSION_EXPERIMENTAL) { return node_api_context_register_func; } else if (module_api_version >= NODE_API_SUPPORTED_VERSION_MIN && diff --git a/src/node_api.h b/src/node_api.h index c8c7bc6ffb9b94..35e5c3e49dd426 100644 --- a/src/node_api.h +++ b/src/node_api.h @@ -133,8 +133,7 @@ napi_create_external_buffer(napi_env env, napi_value* result); #endif // NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED -#ifdef NAPI_EXPERIMENTAL -#define NODE_API_EXPERIMENTAL_HAS_CREATE_BUFFER_FROM_ARRAYBUFFER +#if NAPI_VERSION >= 10 NAPI_EXTERN napi_status NAPI_CDECL node_api_create_buffer_from_arraybuffer(napi_env env, @@ -142,7 +141,7 @@ node_api_create_buffer_from_arraybuffer(napi_env env, size_t byte_offset, size_t byte_length, napi_value* result); -#endif // NAPI_EXPERIMENTAL +#endif // NAPI_VERSION >= 10 NAPI_EXTERN napi_status NAPI_CDECL napi_create_buffer_copy(napi_env env, size_t length, diff --git a/src/node_version.h b/src/node_version.h index 00acbd732dd7a4..bde0534a02172d 100644 --- a/src/node_version.h +++ b/src/node_version.h @@ -100,7 +100,7 @@ // The NAPI_VERSION supported by the runtime. This is the inclusive range of // versions which the Node.js binary being built supports. -#define NODE_API_SUPPORTED_VERSION_MAX 9 +#define NODE_API_SUPPORTED_VERSION_MAX 10 #define NODE_API_SUPPORTED_VERSION_MIN 1 // Node API modules use NAPI_VERSION 8 by default if it is not explicitly diff --git a/test/js-native-api/test_cannot_run_js/binding.gyp b/test/js-native-api/test_cannot_run_js/binding.gyp index 0b827ff34d129f..dfaaf408296d1d 100644 --- a/test/js-native-api/test_cannot_run_js/binding.gyp +++ b/test/js-native-api/test_cannot_run_js/binding.gyp @@ -5,14 +5,14 @@ "sources": [ "test_cannot_run_js.c" ], - "defines": [ "NAPI_EXPERIMENTAL" ], + "defines": [ "NAPI_VERSION=10" ], }, { "target_name": "test_pending_exception", "sources": [ "test_cannot_run_js.c" ], - "defines": [ "NAPI_VERSION=8" ], + "defines": [ "NAPI_VERSION=9" ], } ] } diff --git a/test/js-native-api/test_cannot_run_js/test_cannot_run_js.c b/test/js-native-api/test_cannot_run_js/test_cannot_run_js.c index 9a4b9547493505..dddb8b59421419 100644 --- a/test/js-native-api/test_cannot_run_js/test_cannot_run_js.c +++ b/test/js-native-api/test_cannot_run_js/test_cannot_run_js.c @@ -22,7 +22,7 @@ static void Finalize(napi_env env, void* data, void* hint) { // napi_pending_exception is returned). This is not deterministic from // the point of view of the addon. -#ifdef NAPI_EXPERIMENTAL +#if NAPI_VERSION > 9 NODE_API_BASIC_ASSERT_RETURN_VOID( result == napi_cannot_run_js || result == napi_ok, "getting named property from global in finalizer should succeed " @@ -32,19 +32,10 @@ static void Finalize(napi_env env, void* data, void* hint) { result == napi_pending_exception || result == napi_ok, "getting named property from global in finalizer should succeed " "or return napi_pending_exception"); -#endif // NAPI_EXPERIMENTAL +#endif // NAPI_VERSION > 9 free(ref); } -static void BasicFinalize(node_api_basic_env env, void* data, void* hint) { -#ifdef NAPI_EXPERIMENTAL - NODE_API_BASIC_CALL_RETURN_VOID( - env, node_api_post_finalizer(env, Finalize, data, hint)); -#else - Finalize(env, data, hint); -#endif -} - static napi_value CreateRef(napi_env env, napi_callback_info info) { size_t argc = 1; napi_value cb; @@ -55,8 +46,7 @@ static napi_value CreateRef(napi_env env, napi_callback_info info) { NODE_API_CALL(env, napi_typeof(env, cb, &value_type)); NODE_API_ASSERT( env, value_type == napi_function, "argument must be function"); - NODE_API_CALL(env, - napi_add_finalizer(env, cb, ref, BasicFinalize, NULL, ref)); + NODE_API_CALL(env, napi_add_finalizer(env, cb, ref, Finalize, NULL, ref)); return cb; } diff --git a/test/js-native-api/test_general/test.js b/test/js-native-api/test_general/test.js index 3d4f2f9715678e..843c6aee3af47f 100644 --- a/test/js-native-api/test_general/test.js +++ b/test/js-native-api/test_general/test.js @@ -34,7 +34,7 @@ assert.notStrictEqual(test_general.testGetPrototype(baseObject), test_general.testGetPrototype(extendedObject)); // Test version management functions -assert.strictEqual(test_general.testGetVersion(), 9); +assert.strictEqual(test_general.testGetVersion(), 10); [ 123, diff --git a/test/js-native-api/test_string/binding.gyp b/test/js-native-api/test_string/binding.gyp index 7fc4d9c24226d4..82a1185c3d9d76 100644 --- a/test/js-native-api/test_string/binding.gyp +++ b/test/js-native-api/test_string/binding.gyp @@ -7,7 +7,7 @@ "test_null.c", ], "defines": [ - "NAPI_EXPERIMENTAL", + "NAPI_VERSION=10", ], }, ], diff --git a/test/node-api/test_buffer/binding.gyp b/test/node-api/test_buffer/binding.gyp index 2fd28280d404c4..0a1dc92de7ffb4 100644 --- a/test/node-api/test_buffer/binding.gyp +++ b/test/node-api/test_buffer/binding.gyp @@ -3,7 +3,7 @@ { "target_name": "test_buffer", "defines": [ - 'NAPI_EXPERIMENTAL' + 'NAPI_VERSION=10' ], "sources": [ "test_buffer.c" ] }, diff --git a/test/node-api/test_reference_by_node_api_version/binding.gyp b/test/node-api/test_reference_by_node_api_version/binding.gyp index 2ee1d24763b0b3..4987828ffb3d86 100644 --- a/test/node-api/test_reference_by_node_api_version/binding.gyp +++ b/test/node-api/test_reference_by_node_api_version/binding.gyp @@ -3,12 +3,12 @@ { "target_name": "test_reference_all_types", "sources": [ "test_reference_by_node_api_version.c" ], - "defines": [ "NAPI_EXPERIMENTAL" ], + "defines": [ "NAPI_VERSION=10" ], }, { "target_name": "test_reference_obj_only", "sources": [ "test_reference_by_node_api_version.c" ], - "defines": [ "NAPI_VERSION=8" ], + "defines": [ "NAPI_VERSION=9" ], } ] } From 8dc39e5e2ea76180e54b3ac3f4c6b3f6614b2d11 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Sat, 28 Dec 2024 16:06:52 -0800 Subject: [PATCH 56/61] process: add process.ref() and process.unref() methods The `process.ref(...)` and `process.unref(...)` methods are intended to replace the use of `ref()` and `unref()` methods defined directly on individual API objects. The existing `ref()` and `unref()` methods will be marked as legacy and won't be removed but new APIs should use `process.ref()` and `process.unref()` instead. Refs: https://github.com/nodejs/node/issues/53266 PR-URL: https://github.com/nodejs/node/pull/56400 Reviewed-By: Matteo Collina Reviewed-By: Yagiz Nizipli Reviewed-By: Chemi Atlow --- doc/api/process.md | 34 ++++++++++++++ lib/internal/bootstrap/node.js | 2 + lib/internal/process/per_thread.js | 14 ++++++ test/parallel/test-process-ref-unref.js | 60 +++++++++++++++++++++++++ 4 files changed, 110 insertions(+) create mode 100644 test/parallel/test-process-ref-unref.js diff --git a/doc/api/process.md b/doc/api/process.md index 540d41b92489cc..c3309eed72a82a 100644 --- a/doc/api/process.md +++ b/doc/api/process.md @@ -3228,6 +3228,23 @@ const { ppid } = require('node:process'); console.log(`The parent process is pid ${ppid}`); ``` +## `process.ref(maybeRefable)` + + + +* `maybeRefable` {any} An object that may be "refable". + +An object is "refable" if it implements the Node.js "Refable protocol". +Specifically, this means that the object implements the `Symbol.for('node:ref')` +and `Symbol.for('node:unref')` methods. "Ref'd" objects will keep the Node.js +event loop alive, while "unref'd" objects will not. Historically, this was +implemented by using `ref()` and `unref()` methods directly on the objects. +This pattern, however, is being deprecated in favor of the "Refable protocol" +in order to better support Web Platform API types whose APIs cannot be modified +to add `ref()` and `unref()` methods but still need to support that behavior. + ## `process.release` + +* `maybeUnfefable` {any} An object that may be "unref'd". + +An object is "unrefable" if it implements the Node.js "Refable protocol". +Specifically, this means that the object implements the `Symbol.for('node:ref')` +and `Symbol.for('node:unref')` methods. "Ref'd" objects will keep the Node.js +event loop alive, while "unref'd" objects will not. Historically, this was +implemented by using `ref()` and `unref()` methods directly on the objects. +This pattern, however, is being deprecated in favor of the "Refable protocol" +in order to better support Web Platform API types whose APIs cannot be modified +to add `ref()` and `unref()` methods but still need to support that behavior. + ## `process.uptime()` > Stability: 1.0 - Early development @@ -1654,7 +1654,7 @@ Disable the experimental [`node:sqlite`][] module. @@ -2343,7 +2343,7 @@ finished executing even if the event loop would otherwise remain active. * `maybeRefable` {any} An object that may be "refable". @@ -4288,7 +4288,7 @@ In [`Worker`][] threads, `process.umask(mask)` will throw an exception. ## `process.unref(maybeRefable)` * `maybeUnfefable` {any} An object that may be "unref'd". diff --git a/doc/api/typescript.md b/doc/api/typescript.md index 925324a65f0174..6551c8f484058b 100644 --- a/doc/api/typescript.md +++ b/doc/api/typescript.md @@ -2,7 +2,7 @@