-
-
Notifications
You must be signed in to change notification settings - Fork 6.3k
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: normalize html path in transformIndexHtml()
#18976
base: main
Are you sure you want to change the base?
Conversation
Also noticed that SSR guide uses Not sure if this is known, but it doesn't work with |
return applyHtmlTransforms(html, transformHooks, { | ||
path: url, | ||
filename: getHtmlFilename(url, server), | ||
path: normalized.url, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we shouldn't normalize path
here. We don't know if /
is responding with the same HTML file with /index.html
when appType: 'custom'
.
Instead, I think we should make devHtmlHook
function work with path
ending with /
.
const devHtmlHook: IndexHtmlTransformHook = async ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My reasoning was that the path should be the same during build and development. And I assumed that the paths always refer to a file during build.
Can the html path be /
during build?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can the html path be
/
during build?
No, it always ends with .html
during build currently. But it's valid to only call transformIndexHtml
in dev and not use it in build.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In that case, if no constraints are placed on the path, shouldn't the user decide filename as well?
What if there would be 2 versions of the function, and one of them would work like the builtin vite server but allow using SSR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In that case, if no constraints are placed on the path, shouldn't the user decide filename as well?
Yes, I was thinking of adding file
parameter to server.transformIndexHtml
. Something like:
type TransformIndexHtmlInput = {
/** the served URL path (may include queries and hashes) (should include base if exists) */
url: string
/**
* the file path to the HTML file
* used to resolve the relative URLs for script tags and other assets
*/
file: string
}
interface ViteDevServer {
transformIndexHtml(input: TransformIndexHtmlInput): Promise<string>
/** @deprecated -- kept for backward compat */
transformIndexHtml(
url: string,
html: string,
originalUrl?: string,
): Promise<string>
}
But this requires more code changes. It would be awesome if you want to work on this (no pressure, I'm also fine only with the fix).
Problem
Currently, the
url
accepted bytransformIndexHtml()
is passed unaltered toapplyHtmlTransforms()
.This causes issues if the url/index.html?a=b
orindex.html#id
).This is different from how default vite server and build behave; they use urls that refer to a file. (The server modifies urls in
htmlFallbackMiddleware()
).Solution
I extracted html path normalization from
htmlFallbackMiddleware()
and used it increateDevHtmlTransformFn()
.Fixes #18964