mswjs / interceptors

Low-level network interception library.
https://npm.im/@mswjs/interceptors
MIT License
535 stars 120 forks source link

Error using BatchInterceptor with browserInterceptors in browser environment. #476

Closed steve-zhu-sh closed 9 months ago

steve-zhu-sh commented 9 months ago

Still getting the issue from here: https://github.com/mswjs/interceptors/pull/361#issuecomment-1455223410

Type 'readonly [FetchInterceptor, XMLHttpRequestInterceptor]' is not assignable to type 'readonly Interceptor<any>[]'.
  Type 'FetchInterceptor | XMLHttpRequestInterceptor' is not assignable to type 'Interceptor<any>'.
    Type 'FetchInterceptor' is not assignable to type 'Interceptor<any>'.
      Types have separate declarations of a private property 'symbol'.ts(2322)
import { BatchInterceptor } from '@mswjs/interceptors';
import browserInterceptors from '@mswjs/interceptors/presets/browser';

const interceptor = new BatchInterceptor({
  name: 'interceptor',
  interceptors: browserInterceptors,
});

This looks to be because the exports in package.json for "." does not include a browser config. It should be something like this:


{
 // ...
  "exports": {
    ".": {
      "browser": {
        "types": "./lib/browser/index.d.ts",
        "require": "./lib/browser/index.js",
        "import": "./lib/browser/index.mjs",
        "default": "./lib/browser/index.js"
      },
      "types": "./lib/node/index.d.ts",
      "require": "./lib/node/index.js",
      "import": "./lib/node/index.mjs",
      "default": "./lib/node/index.js"
    },
    // ...
  },
  // ...
}
kettanaito commented 9 months ago

Hey, @steve-zhu-sh. Thanks for reporting this.

This is strange. We do have the browser export for the /presets/browser path:

Can you please make sure you're using the latest version of @mswjs/interceptors? Perhaps a version mismatch issue. If not, a reproduction repository would be the way forward to resolve this. Thanks.

steve-zhu-sh commented 9 months ago

Hi @kettanaito , the /preset/browser path does have the browser export, but the . path also needs a browser export, otherwise the definition for BatchInterceptor is not compatible with browserInterceptors since BatchInterceptor is imported from the . path.

From the error message Types have separate declarations of a private property 'symbol', I'm taking this to mean that the symbol definition for BatchInterceptor is incompatible between the import from ./lib/browser/index.js and ./lib/node/index.js.

steve-zhu-sh commented 9 months ago

Here's the entire exports change needed in package.json for clarity:

  "exports": {
    ".": {
+     "browser": {
+       "types": "./lib/browser/index.d.ts",
+       "require": "./lib/browser/index.js",
+       "import": "./lib/browser/index.mjs",
+       "default": "./lib/browser/index.js"
+     },
      "types": "./lib/node/index.d.ts",
      "require": "./lib/node/index.js",
      "import": "./lib/node/index.mjs",
      "default": "./lib/node/index.js"
    },
    "./ClientRequest": {
      "browser": null,
      "types": "./lib/node/interceptors/ClientRequest/index.d.ts",
      "require": "./lib/node/interceptors/ClientRequest/index.js",
      "import": "./lib/node/interceptors/ClientRequest/index.mjs",
      "default": "./lib/node/interceptors/ClientRequest/index.js"
    },
    "./XMLHttpRequest": {
      "browser": {
        "types": "./lib/browser/interceptors/XMLHttpRequest/index.d.ts",
        "require": "./lib/browser/interceptors/XMLHttpRequest/index.js",
        "import": "./lib/browser/interceptors/XMLHttpRequest/index.mjs",
        "default": "./lib/browser/interceptors/XMLHttpRequest/index.js"
      },
      "types": "./lib/node/interceptors/XMLHttpRequest/index.d.ts",
      "require": "./lib/node/interceptors/XMLHttpRequest/index.js",
      "import": "./lib/node/interceptors/XMLHttpRequest/index.mjs",
      "default": "./lib/node/interceptors/XMLHttpRequest/index.js"
    },
    "./fetch": {
      "browser": {
        "types": "./lib/browser/interceptors/fetch/index.d.ts",
        "require": "./lib/browser/interceptors/fetch/index.js",
        "import": "./lib/browser/interceptors/fetch/index.mjs",
        "default": "./lib/browser/interceptors/fetch/index.js"
      },
      "types": "./lib/node/interceptors/fetch/index.d.ts",
      "require": "./lib/node/interceptors/fetch/index.js",
      "import": "./lib/node/interceptors/fetch/index.mjs",
      "default": "./lib/node/interceptors/fetch/index.js"
    },
    "./RemoteHttpInterceptor": {
      "browser": null,
      "types": "./lib/node/RemoteHttpInterceptor.d.ts",
      "require": "./lib/node/RemoteHttpInterceptor.js",
      "import": "./lib/node/RemoteHttpInterceptor.mjs",
      "default": "./lib/node/RemoteHttpInterceptor.js"
    },
    "./presets/node": {
      "browser": null,
      "types": "./lib/node/presets/node.d.ts",
      "require": "./lib/node/presets/node.js",
      "import": "./lib/node/presets/node.mjs",
      "default": "./lib/node/presets/node.js"
    },
    "./presets/browser": {
      "browser": {
        "types": "./lib/browser/presets/browser.d.ts",
        "require": "./lib/browser/presets/browser.js",
        "import": "./lib/browser/presets/browser.mjs",
        "default": "./lib/browser/presets/browser.js"
      }
    }
  },
