-
Notifications
You must be signed in to change notification settings - Fork 29.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
lib: define FormData and fetch etc. in the built-in snapshot #51598
Conversation
Now that --experimental-fetch is true by default, define the dependent interfaces in the built-in snapshot and only delete them at run time when --no-experimental-fetch is set.
Review requested:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work! This should help improve the performance of global fetch
usage. Thank you 🌟
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Landed in ce56887 |
Now that --experimental-fetch is true by default, define the dependent interfaces in the built-in snapshot and only delete them at run time when --no-experimental-fetch is set. PR-URL: nodejs#51598 Reviewed-By: Ethan Arrowood <ethan@arrowood.dev> Reviewed-By: Matthew Aitken <maitken033380023@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Now that --experimental-fetch is true by default, define the dependent interfaces in the built-in snapshot and only delete them at run time when --no-experimental-fetch is set. PR-URL: #51598 Reviewed-By: Ethan Arrowood <ethan@arrowood.dev> Reviewed-By: Matthew Aitken <maitken033380023@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Now that --experimental-fetch is true by default, define the dependent interfaces in the built-in snapshot and only delete them at run time when --no-experimental-fetch is set. PR-URL: nodejs#51598 Reviewed-By: Ethan Arrowood <ethan@arrowood.dev> Reviewed-By: Matthew Aitken <maitken033380023@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
This was a breaking change for tests mocking global fetch, for ex: import { describe, it } from 'node:test';
import assert from 'node:assert/strict';
import { fetchSomething } from '../lib/index.mjs';
describe('my test suite', () => {
it('fetch stuff', async (t) => {
const mockValue = { key: 'value' };
const mockFetch = async () => ({
json: async () => mockValue,
status: 200,
});
t.mock.method(global, 'fetch', mockFetch);
assert.deepStrictEqual(await fetchSomething(), mockValue);
t.mock.restoreAll();
});
}); which produces: ✖ fetch stuff (0.894292ms)
TypeError [ERR_INVALID_ARG_VALUE]: The argument 'methodName' must be a method. Received undefined
at MockTracker.method (node:internal/test_runner/mock/mock:241:13)
at TestContext.<anonymous> (my.test.mjs:13:12)
at Test.runInAsyncScope (node:async_hooks:206:9)
at Test.run (node:internal/test_runner/test:641:25)
at Suite.processPendingSubtests (node:internal/test_runner/test:382:18)
at Test.postRun (node:internal/test_runner/test:732:19)
at Test.run (node:internal/test_runner/test:690:12)
at async Promise.all (index 0)
at async Suite.run (node:internal/test_runner/test:966:7)
at async startSubtest (node:internal/test_runner/harness:218:3) {
code: 'ERR_INVALID_ARG_VALUE'
} to fix the break, i had to reference fetch;
t.mock.method(global, 'fetch', mockFetch); |
@hulkish please report breaking changes as new issue, thanks. |
Now that --experimental-fetch is true by default, define the dependent interfaces in the built-in snapshot and only delete them at run time when --no-experimental-fetch is set. PR-URL: #51598 Reviewed-By: Ethan Arrowood <ethan@arrowood.dev> Reviewed-By: Matthew Aitken <maitken033380023@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Now that --experimental-fetch is true by default, define the dependent interfaces in the built-in snapshot and only delete them at run time when --no-experimental-fetch is set. PR-URL: #51598 Reviewed-By: Ethan Arrowood <ethan@arrowood.dev> Reviewed-By: Matthew Aitken <maitken033380023@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
* chore: bump node in DEPS to v20.12.0 * chore: update build_add_gn_build_files.patch * chore: update patches * chore: bump node in DEPS to v20.12.1 * chore: update patches * build: encode non-ASCII Latin1 characters as one byte in JS2C nodejs/node#51605 * crypto: use EVP_MD_fetch and cache EVP_MD for hashes nodejs/node#51034 * chore: update filenames.json * chore: bump node in DEPS to v20.12.2 * chore: update patches * src: support configurable snapshot nodejs/node#50453 * test: remove test-domain-error-types flaky designation nodejs/node#51717 * src: avoid draining platform tasks at FreeEnvironment nodejs/node#51290 * chore: fix accidentally deleted v8 dep * lib: define FormData and fetch etc. in the built-in snapshot nodejs/node#51598 * chore: rebase on main * chore: remove stray log --------- Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com> Co-authored-by: Cheng <zcbenz@gmail.com> Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com> Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com>
* chore: bump node in DEPS to v20.13.1 * chore: bump node in DEPS to v20.14.0 * chore: update build_add_gn_build_files.patch * chore: update patches * chore: update patches * build: encode non-ASCII Latin1 characters as one byte in JS2C nodejs/node#51605 * crypto: use EVP_MD_fetch and cache EVP_MD for hashes nodejs/node#51034 * chore: update filenames.json * chore: update patches * src: support configurable snapshot nodejs/node#50453 * test: remove test-domain-error-types flaky designation nodejs/node#51717 * src: avoid draining platform tasks at FreeEnvironment nodejs/node#51290 * chore: fix accidentally deleted v8 dep * lib: define FormData and fetch etc. in the built-in snapshot nodejs/node#51598 * chore: remove stray log * crypto: enable NODE_EXTRA_CA_CERTS with BoringSSL nodejs/node#52217 * test: skip test for dynamically linked OpenSSL nodejs/node#52542 * lib, url: add a `windows` option to path parsing nodejs/node#52509 * src: use dedicated routine to compile function for builtin CJS loader nodejs/node#52016 * test: mark test as flaky nodejs/node#52671 * build,tools: add test-ubsan ci nodejs/node#46297 * src: preload function for Environment nodejs/node#51539 * deps: update c-ares to 1.28.1 nodejs/node#52285 * chore: fixup * events: extract addAbortListener for safe internal use nodejs/node#52081 * module: print location of unsettled top-level await in entry points nodejs/node#51999 * fs: add stacktrace to fs/promises nodejs/node#49849 * chore: fixup indices --------- Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com> Co-authored-by: Cheng <zcbenz@gmail.com> Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com> Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com>
Now that --experimental-fetch is true by default, define the dependent interfaces in the built-in snapshot and only delete them at run time when --no-experimental-fetch is set.