Conversation
🦋 Changeset detectedLatest commit: 2b721a1 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
resure
marked this pull request as ready for review
September 30, 2026 20:41
Author
|
Wow, this "easy"-cla, docu-sign bullshit is beyond me, sorry. Closing this PR, but feel free to grab patch if somebody would need it: https://gist.github.com/resure/bc376397ef0df3669e696d4ea8b76812 |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
The client is injected into workers too. Some setups start a worker from a
blob:URL, for example a small blob that sets the public path and callsimportScriptsso worker code can load from a CDN (@gravity-ui/app-builder does this). In such a workerself.locationhas ablob:protocol and an empty hostname and port, socreateSocketURLdoesn't replace0.0.0.0with the page host and doesn't switch towss:. If the dev server runs plain HTTP behind a TLS proxy, the worker triesws://0.0.0.0/..., and on an HTTPS pagenew WebSocketthrowsSecurityErrorwhile the worker loads. SettingwebSocketURL.protocol: 'wss'only turns that into endless reconnects towss://0.0.0.0. With no protocol in the query, the client builds ablob://...socket URL.A blob worker's
self.location.originis still the origin of the page that created it (checked in Chromium). With this change, ablob:worker with a real origin takes its hostname, protocol and port from that origin. Pages, http(s) workers, an explicitly set hostname or protocol,data:workers and blobs with an opaque ('null') origin work as before.The same fix is proposed for @rspack/dev-server, whose client has the same code: rstackjs/rspack-dev-server#267. There it was tested end to end: with the patch applied, a real app's blob worker on an HTTPS dev stand connects to the page host and receives HMR updates. In this repository only the unit tests cover it.
What kind of change does this PR introduce?
fix
Did you add tests for your changes?
Yes.
createSocketURL.test.jsgets four blob-worker cases: an HTTPS page, an HTTP page, a custom port and an explicit hostname. All four fail without the fix. They go at the end of the table on purpose: the client opens its socket on import using the location left by the previous case, so a blob case in the middle would break the import for the cases after it on unfixed code. No e2e worker test was added.Does this PR introduce a breaking change?
No.
If relevant, what needs to be documented once your changes are merged or what have you added to the documentation?
Nothing. A patch changeset is included.
Notice: this pr is mostly AI-generated. It originated from similar PR (rstackjs/rspack-dev-server#267) to rspack-dev-server which I verified on a real app.