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: better support for external URL as config.base #4274

Closed
wants to merge 3 commits into from

Conversation

aleclarson
Copy link
Member

Description

This PR makes some changes to account for config.base possibly being an external URL (eg: http://localhost:5000/).


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.

@aleclarson aleclarson added the p3-minor-bug An edge case that only affects very specific usage (priority) label Jul 16, 2021
@patak-dev
Copy link
Member

@aleclarson do you have a reproduction for what this PR is fixing? When running in dev mode, base will only be the pathname part of the external URL, so the current code should work fine. Check here for the base resolving during config:

base = parsed.pathname || '/'

@aleclarson
Copy link
Member Author

aleclarson commented May 19, 2022

Seems odd that we don't use the full base value in development. If someone wants a different base in development, they should just use a conditional config. This is especially concerning behavior since there's no way to avoid it.

@patak-dev
Copy link
Member

How that would work? Normally an external base is the URL base where it is deployed, no? Or would you force users to conditionally change the base during development and extract the pathname? And for the use case when the external base is localhost, why is that useful compared to using the port config and the base for the pathname? 🤔

@patak-dev
Copy link
Member

Lets close this and discuss possible changes to how Vite treats base in dev mode in another issue or PR. IMO, it is ok that the dev server would use a simplified scheme compared to build. Both for external and relative base Vite us forcing '/' during dev now. See also #8450 for new advanced build base options, maybe a similar server.advancedBaseOptions could be added later

@patak-dev patak-dev closed this Jun 14, 2022
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.

None yet

3 participants