feat: add `ignoreCommonAssetRequests` option #14

grxy · 2025-07-22T17:46:07Z

Summary

This PR introduces a new ignoreCommonAssetRequests option. When enabled, common asset requests are passed through and not intercepted by any provided MSW handlers.

Details

Added the ignoreCommonAssetRequests boolean option to createNetworkFixture and NetworkFixture.
When enabled, asset requests identified by isCommonAssetRequest are allowed to pass through without interception.
Added and updated tests in tests/requests.test.ts to verify both interception and pass-through behavior for asset requests.

Motivation

This feature allows users to avoid intercepting common asset requests during testing, which can be expensive when one is using a vite development server.

Closes #13

grxy · 2025-07-22T17:47:29Z

src/index.ts

    async ({ page }, use) => {
      const worker = new NetworkFixture({
        page,
+        ignoreCommonAssetRequests: args?.ignoreCommonAssetRequests ?? false,


I opted to make this an option and default it to false so as not to introduce a breaking change.

I suggest drop this and make it true by default since you're introducing a breaking change anyway (feat:).

I updated to make the option true in 5db5185. Are you suggesting that we remove the option altogether? I'm hesitant to do that unless we implement the logic you suggested that only calls for each passed in MSW handler, since it would prevent mocking assets altogether in that case.

grxy · 2025-07-22T17:50:27Z

tests/requests.test.ts

+    network: createNetworkFixture({ ignoreCommonAssetRequests: true }),
+  })
+
+  testIgnoreAssets('passes through an asset request when interceptCommonAssetRequests is true', async ({ network, page }) => {


One downside of enabling this behavior is that asset requests can no longer be mocked at all. Is that acceptable? Maybe there's a way we can modify the API of network.use to allow overriding default fixture options as well as passing handlers?

kettanaito · 2025-07-24T16:32:37Z

I'm wondering, if having a permissive page.route() is causing performance issues, would it make sense to instead create page.route for every handler instead? Thinking out loud here. So nothing explicitly defined in your handlers even gets intercepted.

What do you think, @grxy?

kettanaito · 2025-07-24T16:34:46Z

src/index.ts

        body: request.postDataBuffer(),
      })

+      if (this.#ignoreCommonAssetRequests && isCommonAssetRequest(fetchRequest)) {


There's one thing I still don't quite understand. By ignoring requests here we are claiming that the performance degradation originates from the getResponse() logic below. But that is highly unlikely (unless you have millions of handlers, which isn't the case).

I believe we need to profile this problem better before merging a solution.

Is page.route(/.+/, () => route.continue()) slow for the same scenario?

Is there anything in your scenario that suggests getResponse() becoming the root cause?

Is page.route(/.+/, () => route.continue()) slow for the same scenario?

I think this would be slower at scale (e.g. thousands of requests) due to unnecessary overhead, but not meaningful for fewer total requests.

Is there anything in your scenario that suggests getResponse() becoming the root cause?

getResponse seems to be a contributing factor, but is not necessarily the only cause. I think this really is a scaling problem, on the order of # of requests X # of handlers X request overhead. If a test is making lots of requests (in my case thousands of vite requests), a few ms of overhead for each of these adds up quickly.

I did some not-very-scientific profiling, and here are the results I am seeing:

With ignoreCommonAssetRequests === false:

Setup (http://localhost:5173/) took 6.999666999999931ms Setup (http://localhost:5173/@vite/client) took 4.065000000000055ms Setup (http://localhost:5173/node_modules/.pnpm/[email protected]_@[email protected][email protected]/node_modules/vite/dist/client/env.mjs) took 2.3236670000000004ms

With ignoreCommonAssetRequests === true:

Setup (http://localhost:5173/) took 4.779042000000004ms Setup (http://localhost:5173/@vite/client) took 0.9514169999999922ms Setup (http://localhost:5173/node_modules/.pnpm/[email protected]_@[email protected][email protected]/node_modules/vite/dist/client/env.mjs) took 0.771541999999954ms

Setup here represents the cost of executing the code before route.continue() is eventually called. This was done with 25 placeholder MSW handlers.

My conclusions/suspicions are as follows:

More handlers === more potential overhead for each request

Intercepted, but then continued requests are the most expensive

Something about vite request matching is expensive...maybe the URLs are expensive to match in the underlying getResponses code?

Hundreds to thousands of these requests kicking off at once might be a source of resource contention

kettanaito

We need to discuss this in more detail and profile this issue.

grxy

I'm wondering, if having a permissive page.route() is causing performance issues, would it make sense to instead create page.route for every handler instead? Thinking out loud here. So nothing explicitly defined in your handlers even gets intercepted.

I think that could be an option, and would make the plugin closer to ordinary playwright behavior, but it could be considered a divergence from MSW itself, which intercepts all the things by default. There would still be overhead in this case...but in the playwright matching logic rather than this plugin. Also, there could be potential issues translating MSW route matching logic to Playwright (I'm not sure if they are 1:1 today) so that could be a source of issues in the future. In the end, I think it probably comes down to a "product" decision, so it's up to you how you want the implementation to be architected.

grxy · 2025-07-31T01:12:40Z

src/index.ts

        body: request.postDataBuffer(),
      })

+      if (this.#ignoreCommonAssetRequests && isCommonAssetRequest(fetchRequest)) {


Is page.route(/.+/, () => route.continue()) slow for the same scenario?

I think this would be slower at scale (e.g. thousands of requests) due to unnecessary overhead, but not meaningful for fewer total requests.

Is there anything in your scenario that suggests getResponse() becoming the root cause?

getResponse seems to be a contributing factor, but is not necessarily the only cause. I think this really is a scaling problem, on the order of # of requests X # of handlers X request overhead. If a test is making lots of requests (in my case thousands of vite requests), a few ms of overhead for each of these adds up quickly.

I did some not-very-scientific profiling, and here are the results I am seeing:

With ignoreCommonAssetRequests === false:

Setup (http://localhost:5173/) took 6.999666999999931ms Setup (http://localhost:5173/@vite/client) took 4.065000000000055ms Setup (http://localhost:5173/node_modules/.pnpm/[email protected]_@[email protected][email protected]/node_modules/vite/dist/client/env.mjs) took 2.3236670000000004ms

With ignoreCommonAssetRequests === true:

Setup (http://localhost:5173/) took 4.779042000000004ms Setup (http://localhost:5173/@vite/client) took 0.9514169999999922ms Setup (http://localhost:5173/node_modules/.pnpm/[email protected]_@[email protected][email protected]/node_modules/vite/dist/client/env.mjs) took 0.771541999999954ms

Setup here represents the cost of executing the code before route.continue() is eventually called. This was done with 25 placeholder MSW handlers.

My conclusions/suspicions are as follows:

More handlers === more potential overhead for each request

Intercepted, but then continued requests are the most expensive

Something about vite request matching is expensive...maybe the URLs are expensive to match in the underlying getResponses code?

Hundreds to thousands of these requests kicking off at once might be a source of resource contention

…eedback

grxy · 2025-08-22T17:00:35Z

@kettanaito Would like to get this back on your radar if you are available to review again. Thanks for taking a look!

feat: add ignoreCommonAssetRequests option

5841b62

grxy marked this pull request as ready for review July 22, 2025 17:46

grxy commented Jul 22, 2025

View reviewed changes

kettanaito reviewed Jul 24, 2025

View reviewed changes

kettanaito requested changes Jul 24, 2025

View reviewed changes

grxy commented Jul 31, 2025

View reviewed changes

chore: update interceptCommonAssetRequests default value based on f…

5db5185

…eedback

grxy requested a review from kettanaito July 31, 2025 16:42

Uh oh!

Uh oh!

feat: add ignoreCommonAssetRequests option #14

Are you sure you want to change the base?

feat: add ignoreCommonAssetRequests option #14

Uh oh!

Conversation

grxy commented Jul 22, 2025

Summary

Details

Motivation

Uh oh!

grxy Jul 22, 2025

Choose a reason for hiding this comment

Uh oh!

kettanaito Jul 24, 2025

Choose a reason for hiding this comment

Uh oh!

grxy Jul 31, 2025

Choose a reason for hiding this comment

Uh oh!

grxy Jul 22, 2025

Choose a reason for hiding this comment

Uh oh!

kettanaito commented Jul 24, 2025

Uh oh!

kettanaito Jul 24, 2025

Choose a reason for hiding this comment

Uh oh!

grxy Jul 31, 2025

Choose a reason for hiding this comment

Uh oh!

kettanaito left a comment

Choose a reason for hiding this comment

Uh oh!

grxy left a comment

Choose a reason for hiding this comment

Uh oh!

grxy Jul 31, 2025

Choose a reason for hiding this comment

Uh oh!

grxy commented Aug 22, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

feat: add `ignoreCommonAssetRequests` option #14

feat: add `ignoreCommonAssetRequests` option #14