[experimental] Pass `NetworkFrameResolutionContext` to `NetworkSource.enable()`

[christoph-fricke]

This issue has come up while migrating @msw/playwright to the new network source architecture.

I want to bring this idea forward for discussion to improve the new network source API while it is still exported as "experimental".

Problem Statement

I think network sources can benefit from having access to the NetworkFrameResolutionContext that users provide to defineNetwork. Currently this context is only provided to frames.

In the current implementation, both ServiceWorkerSource and FallbackHttpSource accept their own quit option, which results in network definitions looking like this:

const network = defineNetwork({
  sources: [new FallbackHttpSource({ quiet: true })],
  handlers[],
  context: { quit: true },
})

If they where able access NetworkFrameResolutionContext.quit instead, the duplicate configuration can be removed:

const network = defineNetwork({
  sources: [new FallbackHttpSource()],
  handlers[],
  context: { quit: true },
})

In the @msw/playwright rewrite I could benefit from having access to NetworkFrameResolutionContext.baseUrl in the network source to use the value when calling browserContext.route(...). Handlers only match requests that start with prefix if it is provided. Therefore, browserContext.route(...) also only has to intercept requests that start with the baseUrl (instead of all network traffic) to reduce overhead in Playwright.

Proposed Solution

Since the context can be reconfigured with network.configure, we have find the right time for passing along a stable context to the source. I think passing the value when the network, and thus all sources, get enabled is ideal. I can't imagine a source needing access to the context before it gets enabled.

export abstract class NetworkSource<
  Frame extends AnyNetworkFrame = AnyNetworkFrame,
> {
  public abstract enable(context: NetworkFrameResolutionContext): unknown | Promise<unknown>
}

Furthermore, NetworkFrameResolutionContext has to be exported so custom sources are able to type their enable method. With this change, we should consider renaming NetworkFrameResolutionContext as its scope expands beyond just frame resolution.

[kettanaito]

This is an interesting point. The resolution context is currently passed to the layer that immediately relies on it: frame.resolve(). But you are proposing we expose it to the network source, which can also make sense (like generating page.route() with the correct base URL, like you said).

Since the context can be reconfigured with network.configure, we have find the right time for passing along a stable context to the source.

Reconfigure only works before you call .enable(). You cannot reconfigure a running network so this way .enable() always gets the latest context and doesn't have to care for anything else.

In the @msw/playwright rewrite I could benefit from having access to NetworkFrameResolutionContext.baseUrl in the network source

I think you understood this a bit backwards. The user has no access to affect baseUrl in @msw/playwright. It's the binding that should set baseUrl to page.url() so relative URLs in handlers are resolved against what the user expects—the current page's location.

Does this affect your feature request?

[christoph-fricke]

The user has no access to affect baseUrl in @msw/playwright. It's the binding that should set baseUrl to page.url() so relative URLs in handlers are resolved against what the user expects—the current page's location.

What exactly do you mean with binding? Some concept in the network source architecture, i.e. the source or frame, or a wrapper around the API? That is one of my open questions in mswjs/playwright#48: Should the library continue to expose a defineNetworkFixture wrapper or just the source? My understanding has been that the various wrappers are deprecated and using defineNetwork will be the future of setting up MSW. With defineNetwork users are able to configure baseUrl.

Independent of what the binding is, I agree that @msw/playwright should also infer a baseUrl. page.url() might not always works (as seen in the existing implementation), since a BrowserContext does not equal a single page. But something is possible and it should be possible to provide a inferred baseUrl by overwriting resolve in the custom Http frame:

// I am including this snippet here to highlight the gap in the exported types
import type { RequestHandler } from 'msw'
import type { UnhandledFrameHandle } from '../../node_modules/msw/lib/core/experimental/on-unhandled-frame.mjs'
import type { NetworkFrameResolutionContext } from '../../node_modules/msw/lib/core/experimental/frames/network-frame.mjs'

export class PlaywrightHttpNetworkFrame extends HttpNetworkFrame {
  #route: Route
  #inferredBaseUrl?: string

  constructor(options: PlaywrightHttpNetworkFrameOptions) {
    super(options)
    this.#route = options.route
    this.#inferredBaseUrl = options.inferredBaseUrl
  }

