interceptors icon indicating copy to clipboard operation
interceptors copied to clipboard

ES Modules (node): named import of `request` doesn't get intercepted

Open lionralfs opened this issue 4 years ago • 4 comments

Hey,

I'm using node version 16.6.2 along with ES Modules, which means I import node's native https lib like this:

import { request } from 'https';

request('https://example.com/test', (res) => {})

However, requests are not intercepted when request is imported this way. The same behavior can be observed when doing this:

import * as https from 'https';

https.request('https://example.com/test', (res) => {})

Here's a repo to show the complete code.

lionralfs avatar Jan 17 '22 23:01 lionralfs

Thanks for reporting this, @lionralfs. This indeed looks off. It shouldn't matter how the request function is imported, it references the same function.

Are you sure you're not using node:https?

kettanaito avatar Jan 24 '22 10:01 kettanaito

Not sure what you mean by node:https, but I'm using node's built-in https module. As noted in my repo, requests are intercepted if the entire module is imported, and then used like this:

import https from 'https';

https.request('...')

lionralfs avatar Jan 24 '22 11:01 lionralfs

I was referring to the new way to import Node.js native modules:

import { request } from 'node:https'

That's not the case for you, so my guess looks irrelevant.

I've changed one of our integration tests for https.request to use the named import as you do. Could you take a look at https://github.com/mswjs/interceptors/pull/203 and see if it's different from your use case in any way? Requests are still intercepted in the test.

kettanaito avatar Jan 25 '22 15:01 kettanaito

Hm, that's odd. The only notable difference is the way I import the interceptor:

import { interceptClientRequest } from '@mswjs/interceptors/lib/interceptors/ClientRequest/index.js';

The way you imported it in your tests before (import * as https from 'https') also doesn't work for me: https://github.com/lionralfs/mswjs-interceptors-bug-repro/blob/d93f27be924fec479adbcec06a14043ab7cbbb09/also-fails.js

Also note that I'm using "type": "module" in my package.json which makes node treat my '.js' files as ES Modules: https://github.com/lionralfs/mswjs-interceptors-bug-repro/blob/d93f27be924fec479adbcec06a14043ab7cbbb09/package.json#L2

lionralfs avatar Jan 25 '22 18:01 lionralfs

I think this has to do with what ESM does to named/default exports. What I've learned recently is that these two would yield totally different things in ESM:

import * as http from 'http'
http.request

import { request } from 'http'

Those two request functions won't share their identity because ESM would do some magic with imports, converting them into an object or something (really not an ESM pro here, sorry).

This is important because the Interceptors patch http with a specific import type:

https://github.com/mswjs/interceptors/blob/ddf0389c0a0f2f98350f3857d1e301f806c66587/src/interceptors/ClientRequest/index.ts#L1

I wonder if accounting for named imports would solve this issue:

import http from 'http'
import { request } from 'http'

// do the patching
http.request = patch()
request = patch()

But that will never work because re-assigning an import is a no-op, and if we store the request in a variable, we will be overriding and restoring only that request variable scoped to the interceptor module.

import http from 'http'

const { request } = http

// These really only apply in the scope of this module
// and won't be able to affect your tests.
request = patch()
request = undo()

I don't think this is possible with ESM, to my best knowledge. If you have an idea, please comment on this issue.

kettanaito avatar Nov 19 '22 01:11 kettanaito

Looks like https://github.com/nock/nock/issues/24611 describes the same problem. Doesn't look like anybody has figured out how to monkey patch through named ESMs.

richardm-stripe avatar Jul 18 '23 01:07 richardm-stripe