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.

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

🐛 BUG: [AST] Style tags get merged into html tag in certain situations #712

Closed
Princesseuh opened this issue Jan 23, 2023 · 7 comments · Fixed by #974
Closed

🐛 BUG: [AST] Style tags get merged into html tag in certain situations #712

Princesseuh opened this issue Jan 23, 2023 · 7 comments · Fixed by #974
Assignees
Labels
- P4: important Violate documented behavior or significantly improves performance (priority) downstream-blocker This issue is blocking something downstream feat: ast Related to the JSON AST output (scope)

Comments

@Princesseuh
Copy link
Member

What version of @astrojs/compiler are you using?

1.0.1

What package manager are you using?

pnpm

What operating system are you using?

macOS

Describe the Bug

In some cases, style tags gets merged into html. It seems to depends on the content of head, normal content works, but having a component there cause an issue. Cause the following issue in the Prettier plugin: withastro/prettier-plugin-astro#316

Minimum repro:

<html lang="en">
<head>
    <BaseHead />
</head>
</html>

<style>
    @use "../styles/global.scss";
</style>

gets formatted into:

<html lang="en">
  <head>
    <BaseHead />
  </head>
  <style>
    @use "../styles/global.scss";
  </style>
</html>

Result AST: https://gist.github.com/Princesseuh/7ac0827647d2ff540a7bcadc7e8c11dc

Link to Minimal Reproducible Example

See description

jasikpark added a commit that referenced this issue Jan 23, 2023
This checks in the input from #712 that moves the component from the head. The way I've written this test is kinda painful for generating the unit test snapshots though...
jasikpark added a commit that referenced this issue Jan 23, 2023
This checks in the input from #712 that moves the component from the head. The way I've written this test is kinda painful for generating the unit test snapshots though...
@natemoo-re natemoo-re self-assigned this Feb 14, 2023
@natemoo-re natemoo-re added the feat: ast Related to the JSON AST output (scope) label Jun 27, 2023
@natemoo-re natemoo-re added the downstream-blocker This issue is blocking something downstream label Jul 13, 2023
@HummingMind
Copy link

+1. Same issue.
Since I have "format on save" emabled, this happens every single save of the main layout. I almost checked it into source control that way once. 😢

Hoping for a fix soon! 🍻
Thank you!

@mks-h
Copy link

mks-h commented Sep 2, 2023

At least in my codebase, this issue doesn't break anything. I just let it be, and commit the way it is.

@chankruze
Copy link

chankruze commented Sep 3, 2023

It's still happening.

---
import BaseHead from "../components/BaseHead.astro";
import Header from "../components/Header.astro";
import { SITE_DESCRIPTION, SITE_TITLE } from "../consts";
interface Props {
  title: string;
}

const { title } = Astro.props;
---

<!doctype html>
<html lang="en">
  <head>
    <BaseHead title={SITE_TITLE} description={SITE_DESCRIPTION} />
  </head>
  <body>
    <Header title={SITE_TITLE} />
    <slot />
    <style is:global>
      :root {
        --accent: 136, 58, 234;
        --accent-light: 224, 204, 250;
        --accent-dark: 49, 10, 101;
        --accent-gradient: linear-gradient(
          45deg,
          rgb(var(--accent)),
          rgb(var(--accent-light)) 30%,
          white 60%
        );
      }
      html {
        font-family: system-ui, sans-serif;
        background: #13151a;
        background-size: 224px;
      }
      code {
        font-family:
          Menlo,
          Monaco,
          Lucida Console,
          Liberation Mono,
          DejaVu Sans Mono,
          Bitstream Vera Sans Mono,
          Courier New,
          monospace;
      }
    </style>
  </body>
</html>

@Princesseuh Princesseuh added the - P4: important Violate documented behavior or significantly improves performance (priority) label Sep 8, 2023
@cprcrack
Copy link

cprcrack commented Nov 9, 2023

Is there a workaround for this? It's incredibly annoying...

@cprcrack
Copy link

cprcrack commented Nov 9, 2023

Same thing happens with <script> tags by the way, they get moved to the <body>

@paidge
Copy link

paidge commented Jan 10, 2024

Hi and happy new year. I met the same problem. For a workaround, I disable Autosave during the edition of the file

@yudai-nkt
Copy link

yudai-nkt commented Feb 25, 2024

In some cases, style tags gets merged into html. It seems to depends on the content of head, normal content works, but having a component there cause an issue.

It looks like we can reproduce this without <head> element involved. Below is a reproduction taken from withastro/prettier-plugin-astro#408, where I've mistakenly submitted a duplicate:

import { format } from "prettier";

const source = `
<div>
</div>
<style>
</style>
`;

console.log(
  await format(source, { parser: "astro", plugins: ["prettier-plugin-astro"] })
);

console.log(
  await format(source.replace("div", "html"), {
    parser: "astro",
    plugins: ["prettier-plugin-astro"],
  })
);

I hope this smaller reproduction helps troubleshooting the problem.

MoustaphaDev added a commit that referenced this issue Mar 2, 2024
Princesseuh added a commit that referenced this issue Jul 16, 2024
* chore: `only` helper for json test

* fix: remove divergence from html spec

* test: add test for #712

* `addLoc` before popping the stack of oe

* test: add test

* literal parsing after `body` and `html`

* test: add test for style tag after body

* test: add tsx tests

* test: add missing semicolon in test

* test: add more tests from duplicates

* nit: formatting

* chore: changeset

* test: add AST tests

---------

Co-authored-by: Nate Moore <[email protected]>
Co-authored-by: Erika <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
- P4: important Violate documented behavior or significantly improves performance (priority) downstream-blocker This issue is blocking something downstream feat: ast Related to the JSON AST output (scope)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants