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: basePath #1282

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

fix: basePath #1282

wants to merge 1 commit into from

Conversation

yuusheng
Copy link

@yuusheng yuusheng commented Mar 2, 2025

fix: #698

Copy link

vercel bot commented Mar 2, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
waku ✅ Ready (Inspect) Visit Preview Mar 2, 2025 3:43pm

Copy link

codesandbox-ci bot commented Mar 2, 2025

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

@yuusheng
Copy link
Author

yuusheng commented Mar 2, 2025

Still working in progress. Currently ssr is working in dev mode. But src/main is 404.

image

@yuusheng
Copy link
Author

yuusheng commented Mar 2, 2025

Some other things need fixing:

  1. Redirect to basePath if user opens http://localhost:3000
  2. Allow using both http://localhost:3000/test/ and http://localhost:3000/test. Only http://localhost:3000/test/ works for now
  3. Generated router should also respect basePath

@dai-shi
Copy link
Owner

dai-shi commented Mar 2, 2025

Hi, thanks for working on it.
Can you please create e2e/fixtures/base-url and the spec, instead of
examples/55_base-url ? We need it to maintain the capability.

@dai-shi
Copy link
Owner

dai-shi commented Mar 2, 2025

  1. Redirect to basePath if user opens http://localhost:3000

I think it's out of the scope.

@yuusheng
Copy link
Author

yuusheng commented Mar 3, 2025

Can you please create e2e/fixtures/base-url and the spec, instead of
examples/55_base-url ?

OK, I will remove that when the PR is ready to merge. It's quite hard to debug without that 😭.

I think it's out of the scope.

As this is the default behavior of Vite, I think this is more friendly to developer. Or we can talk about that in the future instead of in this PR

@dai-shi
Copy link
Owner

dai-shi commented Mar 3, 2025

OK, I will remove that when the PR is ready to merge. It's quite hard to debug without that 😭.

Alright, then create both e2e/fixtures/base-url and the spec, as well as examples/55_base-url and remove examples/55_base-url before merging.
The e2e spec is required to review the PR. You can take time. I'm looking forward to it.

As this is the default behavior of Vite,

Hmm, I'm not sure why it's the default behavior. How does it behave after the build?

@yuusheng
Copy link
Author

yuusheng commented Mar 3, 2025

Hmm, I'm not sure why it's the default behavior. How does it behave after the build?

Vite will redirect only in dev mode as it'll be all static assets after the build. After being deployed, redirection behavior will be handled by some gateway like Nginx stuff.

@dai-shi
Copy link
Owner

dai-shi commented Mar 4, 2025

Vite will redirect only in dev mode

That's what I expected. So, I thought it would be better to keep the behavior the same in both DEV and PRD modes in Waku, but I don't mind following Vite's convention if that helps.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG]: basePath is not working except default value
2 participants