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
fix(server): use --host duplicate port number (#12205) #12211
Conversation
/ecosystem-ci run |
📝 Ran ecosystem CI: Open
|
if (port > 65535) { | ||
reject(new Error('Port not found')) | ||
} | ||
const server = net.connect(port, host) |
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.
Interesting approach. I'm a bit worried about the inverted logic here. Are we sure that connect
only fails when the port is unused? Could a used port still reject the connection making this logic invalid?
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.
I looked up a lot of data. It is only the IP or port number with invalid connection that fails.
Of course, I can't guarantee that this is correct.
I tested Node16 and Node18 in Windows and Mac, and they all work normally
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.
Ecosystem CI is happy, including vite-setup-catalogue. I think we could merge this one to try it out during the beta period and revert later if it causes issues. @sapphi-red what do you think?
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.
Yeah, it's an interesting approach. I think it's a good idea to try this in beta.
Co-authored-by: 翠 / green <green@sapphi.red>
Co-authored-by: 翠 / green <green@sapphi.red>
This fail might cause an issue. This PR's approach doesn't work well in a concurrent situation. I guess this fail happened by the following steps:
|
Umm, it failed 3 out of 7 times. |
In this way, concurrency seems to be inevitable, and better methods need to be found 🤔 |
Description
fix #12205
fix #12144
superseds close #10651
close #10638
When localhost(vite dev) is listening on port 5173, it's possible for 127.0.0.1(vite dev --host) to listen on port 5173 again.
Additional context
Re: #12208
What is the purpose of this pull request?
Before submitting the PR, please make sure you do the following
fixes #123
).