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

Allow MSW to intercept connect-node client HTTP-requests #1324

Conversation

mykhailo-hrynko
Copy link

MSW library can't patch node:http, https and http2 modules if they are imported with * as or named import syntax (mswjs/msw#2049)

To allow mocking connect-node HTTP requests, we can use default imports.

So this PR replaces:

import * as http from "http";
import * as https from "https";
import * as http2 from "http2";

with

import http from "node:http";
import https from "node:https";
import http2 from "node:http2";

…, https and http2

Signed-off-by: Misha Hrynko <mykhailo.hrynko@solidgate.com>
@mykhailo-hrynko mykhailo-hrynko force-pushed the connect-node/support-msw-mock branch from a5cd1c9 to c47e10f Compare November 6, 2024 11:27
@mykhailo-hrynko mykhailo-hrynko changed the title fix [connect-node]: allow MSW to intercept http transports Allow MSW to intercept connect-node client HTTP-requests Nov 6, 2024
@timostamm
Copy link
Member

As you can see in the CI logs, for example here, this will fail compilation:

src/http2-session-manager.spec.ts(16,8): error TS1192: Module '"node:http2"' has no default export.

We would need to enable esModuleInterop to be able to build this import statement. While that is probably mostly harmless, making this change would result in very brittle support for MSW. There is nothing preventing us from accidentally adding an import start statement back a couple of commit later. It's perfectly valid syntax, after all. So this change will also need tests or linters to ensure that we stick to default imports.

To be frank, this seems like quite a lot of things to move around to work around a limitation in MSW. And ultimately, it's questionable whether it provides actual value. Let me comment on #1325 with more context.

@mykhailo-hrynko
Copy link
Author

@timostamm
Sorry! My bad! Did not run checks locally.

Checking if it's fixable

@mykhailo-hrynko mykhailo-hrynko deleted the connect-node/support-msw-mock branch November 6, 2024 14:02
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.

2 participants