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.

Sign up for GitHub

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

Jump to bottom

chore: extract DEFAULT_DEV_PORT and DEFAULT_PREVIEW_PORT #11669

Merged
merged 1 commit into from
Jan 17, 2023
Merged

chore: extract DEFAULT_DEV_PORT and DEFAULT_PREVIEW_PORT #11669

merged 1 commit into from
Jan 17, 2023

Conversation

chenfan0
Copy link
Contributor

Description

In the packages/vite/src/node/server/index.ts file, 5173 is used 3 times, and I personally think that 5173 should be extracted as a constant

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 PR Title 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.

@chenfan0 chenfan0 changed the title chore: extract 5173 as a constant chore: extract DEFAULT_DEV_PORT and DEFAULT_PREVIEW_PORT Jan 12, 2023
Shinigami92
Shinigami92 previously approved these changes Jan 12, 2023
View reviewed changes
Shinigami92
Shinigami92 reviewed Jan 12, 2023
View reviewed changes
Copy link
Member

@Shinigami92 Shinigami92 left a comment

Choose a reason for hiding this comment

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

Maybe we also need to re-export it from https://github.com/vitejs/vite/blob/main/packages/vite/src/node/index.ts to make it available outside (via import ... from 'vite'), do we?

@chenfan0
Copy link
Contributor Author

chenfan0 commented Jan 12, 2023 •
edited by Shinigami92
Loading

Maybe we also need to re-export it from https://github.com/vitejs/vite/blob/main/packages/vite/src/node/index.ts to make it available outside (via import ... from 'vite'), do we?

/packages/vite/src/node/index.ts file will export packages/vite/src/node/publicUtils.ts. so it can be accessed externally(via import ... from 'vite')

@Shinigami92 Shinigami92 added the p2-nice-to-have Not breaking anything but nice to have (priority) label Jan 12, 2023
@Shinigami92 Shinigami92 added this to the 4.1 milestone Jan 12, 2023
@chenfan0 chenfan0 requested review from Shinigami92 and removed request for bluwy and sapphi-red January 12, 2023 09:12
@bluwy bluwy merged commit c9f009d into vitejs:main Jan 17, 2023
futurGH pushed a commit to futurGH/vite that referenced this pull request Feb 26, 2023
@chenfan0 @futurGH
chore: extract DEFAULT_DEV_PORT and DEFAULT_PREVIEW_PORT (vitejs#11669)
51d474c
@chenfan0 chenfan0 deleted the chore/extract-port branch June 21, 2023 09:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bluwy bluwy bluwy approved these changes

@Shinigami92 Shinigami92 Awaiting requested review from Shinigami92

Assignees
No one assigned
Labels
p2-nice-to-have Not breaking anything but nice to have (priority)
Projects
None yet
Milestone
4.1
Development

Successfully merging this pull request may close these issues.
3 participants
@chenfan0 @Shinigami92 @bluwy