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.

Sign up for GitHub

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

Jump to bottom

Support parsing .htm as same as .html #10997

Open
4 tasks done
PeyaPeyaPeyang opened this issue Nov 19, 2022 · 9 comments · May be fixed by #11001
Open
4 tasks done

Support parsing .htm as same as .html #10997

PeyaPeyaPeyang opened this issue Nov 19, 2022 · 9 comments · May be fixed by #11001
Labels
contribution welcome enhancement: pending triage

Comments

@PeyaPeyaPeyang
Copy link

PeyaPeyaPeyang commented Nov 19, 2022 •
edited
Loading

Description

We are currently working on a rebuild of our old website.
I included the .htm file in the build target for now, but the build failed with a Parse error.
.htm files are rendered correctly in most modern browsers.
According to RFC2854 and this IANA text, .html and .htm are commonly used HTML extensions and I believe they should be used in vite.

It is a small thing, but I think it will be useful.

Also, I found a regular expression in the HTML plugin file that seems to support .htm, but it is not actually supported.
If the suggestion in this Issue is not good, why is this regex in there?

Suggested solution

Actual works method

in node/plugins/html/ts:301

    async transform(html, id) {
      if (id.endsWith('.html')) {
        const relativeUrlPath = path.posix.relative(

to

    async transform(html, id) {
      if (id.endsWith('.html') || id.endsWith('.htm')) {
        const relativeUrlPath = path.posix.relative(

Alternative

No response

Additional context

Parse error I got:

> homepage@0.0.0 build T:\projects\vite-test
> vite build

vite v3.2.4 building for production...
✓ 0 modules transformed.
[vite:build-import-analysis] Parse error @:8:28
file: T:/projects/vite-test/src/pages/a.htm:8:27
 6:           content="width=device-width, user-scalable=no, initial-scale=1.0, maximum-scale=1.0, minimum-scale=1.0">
 7:     <meta http-equiv="X-UA-Compatible" content="ie=edge">
 8:     <title>Document</title>
                               ^
 9: </head>
10: <body>
error during build:
Error: Parse error @:8:28
    at parse$b (file:///T:/projects/vite-test/node_modules/.pnpm/vite@3.2.4/node_modules/vite/dist/node/chunks/dep-67e7f8ab.js:32642:355)
    at Object.transform (file:///T:/projects/vite-test/node_modules/.pnpm/vite@3.2.4/node_modules/vite/dist/node/chunks/dep-67e7f8ab.js:42081:27)
 ERROR  Command failed with exit code 1.

Validations
@sapphi-red
Copy link
Member

It seems there's many places using id.endsWith('.html').
I think it makes sense to use that htmlLangRE.

@github-actions
Copy link

Hello @PeyaPeyaPeyang. We like your proposal/feedback and would appreciate a contribution via a Pull Request by you or another community member. We thank you in advance for your contribution and are looking forward to reviewing it!

@Potato1682 Potato1682 linked a pull request Nov 20, 2022 that will close this issue
Open
9 tasks
@Potato1682
Copy link

I've made a PR for this: #11001

@bluwy
Copy link
Member

bluwy commented Nov 20, 2022

I think it's a nice to have if it's simple to support, but with the html fallback this makes it a bit complicated.

  • Should we support reading from both index.htm and index.html fallback?
  • MPA support should consider .htm too?
  • The optimizer entries glob also needs to consider it

It feels like it would bring a lot of legacy extensions handling for a modern tool.

@Potato1682
Copy link

I think index.html should be the priority, too.
We might have to implement warnings when both the .html and .htm files are detected and use .html as possible.

@PeyaPeyaPeyang
Copy link
Author

A situation where .html and .htm with the same name exist at the same time is the wrong way to build a site, I think.
If anything, it would be better to issue a build error or warning and give priority to the .html.

@sapphi-red
Copy link
Member

Oh, I didn't think of the SPA fallback.
sirv already supports .htm. So in preview, index.htm currently works.

vite/packages/vite/src/node/preview.ts

Lines 115 to 126 in a03860f

const assetServer = sirv(distDir, {
etag: true,
dev: true,
single: config.appType === 'spa',
setHeaders(res) {
if (headers) {
for (const name in headers) {
res.setHeader(name, headers[name]!)
}
}
}
})

I guess htmlFallbackMiddleware will be a bit complicated but other parts won't be so complicated.

@jasonkester
Copy link

This issue happens for any non-html extension. I ran into it trying to get it to export a .aspx file so that I could run it on the server in ASP.NET and send it to the user with data pre-populated.

I fixed it in a similar method to the user above, searching vite's node_modules folder for the string id.endsWith('.html'), then adding an or clause to include the extension I wanted.

I'd suggest that whoever fixes this for the project modify that line to allow any extension that's being used in the rolloutOptions section of vite.config.

@lubomirblazekcz
Copy link
Contributor

lubomirblazekcz commented Mar 8, 2023 •
edited
Loading

It would also help with support for non-html file types like .pug, .twig, .liquid, .md etc. So replacing endsWith('.html') with htmlLangRE with extendned file types support would be great, or even better to add config option similar to https://vitejs.dev/config/shared-options.html#resolve-extensions

This would also help resolve #8000

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
contribution welcome enhancement: pending triage
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: handle .htm as same as .html (fix #10997) Potato1682/vite
6 participants
@jasonkester @lubomirblazekcz @bluwy @PeyaPeyaPeyang @Potato1682 @sapphi-red