Skip to content
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

Draft: Add tests for node native fetch interceptor (WIP) #5

Open
wants to merge 8 commits into
base: chore/migrate-to-node-v18
Choose a base branch
from

Conversation

milesrichardson
Copy link

depends on #3

This is a WIP and is a follow-up to mswjs#283

So far it basically copies the existing fetch.ts tests but without a polyfill, without https support, and without any of the body/clone/request test variants.

milesrichardson added a commit to splitgraph/madatdata that referenced this pull request Aug 31, 2022
- Basically, `msw` can mostly work with node-native `fetch` powered
by undici, but it does not currently create its lone export `createServer`
with the `FetchInterceptor` that is necessary to intercept `globalFetch`
- Clone the library locally, fix this issue, build it locally, and copy the build
files into this Yarn patch

Upstream, I've started the chain of PRs necessary to fix this:

- mswjs/interceptors#283 (migrate to Node 18)
- milesforks/msw-interceptors#5 (wip, add native fetch tests)
- Last: one line change to msw that I made in our patch here
@milesrichardson milesrichardson force-pushed the test/node-native-global-interceptor branch from aab433c to 7f6847d Compare September 28, 2022 23:14
@milesrichardson milesrichardson changed the base branch from chore/migrate-to-node-v18 to main September 28, 2022 23:19
@milesrichardson milesrichardson force-pushed the test/node-native-global-interceptor branch 2 times, most recently from 06f880e to 3b74e5b Compare September 28, 2022 23:29
@milesrichardson milesrichardson changed the base branch from main to chore/migrate-to-node-v18 September 28, 2022 23:29
… defined

Now that Node v17+ has "native fetch", it's possible to have
`globalThis.fetch` but not `location`, both in our own test
code and in user test code.

So, if `location` is not defined, like in `node-native.fetch` integration
tests, then do not try to set it as the base url of a new URL -
the caller is responsible for giving a correct location.
…-native shape

It seems that the `headers` object when testing `fetch`
with `node-native` is nested within `headers._headers`,
but is available with `headers.raw()` - so use that as
the basis of the left side of the comparison when
comparing a header object to the expected one.
…rception

Only `http` tests pass, and this is only the one test file (still
needs `.body`, `.clone` and `.request` variants).

The tests are copied from the general `fetch.test.ts` file,
which is similar because it also runs in a Node environment,
but different because it uses `cross-fetch`, while this new
file uses native `fetch` (note there is no import of a polyfill).
Add `undici` as a development dependency, because we need to
import `undici.Agent` so that we can set a dispatcher that
intercepts insecure https (equivalent of `httpsAgent` with
previous versions of Node).

Technically we could accomplish this without a dependency
on Undici, but it would require re-implementing the entire
`undici.Agent` class, which would be silly.

Note this is a _development_ dependency. It is not a regular
dependency, because we aren't adding any special undici interceptor;
we're only using the `Agent` because it's the only choice we
have (Node does not re-export its undici module; we must depend on it)
…hen it exists

* When no mock response is defined, the interceptor calls `pureFetch`.
* In undici fetch, `RequestInit` accepts an additional property `dispatcher`,
which is used for non-global agent configuration, e.g. an insecure https agent
for a single request.
* If interceptor calls `pureFetch` (because no mock response is defined),
make sure it includes the `dispatcher` if it was originally provided
…6 host

* Don't assume a single `:` indicates an IPv6 address
* Change heuristic for identifying IPv6 address to rely on
whether `new URL()` throws an error

Ideally this serialization wouldn't be necessary, but the
code that constructs the host doesn't have access to the
address family of the server where the address came from
@milesrichardson milesrichardson force-pushed the test/node-native-global-interceptor branch from 3b74e5b to 25a8ffa Compare September 29, 2022 00:41
milesrichardson added a commit to milesforks/msw that referenced this pull request Sep 29, 2022
- Point `@mswjs/interceptors` to a tarball that was built from
the code in this pending PR: milesforks/msw-interceptors#5

Using commit:

milesforks/msw-interceptors@25a8ffa
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant