connectrpc / connect-es

The TypeScript implementation of Connect: Protobuf RPC that works.
https://connectrpc.com/
Apache License 2.0
1.39k stars 82 forks source link

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

Closed mykhailo-hrynko closed 2 weeks ago

mykhailo-hrynko commented 2 weeks ago

MSW library can't patch node:http, https and http2 modules if they are imported with * as or named import syntax (https://github.com/mswjs/msw/issues/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";
timostamm commented 2 weeks ago

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 https://github.com/connectrpc/connect-es/issues/1325 with more context.

mykhailo-hrynko commented 2 weeks ago

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

Checking if it's fixable