OK I switched to using multiple entrypoints in the tsup config and this looks more like what we want.

Here is the structure of the outputted files:

The interceptors import from the chunks which are all the shared bits of code

OK I switched to using multiple entrypoints in the tsup config and this looks more like what we want.

Here is the structure of the outputted files:

The interceptors import from the chunks which are all the shared bits of code

Nice! yeah, that's more like what I think we're looking for!

The interceptors import from the chunks which are all the shared bits of code

@cdimitroulas, This is fantastic!

While we're at it, folks, what is your opinion on #52? I think it would be great to be able to import interceptors directly from the root of the package like so:

import { ClientRequestInterecptor } from '@mswjs/interceptors/ClientRequestInterceptor'

This is possible via exports in package.json but I don't know if that will be universally supported by all consumers (CJS/ESM). This can also be done entirely separately from this change, I'm just asking for your opinion on this.

The interceptors import from the chunks which are all the shared bits of code

@cdimitroulas, This is fantastic!

While we're at it, folks, what is your opinion on #52? I think it would be great to be able to import interceptors directly from the root of the package like so:

import { ClientRequestInterecptor } from '@mswjs/interceptors/ClientRequestInterceptor'

This is possible via exports in package.json but I don't know if that will be universally supported by all consumers (CJS/ESM). This can also be done entirely separately from this change, I'm just asking for your opinion on this.

The interceptors import from the chunks which are all the shared bits of code

@cdimitroulas, This is fantastic!

While we're at it, folks, what is your opinion on #52? I think it would be great to be able to import interceptors directly from the root of the package like so:

import { ClientRequestInterecptor } from '@mswjs/interceptors/ClientRequestInterceptor'

This is possible via exports in package.json but I don't know if that will be universally supported by all consumers (CJS/ESM). This can also be done entirely separately from this change, I'm just asking for your opinion on this.

I think this should work everywhere that package.json exports work.

We may need to add fake folders/package.json for older versions of typescript and node to handle, but otherwise i think this is the path we should be on here.

we should add a block like this to the package.json (once we configure both esm and cjs builds from tsup)

"exports": {
    ".": {
        "types": "...path to index.d.ts for root package",
        "require": "...path to cjs root pacakge",
        "default": "...path to esm root package"
    },
    "./ClientRequest": {
       ...same format as above
    },
    "./XMLHttpRequest": {
       ...same format as above
    },
    "./fetch": {
       ...same format as above
    }
}

then we'll probably also need a folder for each of these (empty, just with a package.json)
ClientRequestIntercepot, XMLHTTPRequest and fetch with the main, types, and module fields corresponding to the same data in the exports blocks.

@mattcosta7 I've added the "exports" field to package.json, please take a look and maybe also test out the build locally to see if it matches your expectations.

I think it would be great to be able to import interceptors directly from the root of the package like so:

import { ClientRequestInterecptor } from '@mswjs/interceptors/ClientRequestInterceptor'

I think this should work fine based on the current build output (yarn build:new for now, I'll overwrite yarn build when we're ready to finalize this PR)

@mattcosta7 @kettanaito when you have a moment I would appreciate if we could finalize some of these discussions so I can finalize the code and mark the PR as ready for review

Hey, @cdimitroulas. I was having a couple of busy weeks at work. I've updated all the discussions, summarizing them below:

• Move headers-polyfill to dependencies as it ships both as CJS and ESM so it's safe to depend on it as-is (no need to bundle it).
• Consider moving the exports changes to a separate pull request. I leave this to you and whether you feel comfortable extracting those changes. If not, we can ship it as a part of this change as well, just going to bump a minor version.

I wouldn't want to introduce empty placeholder directories like @mattcosta7 mentions but I think we need that for full compatibility. Do you know if there's a support map for exports in package.json or something? I believe all modern bundlers and the latest Node respects that field but I wonder how far the support goes in terms of previous versions.

Do you know if there's a support map for exports in package.json or something? I believe all modern bundlers and the latest Node respects that field but I wonder how far the support goes in terms of previous versions.

I'm not sure such a thing exists, I did find this helpful SO answer which is relevant:
https://stackoverflow.com/a/74551879/7133623

Not sure about bundler support but I imagine it's as you say and they all support it (at least on more recent versions)

I've added RemoteHttpInterceptor as the entrypoint so it can be imported.

