Skip to content
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: respect new port when change the config file #6075

Merged
merged 6 commits into from
Dec 14, 2021

Conversation

CHOYSEN
Copy link
Contributor

@CHOYSEN CHOYSEN commented Dec 11, 2021

Description

Fixes #6074

What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

Before submitting the PR, please make sure you do the following

  • Read the Contributing Guidelines.
  • Read the Pull Request Guidelines and follow the Commit Convention.
  • Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
  • Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
  • Ideally, include relevant tests that fail without this PR but pass with it.

patak-dev
patak-dev previously approved these changes Dec 11, 2021
@patak-dev
Copy link
Member

@CHOYSEN did you test what happens when you have two dev servers running (port 3000 and 3001) and change the port?

@Shinigami92 Shinigami92 added the p3-minor-bug An edge case that only affects very specific usage (priority) label Dec 11, 2021
Shinigami92
Shinigami92 previously approved these changes Dec 11, 2021
bluwy
bluwy previously approved these changes Dec 11, 2021
@CHOYSEN
Copy link
Contributor Author

CHOYSEN commented Dec 12, 2021

It works as expect, i try to change the first dev server from 3000 to 3001. @patak-dev

test@C02FV5VTMD6M vite-config-issue 2 % pnpm dev

> vite-config-issue@0.0.0 dev /Users/test/Desktop/vite-config-issue 2
> vite


  vite v2.7.1 dev server running at:

  > Local: http://localhost:3000/
  > Network: use `--host` to expose

  ready in 403ms.

下午5:03:26 [vite] vite.config.ts changed, restarting server...
Port 3001 is in use, trying another one...
下午5:03:26 [vite] server restarted.


  > Local: http://localhost:3002/
  > Network: use `--host` to expose

@CHOYSEN CHOYSEN dismissed stale reviews from bluwy, Shinigami92, and patak-dev via 9b0f389 December 12, 2021 03:27
@CHOYSEN
Copy link
Contributor Author

CHOYSEN commented Dec 12, 2021

For strictPort, we print the error and waiting for user to change the port again.

import { defineConfig } from "vite";

export default defineConfig({
  server: {
    port: 3000,
    strictPort: true
  }
});

Change to port 3001:

test@C02FV5VTMD6M vite-config-issue 2 % yarn dev
yarn run v1.22.10
warning package.json: No license field
$ vite

  vite v2.7.1 dev server running at:

  > Local: http://localhost:3000/
  > Network: use `--host` to expose

  ready in 380ms.

上午11:30:31 [vite] vite.config.ts changed, restarting server...
Error: Port 3001 is already in use

Change to port 3002:

上午11:31:07 [vite] vite.config.ts changed, restarting server...
上午11:31:07 [vite] server restarted.


  > Local: http://localhost:3002/
  > Network: use `--host` to expose

@bluwy
Copy link
Member

bluwy commented Dec 12, 2021

It would be nice to handle host as well so we can close #5996.

@CHOYSEN
Copy link
Contributor Author

CHOYSEN commented Dec 14, 2021

It would be nice to handle host as well so we can close #5996.

Hi @btea, would you like to agree this?

@btea
Copy link
Collaborator

btea commented Dec 14, 2021

It would be nice to handle host as well so we can close #5996.

Hi @btea, would you like to agree this?

Yes.

@CHOYSEN
Copy link
Contributor Author

CHOYSEN commented Dec 14, 2021

It would be nice to handle host as well so we can close #5996.

Hi @btea, would you like to agree this?

Yes.

Thanks!

Copy link
Member

@bluwy bluwy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what's up with CI, but this looks good to me.

@patak-dev
Copy link
Member

I re-run the jobs, CI is a bit moody lately. We need to move away from Jest, we are working on it 😉

@patak-dev patak-dev merged commit 3ceffcc into vitejs:main Dec 14, 2021
@CHOYSEN CHOYSEN deleted the fix/02 branch December 15, 2021 02:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p3-minor-bug An edge case that only affects very specific usage (priority)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

vite doesn't respect new port in config file
5 participants