withastro / compiler

The Astro compiler. Written in Go. Distributed as WASM.
Other
482 stars 59 forks source link

Missing <body> tag if astro finds an unrecognized head tag #982

Open molszanski opened 6 months ago

molszanski commented 6 months ago

Astro Info

Astro                    v4.5.0
Node                     v18.17.1
System                   macOS (arm64)
Package Manager          npm
Output                   static
Adapter                  none
Integrations             none

If this issue only occurs in one browser, which browser is a problem?

Chrome

Describe the Bug

Input:

<!DOCTYPE html>
<html lang="en">
    <head>
        <title>document</title>
        <!-- Delete next line and body will be red -->
        <lol href='lol'/>
    </head>
    <body style="background-color:red">
        <slot />
    </body>
</html>

Output

<!DOCTYPE html>
<html lang="en">
  <head>
    <title>document</title>
    <lol test="123"></lol>
    <div>
  1231231231
      <span>Hello Astro</span>
    </div>
  </html>

Notice the missing </head> and <body> elements.

Minimal Repro: https://stackblitz.com/edit/github-zkwgaq-g5un8g?file=src%2Fpages%2Findex.astro&file=src%2Flayouts%2FLayout.astro&on=stackblitz

What's the expected result?

valid html. Astro should omit tags it doesn't recogdnize / understand

Link to Minimal Reproducible Example

https://stackblitz.com/edit/github-zkwgaq-g5un8g?file=src%2Fpages%2Findex.astro&file=src%2Flayouts%2FLayout.astro&on=stackblitz

Participation

cdtut commented 6 months ago

Put this in <head> and head tag won't be generated in build. You have to remove inner {} then it will work. But sometimes it is needed for conditionals inside loop. It was working in 4.0.6 stopped working in 4.5.0.

If there is any problem the build should fail instead of output files without head.

{[1, 2, 3].map(() => <>
    {<meta name="" content="" />}
</>)}
bluwy commented 6 months ago

The compiled output seems to be incorrect where the body is not rendered:

// ...
return $$render`<html lang="en">
    <head>

        <title>document</title>

        <!-- Delete next line and body will be red -->
        ${$$renderHead($$result)}</head><lol href="lol"></lol>

        ${$$renderSlot($$result,$$slots["default"])}
    </html>`;
}, '<stdin>', undefined);
// ...
cdtut commented 6 months ago

@bluwy is that the same cause of bug I found?

cdtut commented 6 months ago

Bug caused by astro 4.2.2 it works in 4.2.1.

MoustaphaDev commented 6 months ago

It should get fixed by https://github.com/withastro/compiler/pull/939

cdtut commented 6 months ago

@MoustaphaDev the bug I reported should be fixed by that too?

The PR is 2 months old is something blocking merge?

MoustaphaDev commented 6 months ago

Sorry for the late response, I haven't checked but the PR fixes several issues like that, so hopefully. I'll check if it does early next week and let you know! And no there's nothing major blocking it, just needs more testing. I'll hopefully be able to get it over the line next week

cdtut commented 6 months ago

@MoustaphaDev I would like upgrade to latest version if you can check if this fixes the issue.

MoustaphaDev commented 5 months ago

Hey @cdtut, I didn't get to work on the PR unfortunately, and probably won't be able to next week too. Sorry for the inconvenience, hope somebody will be able to pick this up!