  resolve(
    handlers: Array<RequestHandler>,
    onUnhandledFrame: UnhandledFrameHandle,
    resolutionContext?: NetworkFrameResolutionContext,
  ): Promise<boolean | null> {
    return super.resolve(handlers, onUnhandledFrame, {
      ...resolutionContext,
      baseUrl: resolutionContext?.baseUrl ?? this.#inferredBaseUrl,
      quiet: resolutionContext?.quiet !== false,
    })
  }

Does this affect your feature request?

Thinking more about it, maybe a Playwright source should not always blindly limit the interception to a configured baseUrl. It breaks a mix of absolute URL handlers and relative URL handlers (for which the configured baseUrl applies). Absolute URL handlers with a different would no longer have their requests intercepted if it is limited to the baseUrl for relative Url handlers. In conclusion, if users want to limit the interception scope, it should be a separate option. Many setups will mock an API limited to one baseUrl, so I think being able to limit the interception scope is still a valid improvement - just through a different configuration option.

Nonetheless, I still think that having access to the configured context in network sources is a good improvements. As mentioned, ServiceWorkerSource and FallbackHttpSource are existing sources that can benefit from it.

[kettanaito]

Should the library continue to expose a defineNetworkFixture wrapper or just the source?

Both are currently supported and will be for the foreseeable future. defineNetwork is primarily designed for customization and building experienced from extraneous sources, such as page.route in Playwright. I don't expect developers to directly call defineNetwork and provide it a PlaywrightNetworkSource. Those are and should remain implementation details of @msw/playwright.

But something is possible and it should be possible to provide a inferred baseUrl by overwriting resolve in the custom Http frame:

I don't believe there's a better approach than the last page.url(). It's the intrinsic limitation of Playwright that it doesn't provide the URL of the page that triggered the request intercepted by page.route. We can dance around this, but our solution will never be truly reliable.

You don't have to override resolve. It already supports the baseUrl through the resolution context it has. There's no issue of providing it with the base URL, only what that URL should be (see the preceding paragraph).

Thinking more about it, maybe a Playwright source should not always blindly limit the interception to a configured baseUrl.

I'm not sure we are talking about the same thing. baseUrl is already a part of MSW internally and has no effect on requests with absolute URLs. Perhaps you are misreading what it does? baseUrl is simply a base to resolve relative URLs against before matching them with request handlers.

msw/src/core/utils/url/getAbsoluteUrl.ts

Line 19 in 580256b

const origin = baseUrl || (typeof location !== 'undefined' && location.href)

Given how location.href is the fallback value, maybe we don't even need to provide baseUrl manually in Playwright's case at all? That might've been an over-engineering on my end.

Nonetheless, I still think that having access to the configured context in network sources is a good improvements. As mentioned, ServiceWorkerSource and FallbackHttpSource are existing sources that can benefit from it.

resolutionContext is already exposed to resolve of the frames. I still haven't heard a compelling argument to expose it for the entire network source though. Let me know if you thought about one!

[christoph-fricke]

@kettanaito I think I know where the confusion is coming from. I have been under the impression that the different setupXXXX APIs are deprecated and will be replaced by defineNetwork in the near future. So everyone uses defineNetwork directly and composes network sources with it. Not gonna lie, I like the idea of such future...

I still believe that the proposal makes total sense with this assumption, as it avoids the duplicated quit option highlighted in the problem statement. But I agree that it is not necessary as long as defineNetwork is not intended to be used directly.

---

Perhaps you are misreading what it does? baseUrl is simply a base to resolve relative URLs against before matching them with request handlers.

That is my understanding as well. I have been loudly thinking about a different baseUrl. One used for calling Playwright's page.route(baseUrl). I thought the could be the same, but they cannot be.

Given how location.href is the fallback value, maybe we don't even need to provide baseUrl manually in Playwright's case at all? That might've been an over-engineering on my end.

That will not work because the network source's code is executed in the Node.js process not the browser. At least I don't see how it could work. The baseUrl still has to be extracted on a per-request basis from the request, which can only happen in the network source or frame wrapper.

You don't have to override resolve. It already supports the baseUrl through the resolution context it has. There's no issue of providing it with the base URL, only what that URL should be (see the preceding paragraph).

I don't believe can work. The Playwright wrapper (defineNetworkFixture) cannot extract a baseUrl and pass it along to defineNetwork because this is not happening in the scope of a request. And it cannot simply extract it from page.url() when calling defineNetwork. The fixtures will almost always be evaluated before a test triggers the first page navigation, which results in page.url() --> about:blank and thus an empty baseUrl.

---

Let's move the baseUrl discussion over to mswjs/playwright#48 as it is more relevant there. As for this proposal, I think we might as well ignore/close it.