-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
feat: vite preview
port is used automatically +1
#4219
Conversation
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.
Seems there were some changes in main, could you also do a git rebase -i main
?
okay,already |
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.
@patak-js @ygj6 I will not have time for this today, but the requested changes should not stop us to merge it
It's just more nicer code refactoring requests
We aren't in a rush, but yes, I don't see that this particular refactoring is needed. We can explore that in another PR if you want. But after testing the PR, maybe I missed a discussion but what is the reason to avoid mirroring the API of |
I also think that both In addition, |
You can also set the dev server port in the CLI https://github.com/vitejs/vite/blob/main/packages/vite/src/node/cli.ts#L67 We shouldn't use What I mean is that |
The code is updated, please continue to help review, thank you. |
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.
Looks good @ygj6. We'll discuss this PR with the team this week to seek approval. Thanks!
Description
related #4185
vite preview
port is used automatically+1
.If the port is specified, it will not
+1
What is the purpose of this pull request?
Before submitting the PR, please make sure you do the following
fixes #123
).