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: CSS imported have higher priority than CSS on .astro file #3357

Closed
1 task
Eloi-Perez opened this issue May 12, 2022 · 10 comments · Fixed by #4907
Closed
1 task

🐛 BUG: CSS imported have higher priority than CSS on .astro file #3357

Eloi-Perez opened this issue May 12, 2022 · 10 comments · Fixed by #4907
Assignees
Labels
- P5: urgent Fix build-breaking bugs affecting most users, should be released ASAP (priority)

Comments

@Eloi-Perez
Copy link
Contributor

Eloi-Perez commented May 12, 2022

What version of astro are you using?

1.0.0-beta.27

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

no

What package manager are you using?

npm

What operating system are you using?

Windows

Describe the Bug

When I import CSS like import '../styles/style.css'; or CSS contained in other .astro file like import Layout from '../layouts/Layout.astro'; This imported CSS as priority over the CSS inside the <style> on a .astro file.
This way, an external CSS rule with the same specificity will overwrite the one on <style is:global>
And you could say, use scoped <style>... but certain elements are not affected by the :global() or is:global, like *, html or body.
So if I create a CSS rule for body on an external file it would be hard to change it unless I use !important
We should not rely on specificity for this, the internal CSS should always be written at the bottom of the file.

To sum up, the CSS file (asset.xxx.css) from astro build and the temporal CSS file from astro dev. IMA have the content in the wrong order.

Link to Minimal Reproducible Example

https://stackblitz.com/edit/github-ndeuc4-qbjgyn?file=src%2Fpages%2Findex.astro,src%2Fpages%2Fpage2.astro

Participation

  • I am willing to submit a pull request for this issue.
@natemoo-re natemoo-re added needs discussion Issue needs to be discussed s2-medium labels May 13, 2022
@natemoo-re
Copy link
Member

Thanks for opening an issue! You're right that our specificity is a bit wonky and difficult to control right now.

This problem is something we've discussed in the past but never came to a resolution on. I'm tagging this issue to be reviewed and discussed by the core team.

Related: we have an open PR to ensure that Astro scoped styles use :where() to avoid adding unintended specificity withastro/compiler#330. Would that fix your problem or do you still think the output order has to be adjusted?

@natemoo-re natemoo-re added - P3: minor bug An edge case that only affects very specific usage (priority) and removed needs discussion Issue needs to be discussed labels May 31, 2022
@Eloi-Perez
Copy link
Contributor Author

Eloi-Perez commented Jun 10, 2022

Hi @natemoo-re , I've been using the @layer CSS property as as a way to get around it.
But since I recently updated to the last Astro version (1.0.0-beta.42 from 1.0.0-beta.27). Now astro dev and astro build order the CSS in different ways making it very inconvenient to write CSS as I may get different results on build time.

@filipwiniarski
Copy link

filipwiniarski commented Jul 6, 2022

+1 - Global styles imported in components land before TailwindCSS base styles, losing specificity battle in the result.

A nasty but quick workaround is to simply make global styles slightly more specific and wrap selectors by e.g. body.

@ghostdevv
Copy link
Contributor

really hoping a solution appears 🙏

@Hotell
Copy link

Hotell commented Aug 17, 2022

this happens in 1.0 stable as well

@tudorpopams
Copy link

Hi @natemoo-re , I've been using the @layer CSS property as as a way to get around it. But since I recently updated to the last Astro version (1.0.0-beta.42 from 1.0.0-beta.27). Now astro dev and astro build order the CSS in different ways making it very inconvenient to write CSS as I may get different results on build time.

The astro dev vs astro build CSS order issue is still happening in the stable release (v1.0.5). This creates a big predictability and regression issue on production environments and the workarounds are very hacky. Is there any chance for this to get a higher priority?

@Eloi-Perez
Copy link
Contributor Author

Eloi-Perez commented Sep 24, 2022

astro@1.3.0
I've seen that now mentioned in the documentation the possibility to "Load a static stylesheet via “link” tags" so I've tested it, at the moment, the order of loading is:

  • Layout.astro <head><link rel="stylesheet" href="/styles/test.css" /></head> (CSS on public folder)
  • index.astro <style></style>
  • Layout.astro <style is:global></style>
  • Layout.astro import '../styles/test.css';
  • index.astro import '../styles/test.css';

Same results using astro dev and astro preview

So using "static stylesheet via “link” tags" skips the normal CSS processing, bundling and optimizations BUT, you could write your globals there and you will have the possibility to overwrite them with <style> inside .astro files.

@pikeas
Copy link

pikeas commented Sep 24, 2022

Please consider revising CSS load order to:

  1. Layout.astro <head><link rel="stylesheet" href="..." />
  2. Layout.astro import '...'
  3. Layout.astro <style is:global>
  4. index.astro import '...
  5. index.astro <style>

If those are all added to <head> (rather than any appended just before </body>) , then <div style="..."> will also correctly cascade.

@natemoo-re natemoo-re added - P4: important Violate documented behavior or significantly impacts performance (priority) and removed - P3: minor bug An edge case that only affects very specific usage (priority) labels Sep 27, 2022
@matthewp matthewp self-assigned this Sep 27, 2022
@matthewp matthewp added - P5: urgent Fix build-breaking bugs affecting most users, should be released ASAP (priority) and removed - P4: important Violate documented behavior or significantly impacts performance (priority) labels Sep 27, 2022
@matthewp
Copy link
Contributor

matthewp commented Sep 29, 2022

Hey everyone, we just pushed at a fix for this that will be released in 1.4.0. Let me clarify the ordering as it's intended so that we're all on the same page. Let's say you have this:

---
import '../styles/base.css';
import '../styles/global.css';
---
<html>
  <head>
    <link rel="stylesheet" href="/another.css">

    <style>
      body {
        background: limegreen;
      } 
    </style>
  </head>
</html>

The ordering you should see is:

<link rel="stylesheet" href="/another.css">

<link rel="stylesheet" href="/styles/base.css">
<link rel="stylesheet" href="/styles/global.css">

<!-- this is your <style> -->
<link rel="stylesheet" href="/src/pages/index.astro?type=style...">

The change made is that your <style> always comes after imported styles. This fixes the bug that people have been seeing.

Here's a couple of things to remember:

  1. We do not touch the <link>s you manually add in any way. We are effectively unaware of them.
  2. We always place the <link>s that we add at the bottom of the <head>.
  3. You can control the ordering by controlling the order that you import things. In the above example, if you want base.css to be lower in the HTML order, then import it after global.css. This is also true of components. If you want to prioritize their styles over global.css, then make global.css the first thing you import.

I hope that clears things up! There's still likely some issues to address. Code-splitting in particular makes preserving order very hard, so built-order is more likely to not follow the above rules. Please still report anything that you find.

@Eloi-Perez
Copy link
Contributor Author

Thank you, I just tested 1.4.0 and it works as intended!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
- P5: urgent Fix build-breaking bugs affecting most users, should be released ASAP (priority)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants