From 3fdd4d3b6f13180d3438a70b3b9a53c7a7a50096 Mon Sep 17 00:00:00 2001 From: YCChen Date: Mon, 18 Mar 2024 16:59:10 -0400 Subject: [PATCH 1/3] lib: refactor lazy loading of undici for fetch method Object.defineProperty is updated to lazily load the undici dependency for the fetch method. This change allows for simpler and more reliable mocking of the fetch method for testing purposes, resolving issues encountered with premature method invocation during testing. Fixes: https://github.com/nodejs/node/issues/52015 --- .../bootstrap/web/exposed-window-or-worker.js | 39 +++++++------------ 1 file changed, 13 insertions(+), 26 deletions(-) diff --git a/lib/internal/bootstrap/web/exposed-window-or-worker.js b/lib/internal/bootstrap/web/exposed-window-or-worker.js index 2459bee85bb8b0..d99ace09c5472b 100644 --- a/lib/internal/bootstrap/web/exposed-window-or-worker.js +++ b/lib/internal/bootstrap/web/exposed-window-or-worker.js @@ -57,32 +57,19 @@ defineReplaceableLazyAttribute(globalThis, 'perf_hooks', ['performance']); const { installObjectURLMethods } = require('internal/url'); installObjectURLMethods(); -{ - // https://fetch.spec.whatwg.org/#fetch-method - function set(value) { - ObjectDefineProperty(globalThis, 'fetch', { - __proto__: null, - writable: true, - value, - }); - } - ObjectDefineProperty(globalThis, 'fetch', { - __proto__: null, - configurable: true, - enumerable: true, - set, - get() { - function fetch(input, init = undefined) { - // Loading undici alone lead to promises which breaks lots of tests so we - // have to load it really lazily for now. - const { fetch: impl } = require('internal/deps/undici/undici'); - return impl(input, init); - } - set(fetch); - return fetch; - }, - }); -} +// https://fetch.spec.whatwg.org/#fetch-method +ObjectDefineProperty(globalThis, 'fetch', { + __proto__: null, + configurable: true, + enumerable: true, + writable: true, + value: function fetch(input, init = undefined) { // eslint-disable-line func-name-matching + // Loading undici alone lead to promises which breaks lots of tests so we + // have to load it really lazily for now. + const { fetch: impl } = require('internal/deps/undici/undici'); + return impl(input, init); + }, +}); // https://xhr.spec.whatwg.org/#interface-formdata // https://fetch.spec.whatwg.org/#headers-class From 5c6a8893db5541d53cb9c05fbc2324fb9ec1e5ca Mon Sep 17 00:00:00 2001 From: YCChen Date: Sun, 7 Apr 2024 12:23:56 +0800 Subject: [PATCH 2/3] lib: implement lazy loading for fetch function Instead of importing undici upfront, the module is now conditionally required using require only when the fetch function is called for the first time and the undici implementation is not already available. This lazy loading approach improves resource usage and test reliability by loading undici only when needed. --- .../bootstrap/web/exposed-window-or-worker.js | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/lib/internal/bootstrap/web/exposed-window-or-worker.js b/lib/internal/bootstrap/web/exposed-window-or-worker.js index d99ace09c5472b..82888b3cd9512e 100644 --- a/lib/internal/bootstrap/web/exposed-window-or-worker.js +++ b/lib/internal/bootstrap/web/exposed-window-or-worker.js @@ -57,17 +57,19 @@ defineReplaceableLazyAttribute(globalThis, 'perf_hooks', ['performance']); const { installObjectURLMethods } = require('internal/url'); installObjectURLMethods(); +let fetchImpl; // https://fetch.spec.whatwg.org/#fetch-method ObjectDefineProperty(globalThis, 'fetch', { __proto__: null, configurable: true, enumerable: true, writable: true, - value: function fetch(input, init = undefined) { // eslint-disable-line func-name-matching - // Loading undici alone lead to promises which breaks lots of tests so we - // have to load it really lazily for now. - const { fetch: impl } = require('internal/deps/undici/undici'); - return impl(input, init); + value: function value(input, init = undefined) { + if (!fetchImpl) { // Implement lazy loading of undici module for fetch function + const undiciModule = require('internal/deps/undici/undici'); + fetchImpl = undiciModule.fetch; + } + return fetchImpl(input, init); }, }); From 7937b695368badd5cc498eec8104508bf910f44c Mon Sep 17 00:00:00 2001 From: YCChen Date: Sun, 7 Apr 2024 20:40:11 +0800 Subject: [PATCH 3/3] lib: test for fetch mock without prior invocation The purpose of this test is to verify the correct behavior of stubbing the globalThis.fetch method with custom implementation. It checks whether the stubbed fetch function correctly returns the expected response without calling fetch iteself first. --- test/parallel/test-fetch-mock.js | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 test/parallel/test-fetch-mock.js diff --git a/test/parallel/test-fetch-mock.js b/test/parallel/test-fetch-mock.js new file mode 100644 index 00000000000000..b457745f1c4c90 --- /dev/null +++ b/test/parallel/test-fetch-mock.js @@ -0,0 +1,20 @@ +'use strict'; +require('../common'); +const { mock, test } = require('node:test'); +const assert = require('node:assert'); + +test('should correctly stub globalThis.fetch', async () => { + const customFetch = async (url) => { + return { + text: async () => 'foo', + }; + }; + + mock.method(globalThis, 'fetch', customFetch); + + const response = await globalThis.fetch('some-url'); + const text = await response.text(); + + assert.strictEqual(text, 'foo'); + mock.restoreAll(); +});