steve-zhu-sh commented 9 months ago

Hi @kettanaito, when you have the chance could you take a look at the comments above?

kettanaito commented 9 months ago

Hey, @steve-zhu-sh. Thanks for describing the issue and even proposing a fix!

I'm wondering if the error here isn't in having src/index.ts in the browser build's entries? I'd rather the browser bundle reused the root-level exports as those are meant to be environment-agnostic (if they aren't, that's a bug).

At the same time, I don't have the time to reorganize the entire exports right now so your solution should work as well. Let me give it a try.

kettanaito commented 9 months ago

Released: v0.25.12 🎉

This has been released in v0.25.12!

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.

steve-zhu-sh commented 9 months ago

Hey, @steve-zhu-sh. Thanks for describing the issue and even proposing a fix!

I'm wondering if the error here isn't in having src/index.ts in the browser build's entries? I'd rather the browser bundle reused the root-level exports as those are meant to be environment-agnostic (if they aren't, that's a bug).

At the same time, I don't have the time to reorganize the entire exports right now so your solution should work as well. Let me give it a try.

@kettanaito I think with the build process (and export contract) you've selected, that isn't possible. Even if the root-level exports that you've implemented in TypeScript pre build step are environment-agnostic, since you've decided to support both a node and browser build by splitting the build into to different environments, there could be things that potentially break that environment agnostic implementation. Since browser and node may have different levels of javascript support, as a contrived example, if you build node for es6 support, but the browser for es3, there could be different levels of transpilation required for the two builds even for the shared environment agnostic modules.

I think some other library authors may choose to build targetting the most recent version of ecmascript and let the library consumer handle transpilation. Or at least build for an LTS version of node and let the consumer handle browser transpilation since the user probably needs to handle that anyway for their own code due to varying levels of browser support for the different versions of ecmascript.

tsup config reference:

import { Options, defineConfig } from 'tsup'

const nodeConfig: Options = {
  entry: [
    './src/index.ts',
    './src/presets/node.ts',
    './src/RemoteHttpInterceptor.ts',
    './src/interceptors/ClientRequest/index.ts',
    './src/interceptors/XMLHttpRequest/index.ts',
    './src/interceptors/fetch/index.ts',
  ],
  outDir: './lib/node',
  platform: 'node',
  format: ['cjs', 'esm'],
  dts: true,
}

const browserConfig: Options = {
  entry: [
    './src/index.ts',
    './src/presets/browser.ts',
    './src/interceptors/XMLHttpRequest/index.ts',
    './src/interceptors/fetch/index.ts',
  ],
  outDir: './lib/browser',
  platform: 'browser',
  format: ['cjs', 'esm'],
  dts: true,
}

export default defineConfig([nodeConfig, browserConfig])