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

Unused Layouts' styles are being imported in Dev Mode #7097

Closed
1 task
LefanTan opened this issue May 15, 2023 · 6 comments · Fixed by #7381
Closed
1 task

Unused Layouts' styles are being imported in Dev Mode #7097

LefanTan opened this issue May 15, 2023 · 6 comments · Fixed by #7381
Assignees
@matthewp
Labels
- P4: important Violate documented behavior or significantly impacts performance (priority)

Comments

@LefanTan
Copy link

LefanTan commented May 15, 2023 •
edited
Loading

What version of astro are you using?

2.4.5

Are you using an SSR adapter? If so, which one?

Node

What package manager are you using?

npm

What operating system are you using?

Mac

What browser are you using?

Chrome, Safari

Describe the Bug

In an Astro project containing multiple Layouts, the global styles would bleed into each other even though a Page might only be using just one layout at a time. Because of the conflicts of CSS that happens only in Dev, this makes working with Layouts extremely difficult on Astro.

Steps to reproduce:

  1. Create a Layout with a <style is:global /> tag. In my example, I had the following style tag
<style is:global>

main {
    background: rgb(255, 227, 196);
}

</style>
  1. Create a Page (we call this Page1) that imports the Layout we just defined.
  2. Create another Page ( we call this Page2) that's just a traditional HTML page with a <main>
<html>
...
  <main>
      Page 2
  </main>
</html>
  1. Start the dev server: npm run dev
  2. You'll notice that Page2's <main> element will have the background color defined in the Layout, even though it isn't using or importing it.

Thanks again to the Astro team.

Link to Minimal Reproducible Example

https://github.com/LefanTan/astro-unused-layout

Participation

  • I am willing to submit a pull request for this issue.
@LefanTan LefanTan changed the title Unused Layouts are being imported in Dev Unused Layouts' styles are being imported in Dev Mode May 15, 2023
@bluwy
Copy link
Member

bluwy commented May 18, 2023

Hi, can you provide the repro with stackblitz or a github repo? Codesandbox shows "task killed" for me and doesn't allow users to download the project :(

@LefanTan
Copy link
Author

LefanTan commented May 18, 2023 •
edited
Loading

@bluwy Hey there, here's the github repo:

https://github.com/LefanTan/astro-unused-layout

@LefanTan
Copy link
Author

LefanTan commented Jun 6, 2023

Just want to bump this issue. Unsure where this lies in priority, but it has made development a nightmare.

There are cases when the "styling" works in development because some unrelated styles are bleeding into each other but when deployed with production build, things will fail because those styles aren't actually there.

@matthewp
Copy link
Contributor

This one is pretty interesting. It combines a few things to make it occur. Essentially this happens:

  1. This project uses Tailwind
  2. When postcss is run it picks up dependencies. Tailwind's glob pattern makes all of the Astro files a dependency.
  3. We call addWatchFile for HMR
  4. Internally Vite turns all calls to addWatchFile into the importedModules on the module entry.
  5. We add styles in dev based on crawling the graph and looking at importedModules.

@bluwy Curious what you think about this one. Is there something other than importedModules that we should be looking at? Is there a way to filter out the imports added via addWatchFile?

@matthewp matthewp added the - P4: important Violate documented behavior or significantly impacts performance (priority) label Jun 12, 2023
@matthewp
Copy link
Contributor

I think we need to check the child module's importers prop to see if it really is a dep.

@matthewp matthewp self-assigned this Jun 12, 2023
@matthewp matthewp mentioned this issue Jun 12, 2023
Merged
@matthewp
Copy link
Contributor

PR up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@matthewp matthewp

Labels
- P4: important Violate documented behavior or significantly impacts performance (priority)
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Prevent accidental CSS inclusion in dev withastro/astro
3 participants
@matthewp @LefanTan @bluwy