Feat/ws sync handlers #2028
Feat/ws sync handlers #2028
Conversation
Switch jest.fn to vi.fn and destructure request and requestId.
Can achieve same result by simply returning early, then use the as-previously-defined handling for the general case.
Add a dummy response to trigger the unhandled response in the collection of handlers for the non-remote setupServer-produced Server. This allows an unhandledRequest to exist, despite the remoteHandler always being triggered if added.
src/node/SetupServerCommonApi.ts
Outdated
| remoteResolutionUrl.pathname = '/socket.io' | ||
| const matcher = new RegExp(`${remoteResolutionUrl.href}/.*`) | ||
| const isRequestForSocket = matcher.test(request.url) | ||
| // Bypass handling if the request is for the socket.io connection | ||
| if (isRequestForSocket) { | ||
| return | ||
| } |
There was a problem hiding this comment.
The pathname here depends on socket-io's default value, because MSW doesn't set the value in opts.path (see setupRemoteServer.ts, line 210, second arg).
If you preferred, then that location could be changed to set the path, and then this wouldn't be dependent on their potential changes to default values.
|
I'm not sure if retargeting this to main rather than feat/ws-sync-handlers will clear up a lot of the commits (most are from merging v2.2.0). |
…ext.js (at least in one case)
…e port This fixes a test that was broken by the previous commit, which checked the behaviour of listHandlers().
This prevents repeatedly triggering disconnects and incorrectly resetting handlers
This is handled instead by the passthrough now.
|
I'm not quite sure how to fix the CI checks here - it seems to be complaining about indentation in a file that I guess was changed in I tried resaving the file (which gave two extra spaces via prettier) but that didn't change the outcome of the action check as far as I can tell. |
chrisb2244
changed the base branch from
main
to
run-test-variants-in-parallel
chrisb2244
changed the base branch from
run-test-variants-in-parallel
to
main
|
#2041 is a much cleaner (although arguably more fake) set of commits to the same destination, and may be easier to review/merge if desired. |
Possible implementation for the feat/ws-sync-handlers branch.
A new dummyResponse is sent by the remote handler when the request is not handled by the remote handler - this allows the calling code (via
executeHandlers) to identify that the request was not handled (even though it has been '(not-)handled' by the remote handler).It's very possible I used the wrong commit tags/types, so am happy to rewrite them as required.
The remotePort is set to a static value here - guidance on how you'd prefer this to be set would be welcome (and I can rebase as needed before merging) - however, I wasn't sure the current intention because the
remoteflag mentioned in part of the upstream draft PR seems to have changed to theremotePortvalue, and its existence is used to determine remoteness (so defaulting seems like it would cause problems).If the intent is for the user to specify a port (static or potentially via environment variable, but within their code) then I think leaving a static value in the tests is perhaps(?) acceptable.
I'll add a comment to part of the changed code with further questions/thoughts on an alternative in case it isn't what/where you wanted changes.