Now, seems like all browser tests are failing because some Node code ends up in lib/index.js:

    BREAKING CHANGE: webpack < 5 used to include polyfills for node.js core modules by default.
    This is no longer the case. Verify if you need this module and configure a polyfill for it.

    If you want to include a polyfill, you need to:
        - add a fallback 'resolve.fallback: { \"http\": require.resolve(\"stream-http\") }'
        - install 'stream-http'
    If you don't want to include a polyfill, you can use an empty module like this:
        resolve.fallback: { \"http\": false }",
        "moduleId": "./lib/chunk-EYK25V6X.js",
        "moduleIdentifier": "/mswjs/interceptors/lib/chunk-EYK25V6X.js",
        "moduleName": "./lib/chunk-EYK25V6X.js",
        "moduleTrace": Array [
          [Object],
          [Object],
        ],
        "stack": "ModuleNotFoundError: Module not found: Error: Can't resolve 'http' in '/mswjs/interceptors/lib'

@cdimitroulas, do you have some suspicion as to why this happens?

No idea, I'll try to figure it out

It's not all the browser tests that are failing, the fetch ones are passing. Only the XMLHttpRequest ones are failing.

It seems that a chunk that is split off by the code-splitting that is used by both the XMLHttpRequest interceptor and the ClientRequest interceptor has this in it:

// node_modules/@remix-run/web-fetch/src/headers.js
var _util = require('util');
var _http = require('http'); var _http2 = _interopRequireDefault(_http);

This suggests that it's somehow related to remix-run/web-fetch :/

It seems remix is configured to automatically deal with this stuff... https://remix.run/docs/en/v1/guides/constraints#server-code-pruning

It's not all the browser tests that are failing, the fetch ones are passing. Only the XMLHttpRequest ones are failing.

It seems that a chunk that is split off by the code-splitting that is used by both the XMLHttpRequest interceptor and the ClientRequest interceptor has this in it:

// node_modules/@remix-run/web-fetch/src/headers.js
var _util = require('util');
var _http = require('http'); var _http2 = _interopRequireDefault(_http);

This suggests that it's somehow related to remix-run/web-fetch :/

OOof hmm. I think since we're bundling this with a node target, we're probably only getting the node imports for remix-run/fetch?

I wonder if we actually need a 'browser' build that's separate from the node build, which could potentially be a separate tsup config? the docs specificy - https://tsup.egoist.dev/#target-environment that target environment is node14 by default? do we need one that targets browsers to get the browser build of that library? (just guessing)

I think the correct flag is esbuild's platform which can be set to node or browser. Unfortunately, trying to build with platform: "browser" just results in the same errors but at build-time instead:

ESM Build start
✘ [ERROR] Could not resolve "http"

    src/interceptors/ClientRequest/index.ts:1:17:
      1 │ import http from 'http'
        ╵                  ~~~~~~

  The package "http" wasn't found on the file system but is built into node. Are you trying to
  bundle for node? You can use "platform: 'node'" to do that, which will remove this error.

✘ [ERROR] Could not resolve "http"

    src/interceptors/ClientRequest/index.ts:1:17:
      1 │ import http from 'http'
        ╵                  ~~~~~~

  The package "http" wasn't found on the file system but is built into node. Are you trying to
  bundle for node? You can use "platform: 'node'" to do that, which will remove this error.

✘ [ERROR] Could not resolve "https"

    src/interceptors/ClientRequest/index.ts:2:18:
      2 │ import https from 'https'
        ╵                   ~~~~~~~

  The package "https" wasn't found on the file system but is built into node. Are you trying to
  bundle for node? You can use "platform: 'node'" to do that, which will remove this error.

...etc.

I'm going to see if there's a way to exclude remix-run/web-fetch in browser builds since theoretically it shouldn't be needed, right? Everything used from there like Response are browser globals

I was able to get the tests to pass, but I'm not sure how the build output should look like when building for node/browser separately.
For now, I just arbitrarily created lib/node/ and lib/browser/ and updated the imports for the tests:

Here's a gist showing the changes I made as I don't feel like this is something I want to commit here:
https://gist.github.com/cdimitroulas/caf930091aa7c63fc696e6521ade465a

It's not all the browser tests that are failing, the fetch ones are passing. Only the XMLHttpRequest ones are failing.

This is a fantastic find.

I wonder if we actually need a 'browser' build that's separate from the node build, which could potentially be a separate tsup config?

This may be the case. We use @remix-run/web-fetch only in Node to polyfill Fetch API primitives. I don't believe that package is ever meant to run in the browser, as it's a Node.js polyfill for Fetch API.

I think we do need to ship Node and browser builds separately, which means that XMLHttpRequest and Fetch interceptors should be compiled to both Node (depends on web-fetch) and browser versions (doesn't need web-fetch, uses browser API).

I believe a separate config is the way here, as we mustn't compile ClientRequest interceptor since it's meant for Node-only execution. That's precisely why those errors happen when you tried setting the platform to "browser", @cdimitroulas.

Something like this should work?

import { defineConfig } from "tsup";

export default defineConfig([
  {
    name: "node",
    entry: [
      "src/index.ts",
      "src/interceptors/ClientRequest",
      "src/interceptors/XMLHttpRequest",
      "src/interceptors/fetch",
    ],
    platform: "node",
    outDir: "./lib",
    format: ["cjs", "esm"],
  },
  {
    name: "browser",
    entry: [
      "src/index.ts",
      "src/interceptors/XMLHttpRequest",
      "src/interceptors/fetch",
    ],
    platform: "browser",
    outDir: "./lib",
    format: ["cjs", "esm"],
  },
]);

The fact that we depend on @remix-run/web-fetch through direct imports may need to change as well. What I used to do in some other projects is this:

// some-module.ts
// Imagine a module (e.g. an interceptor) that imports
// some fetch api utils.
import { Request } from './fetch-api'

// fetch-api.ts
// Define those fetch api utils for Node.
// Here, depend on the polyfill.
export * from '@remix-run/web-fetch'

// fetch-api.browser.ts
// Then, create a browser-side version of these utils,
// reusing the native browser API.
export {
  Request,
  Response, 
  // ...
}

Lastly, instruct the code to import different fetch-api based on the environment where it runs:

{
  "browser": {
    "./lib/fetch-api.js": "./lib/fetch-api.browser.js"
  },
}

There may be a better way of doing this since I'm not a fan of re-exporting browser globals from fetch-api.browser.js. I suppose we can inject @remix-run/web-fetch as a polyfill on build-time and rely on Fetch primitives as if they are globals:

// Reference globalThis.Request which is:
// A. A regular browser global if run in the browser;
// B. The same export from @remix-run/web-fetch in Node.js.
new Request('/foo')

Maybe there are other options you have in mind, please share them.

I suppose we can inject @remix-run/web-fetch as a polyfill on build-time and rely on Fetch primitives as if they are globals:

This sounds better and simpler to me than the alternative you proposed

Unfortunately I don't think the config you proposed will work as both node/browser configs are outputting the same files to the same place (e.g. "lib/interceptors/fetch.js" is an output from both configs).
We'll need something like lib/interceptors/node/fetch.js and lib/interceptors/fetch.js to distinguish them unless I'm missing some trick

@mattcosta7 I reordered the keys in the exports as per your suggestion

@mattcosta7, thanks for the review! And @cdimitroulas, thank you so much for addressing those points so quickly! I've rebased this branch, and solved the package.json conflict. Will continue with testing in MSW, let's see if the order of those fields solved the issue for the IIFE build.

@kettanaito did the latest changes fix the problems you saw when testing in MSW?

Hey, @cdimitroulas. I haven't tested that yet. Planning on returning to open source this/next week, need to arrange a few other things.

The changes to Interceptors look good from what I can gather. The order sensitivity that @mattcosta7 mentioned was a great addition to this, now to see what exactly gets bundled in when tsup builds into IIFE. For some reason, I suspect it doesn't respect the exports field despite the platform being the "browser". I'd expect it to respect that.

The IIFE integration test passes in MSW with these changes to Interceptors linked:

 PASS  test/msw-api/distribution/iife.test.ts
  ✓ supports the usage of the iife bundle in a <script> tag (1169 ms)

🎉

There are, however, failing unit tests because somewhere in Interceptors we rely on TextEncoder, which Jest just bluntly ignores (Jest is notoriously bad strict with ignoring accepted Node.js standard API).

 FAIL  src/utils/logging/serializeRequest.test.ts
  ● Test suite failed to run

    ReferenceError: TextEncoder is not defined

      1 | "use strict";Object.defineProperty(exports, "__esModule", {value: true});// node_modules/web-encoding/src/lib.mjs
    > 2 | var Encoder = TextEncoder;
        |               ^
      3 | var Decoder = TextDecoder;

This happens because web-encoding—the library that we import TextEncoder from—optimistically uses the global TextEncoder API for its Node.js target:

• lib.js is browser-oriented (ambiguous) module.
• lib.mjs is Node-oriented module.

  "main": "./src/lib.js",
  "module": "./src/lib.mjs",
  "browser": "./src/lib.js",
  "types": "./src/lib.d.ts",
  "exports": {
    ".": {
      "import": "./src/lib.mjs",
      "require": "./src/lib.js"
    }
  },

// src/lib.mjs
// In node `export { TextEncoder }` throws:
// "Export 'TextEncoder' is not defined in module"
// To workaround we first define constants and then export with as.
const Encoder = TextEncoder
const Decoder = TextDecoder

export { Encoder as TextEncoder, Decoder as TextDecoder }

Those globals won't be defined in Jest. They are defined in Node.js.

This may have something to do with how we require/bundle web-encoding. It would be nice to force it to use lib.js that is ambiguous and should work in Jest:

// lib.js
"use strict"

exports.TextEncoder =
  typeof TextEncoder !== "undefined" ? TextEncoder : require("util").TextEncoder

exports.TextDecoder =
  typeof TextDecoder !== "undefined" ? TextDecoder : require("util").TextDecoder

This will work because it will use require('util').TextEncoder in Jest, since TextEncoder doesn't exist there.

Solution to web-encoding

I just added a manual shim to Interceptors to provide ambiguous export for environments like Jest.

Issue: Jest doesn't understand the exports field

Looks like Jest doesn't handle exports correctly in its resolver:

 FAIL  test/msw-api/setup-server/life-cycle-events/on.test.ts
  ● Test suite failed to run

    Cannot find module '@mswjs/interceptors/ClientRequest' from '../lib/node/index.js'

    Require stack:
      /Users/kettanaito/Projects/mswjs/msw/lib/node/index.js
      msw-api/setup-server/life-cycle-events/on.test.ts

    > 1 | import { ClientRequestInterceptor } from '@mswjs/interceptors/ClientRequest'
        |                                          ^
      2 | import { XMLHttpRequestInterceptor } from '@mswjs/interceptors/XMLHttpRequest'
      3 | import { RequestHandler } from '../handlers/RequestHandler'
      4 | import { SetupServerApi } from './SetupServerApi'

      at Resolver.resolveModule (../node_modules/jest-resolve/build/index.js:306:11)
      at Object.<anonymous> (../src/node/setupServer.ts:1:42)

• See Support package exports in jest-resolve jestjs/jest#9771

The issue appears to be fixed in Jest@28 (27 too?). But jumping two major versions of Jest foreshadows a traumatic experience. I will try that to prove that MSW tests pass with this Interceptors change.

Jumping from Jest v26 -> v27 doesn't seem too bad https://jestjs.io/blog/2021/05/25/jest-27#features-coming-with-breaking-changes

Jumping from v27 -> v28 requires working through this guide though

I wonder if Jest 27 brings in ESM support... The references I find mention Jest 28 if not 29.

All and all, this looks like an endless loop of testing setup update that I've been postponing for forever. I don't mind doing it in MSW's repo, it just takes time. Everything does.

Working on that mswjs/msw#1394

I wonder if we just used undici in the Fetch API branch, and removed remix-run dependencies that force us to bundle everything as ESM, would that be less changes all around the stack? 🤔

I wonder if we just used undici in the Fetch API branch, and removed remix-run dependencies that force us to bundle everything as ESM, would that be less changes all around the stack? 🤔

offhand, i'm not sure of node support, would we need to lose backward support? (ie, would we only support versions where unidici already exists? and referenceable on globalThis?)

the less we need to bundle, the better I think - so if so, it sounds worth exploring

@mattcosta7,

would we need to lose backward support? (ie, would we only support versions where unidici already exists? and referenceable on globalThis?)

Eventually, yes! That's the plan for this autumn when Node 16 goes to pass and we only have the versions of Node that ship global fetch throughout the board. Until then, we need to resort to some polyfill to support Node 16.

Trying this with the Fetch API branch

✅ Build
✅ Unit tests
❌ Integration tests

 FAIL  test/msw-api/setup-server/scenarios/response-patching.test.ts
  ● Test suite failed to run

    Cannot find module '@mswjs/interceptors/ClientRequest' from '../lib/node/index.js'

    Require stack:
      /mswjs/msw/lib/node/index.js
      msw-api/setup-server/scenarios/response-patching.test.ts

    > 1 | import { ClientRequestInterceptor } from '@mswjs/interceptors/ClientRequest'

All the errors seem to have the same origin: they fail to resolve the "exports" to the interceptors.

This is likely caused by Jest@26 missing support for "exports". But since we provide backward-compatibility with the ./ClientRequest/package.json paths, I'd expect older Jest to work. Hm.

Released: v0.21.0 🎉

This has been released in v0.21.0!

• 📄 Release notes
• 📦 npm package

Make sure to always update to the latest version (npm i @mswjs/interceptors@latest) to get the newest features and bug fixes.

Predictable release automation by @ossjs/release.

Is it just me, or does it appear that this change made the presets unreachable now? This sample code no longer works for importing the browserInterceptors preset:

import { BatchInterceptor } from '@mswjs/interceptors'
import browserInterceptors from '@mswjs/interceptors/lib/presets/browser'

const interceptor = new BatchInterceptor({
  name: 'my-interceptor',
  interceptors: browserInterceptors,
})
interceptor.on('request', listener)

Most probably yes @blaenk - we'll need to add these exports explicitly at the top-level somewhere I think and update the docs to reflect this. Do you have capacity to open a PR on that?

Sorry, I don't have the bandwidth at the moment. For now I pinned to the previous release.