-
-
Notifications
You must be signed in to change notification settings - Fork 11
feat!: rewrite package to new network source architecture #48
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
christoph-fricke
wants to merge
23
commits into
mswjs:main
Choose a base branch
from
christoph-fricke:feat/network-source-api
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 4 commits
Commits
Show all changes
23 commits
Select commit
Hold shift + click to select a range
f83c8ec
feat!: update msw peer-dependency to ^2.13.2
christoph-fricke ba95269
feat: scaffold `PlaywrightSource` as the new network source
christoph-fricke 65d32b3
feat: add basic `PlaywrightHttpNetworkFrame` implementation
christoph-fricke f9b2c6e
test: make first tests pass with network source architecture
christoph-fricke dc880cd
chore: remove resolved todo comment
christoph-fricke ee93708
test: migrate all tests to network source api
christoph-fricke 4340e11
refactor: remove override keyword usage
christoph-fricke e21c712
feat: add empty websocket frame
christoph-fricke e1402a1
feat: implement websocket frame based on `WebSocketNetworkFrame`
christoph-fricke c6cf6ca
feat: infer baseUrl from intercepted request
christoph-fricke 36d598c
feat: accept either `Page` or `BrowserContext`
christoph-fricke 6ff9100
feat: infer baseUrl for intercepted WebSocket connections
christoph-fricke 2e3c505
test: bypass unhandled frames in tests
christoph-fricke bb06227
refactor: rename utils to route-utils
christoph-fricke 4e2f76f
refactor: capsulate route handled error workaround in utils
christoph-fricke 3238938
refactor: unify route handler registration
christoph-fricke 7da7eee
chore: organize imports
christoph-fricke a616f61
feat: add support for custom route patterns
christoph-fricke f087d52
test: deduplicate internal route registration tests
christoph-fricke d5e06cd
chore: remove open todo comments
christoph-fricke 3f60c5a
feat: use network source api in defineNetworkFixture
christoph-fricke d5f6804
refactor: remove quit override from network frames
christoph-fricke a1ba6c3
refactor: always provide inferred base url to frame resolution
christoph-fricke File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| import type { Route } from '@playwright/test' | ||
| import { HttpNetworkFrame } from 'msw/experimental' | ||
| import { fulfillResponse, handleRouteSafely } from '../utils.js' | ||
|
|
||
| interface PlaywrightHttpNetworkFrameOptions { | ||
| request: Request | ||
| id?: string | ||
| route: Route | ||
| } | ||
|
|
||
| export class PlaywrightHttpNetworkFrame extends HttpNetworkFrame { | ||
| #route: Route | ||
| constructor(options: PlaywrightHttpNetworkFrameOptions) { | ||
| super(options) | ||
| this.#route = options.route | ||
| } | ||
|
|
||
| override async respondWith(response?: Response): Promise<void> { | ||
| if (!response) return | ||
|
|
||
| if (response.status === 0) { | ||
| return await handleRouteSafely(() => this.#route.abort()) | ||
| } | ||
|
|
||
| return await handleRouteSafely(() => fulfillResponse(this.#route, response)) | ||
| } | ||
|
|
||
| override passthrough(): Promise<void> { | ||
| return handleRouteSafely(() => this.#route.fallback()) | ||
| } | ||
|
|
||
| override errorWith(reason?: unknown): Promise<void> { | ||
| if (reason instanceof Response) { | ||
| return handleRouteSafely(() => fulfillResponse(this.#route, reason)) | ||
| } | ||
|
|
||
| return handleRouteSafely(() => this.#route.abort()) | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,55 @@ | ||
| import type { BrowserContext, Route } from '@playwright/test' | ||
| import { isCommonAssetRequest } from 'msw' | ||
| import { NetworkSource } from 'msw/experimental' | ||
| import { PlaywrightHttpNetworkFrame } from './frames/http-frame.js' | ||
| import { | ||
| convertToRequest, | ||
| handleRouteSafely, | ||
| INTERNAL_MATCH_ALL_REG_EXP, | ||
| } from './utils.js' | ||
|
|
||
| interface PlaywrightSourceOptions { | ||
| context: BrowserContext | ||
| skipAssetRequests?: boolean | ||
| } | ||
|
|
||
| export class PlaywrightSource extends NetworkSource<PlaywrightHttpNetworkFrame> { | ||
| #context: BrowserContext | ||
| #skipAssetRequests: boolean | ||
|
|
||
| constructor(options: PlaywrightSourceOptions) { | ||
| super() | ||
| this.#context = options.context | ||
| this.#skipAssetRequests = options.skipAssetRequests ?? true | ||
| } | ||
|
|
||
| override async enable(): Promise<void> { | ||
| await this.#context.route( | ||
| INTERNAL_MATCH_ALL_REG_EXP, | ||
| this.#handleRouteRequest.bind(this), | ||
| ) | ||
| } | ||
|
|
||
| override async disable(): Promise<void> { | ||
| super.disable() | ||
| await this.#context.unroute(INTERNAL_MATCH_ALL_REG_EXP) | ||
| } | ||
|
|
||
| async #handleRouteRequest(route: Route): Promise<void> { | ||
| const request = await convertToRequest(route) | ||
|
|
||
| /** | ||
| * @note Skip common asset requests (default). | ||
| * Playwright seems to experience performance degradation when routing all | ||
| * requests through the matching logic below. | ||
| * @see https://github.com/mswjs/playwright/issues/13 | ||
| */ | ||
| if (this.#skipAssetRequests && isCommonAssetRequest(request)) { | ||
| return await handleRouteSafely(() => route.fallback()) | ||
| } | ||
|
|
||
| // TODO: Do we need a id for the frame? Do we need to store the frame in a map like Interceptor- and ServiceWorkerSource? | ||
|
christoph-fricke marked this conversation as resolved.
Outdated
|
||
| const frame = new PlaywrightHttpNetworkFrame({ route, request }) | ||
| await this.queue(frame) | ||
|
kettanaito marked this conversation as resolved.
|
||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| import type { Route } from '@playwright/test' | ||
|
|
||
| /** | ||
| * @note Use a match-all RegExp with an optional group as the predicate | ||
| * for the `page.route()`/`page.unroute()` calls. Playwright treats given RegExp | ||
| * as the handler ID, which allows us to remove only those handlers introduces by us | ||
| * without carrying the reference to the handler function around. | ||
| */ | ||
| export const INTERNAL_MATCH_ALL_REG_EXP = /.+(__MSW_PLAYWRIGHT_PREDICATE__)?/ | ||
|
|
||
| export async function convertToRequest(route: Route): Promise<Request> { | ||
| const request = route.request() | ||
| return new Request(request.url(), { | ||
| method: request.method(), | ||
| headers: new Headers(await request.allHeaders()), | ||
| // TODO: Can we get rid of the type cast? | ||
| body: request.postDataBuffer() as null | ArrayBuffer, | ||
| }) | ||
| } | ||
|
|
||
| export async function fulfillResponse( | ||
| route: Route, | ||
| response: Response, | ||
| ): Promise<void> { | ||
| await route.fulfill({ | ||
| status: response.status, | ||
| headers: Object.fromEntries(response.headers), | ||
| body: response.body ? Buffer.from(await response.arrayBuffer()) : undefined, | ||
| }) | ||
| } | ||
|
|
||
| export async function handleRouteSafely( | ||
| callback: () => Promise<void>, | ||
| ): Promise<void> { | ||
| try { | ||
| await callback() | ||
| } catch (error) { | ||
| /** | ||
| * @note Ignore "Route is already handled!" errors. | ||
| * Playwright has a bug where requests terminated due to navigation | ||
| * cause your in-flight route handlers to throw. There's no means to | ||
| * detect that scenario as both "route.handled" and "route._handlingPromise" are internal. | ||
| * @see https://github.com/mswjs/playwright/issues/35 | ||
| */ | ||
| if ( | ||
| error instanceof Error && | ||
| /route is already handled/i.test(error.message) | ||
| ) { | ||
| return | ||
| } | ||
|
|
||
| throw error | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💭 With support for custom route patterns,
skipAssetRequestsmight be obsolete.I added support for custom patterns, because in my experience most mock setups mock API(s) behind one (a few at most) endpoints. Alongside registering multiple
PlaywrightSourcesources, custom patterns can help reduce the interception overhead for this common case. What do you think about this?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, if we go with the route of mapping handlers to
page.route(), then I suppose there's no need inskipAssetRequestsanymore. Just need to make sure the tests pass.