Skip to content

fix: normalize html path in transformIndexHtml()#18976

Closed
vanaigr wants to merge 2 commits into
vitejs:mainfrom
vanaigr:fix-node-url
Closed

fix: normalize html path in transformIndexHtml()#18976
vanaigr wants to merge 2 commits into
vitejs:mainfrom
vanaigr:fix-node-url

Conversation

@vanaigr

@vanaigr vanaigr commented Dec 16, 2024

Copy link
Copy Markdown

Problem

Currently, the url accepted by transformIndexHtml() is passed unaltered to applyHtmlTransforms().This causes issues if the url

  1. doesn't refer to a file (see SSR dev server "Pre-transform error: Failed to load url" for relative path #18964), or
  2. contains search parameters or hash (/index.html?a=b or index.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 in createDevHtmlTransformFn().

Fixes #18964

@vanaigr

vanaigr commented Dec 16, 2024

Copy link
Copy Markdown
Author

Also noticed that SSR guide uses req.originalUrl as the html url.

Not sure if this is known, but it doesn't work with base config option since the base removing middleware doesn't affect originalUrl.

return applyHtmlTransforms(html, transformHooks, {
path: url,
filename: getHtmlFilename(url, server),
path: normalized.url,

Copy link
Copy Markdown
Member

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 (

Copy link
Copy Markdown
Author

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?

Copy link
Copy Markdown
Member

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.

@vanaigr vanaigr Dec 20, 2024

Copy link
Copy Markdown
Author

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?

Copy link
Copy Markdown
Member

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).

@sapphi-red sapphi-red added feat: html p3-minor-bug An edge case that only affects very specific usage (priority) labels Dec 18, 2024
@vanaigr vanaigr closed this Mar 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feat: html 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.

SSR dev server "Pre-transform error: Failed to load url" for relative path

3 participants