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

feat: Support subpath deployment #252

Merged
merged 3 commits into from
Jan 23, 2025
Merged

Conversation

berlysia
Copy link
Contributor

@berlysia berlysia commented Jan 11, 2025

fixes #243 partially

The problem occurs because the .js file references in each HTML are written as root-relative paths. If the deployment destination is a subpath (e.g., GitHub Pages), the subpath segment must be included when referencing root-relative paths.

When deploying to https://example.com/path/to/some/dir/, the following mismatch arises, causing the .js file to fail to load:

Reference in the output HTML Actual file path = Correct reference
/static/client-${hash}.js /path/to/some/dir/static/client-${hash}.js

This PR enables deployment to subpaths such as GitHub Pages. See https://vite.dev/config/shared-options#base https://vite.dev/guide/env-and-mode#env-variables

@berlysia berlysia changed the title test: subpath deployment Support subpath deployment Jan 11, 2025
@yusukebe yusukebe changed the title Support subpath deployment feat: Support subpath deployment Jan 14, 2025
@@ -33,7 +33,7 @@ export const Script = (options: Options): any => {
<script
type='module'
async={!!options.async}
src={`/${scriptInManifest.file}`}
src={`${import.meta.env.BASE_URL}${scriptInManifest.file}`}
Copy link
Member

Choose a reason for hiding this comment

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

Should we support without a trailing slash pattern?

defineConfig({
  base: '/subdir' // does not have a trailing slash
})

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, that should be ensured

@yusukebe
Copy link
Member

Hi @berlysia

I think this approach is good. I've left a comment.

Copy link
Member

@yusukebe yusukebe left a comment

Choose a reason for hiding this comment

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

LGTM!

@yusukebe
Copy link
Member

Hey @berlysia !

Looks good! I'll merge this and release a new version. Thank you!

@yusukebe yusukebe merged commit e66a876 into honojs:main Jan 23, 2025
2 checks passed
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.

Support "base" config of Vite or alternatives
2 participants