vitejs / vite

Next generation frontend tooling. It's fast!
http://vitejs.dev
MIT License
67.28k stars 6.05k forks source link

Support parsing .htm as same as .html #10997

Open PeyaPeyaPeyang opened 1 year ago

PeyaPeyaPeyang commented 1 year ago

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 commented 1 year ago

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

github-actions[bot] commented 1 year ago

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 commented 1 year ago

I've made a PR for this: #11001

bluwy commented 1 year ago

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.

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

Potato1682 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

Oh, I didn't think of the SPA fallback. sirv already supports .htm. So in preview, index.htm currently works. https://github.com/vitejs/vite/blob/a03860f1320c0bff8e2b8b7fd0e1c42a7565e767/packages/vite/src/node/preview.ts#L115-L126

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

jasonkester commented 1 year ago

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 commented 1 year ago

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 https://github.com/vitejs/vite/issues/8000