feat: direct ws connection fallback #4474
Conversation
|
|
✅ Deploy Preview for rsbuild ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
| nodeResult.family === dnsResult.family && | ||
| nodeResult.address === dnsResult.address; | ||
| return isSame ? undefined : nodeResult.address; | ||
| } |
There was a problem hiding this comment.
Code is copied from Vite up to line 61
https://github.com/vitejs/vite/blob/main/packages/vite/src/node/utils.ts
There was a problem hiding this comment.
It would be nice if we could do some refactoring based on Rsbuild's needs.
There was a problem hiding this comment.
Is there any special needs you would like?
There was a problem hiding this comment.
the current version is LGTM 👍
|
HMR successful on Tauri iOS |
packages/core/src/client/hmr.ts
Outdated
| @@ -211,13 +212,19 @@ function onClose() { | |||
| removeListeners(); | |||
| connection = null; | |||
| reconnectCount++; | |||
| setTimeout(connect, 1000 * 1.5 ** reconnectCount); | |||
| setTimeout(() => connect(true), 1000 * 1.5 ** reconnectCount); | |||
There was a problem hiding this comment.
I am worried that changing the default fallback logic here will cause some cases to fail to connect.
In Vite, each connect will use the host of the current page first, and then use the direct host if it fails. (https://github.com/vitejs/vite/blob/main/packages/vite/src/client/client.ts#L53-L71)
However, this PR forces all re-connects to use only the direct host, but it is obvious that it cannot be connected in some scenarios, such as on Cloud IDE.
There was a problem hiding this comment.
I have changed to onError instead of onClose so that it wont break existing reconnect logic. Would this be better?
There was a problem hiding this comment.
Nice, using onError looks good to me 👍
| ]); | ||
|
|
||
| export async function resolveHostname( | ||
| optionsHost: string | boolean | undefined, |
There was a problem hiding this comment.
Rsbuild's server.host only supports string type, maybe we can remove boolean type here.
| @@ -0,0 +1,75 @@ | |||
| import { promises as dns } from 'node:dns'; | |||
| import { DevConfig, ServerConfig } from 'src/types/config'; | |||
| let name = host === undefined || wildcardHosts.has(host) ? 'localhost' : host; | ||
|
|
||
| if (host === 'localhost') { | ||
| // See #8647 for more details. |
There was a problem hiding this comment.
Do we need to update this comment?
Summary
Related Links
Resolves #4466
Checklist