MDX Layout css applied after component css on build

claytercek commented 4 months ago

Astro Info

Astro                    v4.7.0
Node                     v20.10.0
System                   macOS (x64)
Package Manager          npm
Output                   static
Adapter                  none
Integrations             @astrojs/mdx
                         @astrojs/sitemap
                         @astrojs/react

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

No response

Describe the Bug

When using the mdx integration, if you specify a layout in the frontmatter, the CSS from that layout will be included after any imported component css when built.

In the linked stackoverflow, pages/index.mdx imports a component (Demo.astro) which has some component styles, and has a layout (RootLayout.astro) which includes some global styles that specify the layer order with the @layer directive.

Here's the relevant css:

/* global styles in RootLayout.astro  */

@layer reset, component;

@layer reset {
    * {
        background: white;
        color: black;
    }
}

/* styles in Demo.astro */

@layer component {
    div {
        background: red;
        color: blue;
    }
}

The compiled css ends up looking like this (whitespace added for readability):

@layer component {
    div[data-astro-cid-tb5vpudz] {
        background: red;
        color: #00f
    }
}

@layer reset, component;
@layer reset{
    * {
        background: #fff;
        color: #000
    }
}

Because the component layer is declared before the reset layer, the reset layer is taking precedence.

What's the expected result?

I'd expect the styles from the layout to be included before the styles from any imported components:

@layer reset, component;
@layer reset{
    * {
        background: #fff;
        color: #000
    }
}

@layer component {
    div[data-astro-cid-tb5vpudz] {
        background: red;
        color: #00f
    }
}

Link to Minimal Reproducible Example

https://stackblitz.com/edit/github-ceeysf?file=src%2Fcomponents%2FDemo.astro

Participation

[ ] I am willing to submit a pull request for this issue.

claytercek commented 4 months ago

As a workaround for the layer declaration issue, I've added the following to the <head> of my Layout.astro:

<style is:inline>
    @layer reset, component;
</style>

matthewp commented 4 months ago

Thanks, are layers important for this bug report? Could the same sort of issue not exist without the use of layers?

claytercek commented 4 months ago

Thanks for the quick reply!

Layers are not required for the bug, just used to illustrate the issue.

Per the astro docs, it's recommended that layout components are imported first so that component styles win out if there's conflicting styles with the same specificity.

A common pattern in Astro is to import global CSS inside a Layout component. Be sure to import the Layout component before other imports so that it has the lowest precedence.

bluwy commented 4 months ago

I wonder if it's because we import the layout this way, which messes with the CSS ordering: https://github.com/withastro/astro/blob/75b93d96b2d7cb78f43ae0016f9219894e092b34/packages/integrations/mdx/src/rehype-apply-frontmatter-export.ts#L20-L30

matthewp commented 3 months ago

@ascorbic I assigned this to you but you might want to wait until @bholmesdev is back and talk to him about it as I think he has context for why we dynamically import the Layout. Ideally we could stop doing that, and then the import could be at the top.

bholmesdev commented 3 months ago

@matthewp Thanks for tagging me on this! Had to refresh myself with the the PR linked in the code comment. TLDR: an async import prevents dev server slowdowns for layouts that make recursive globs for the page. Using async imports to preserve the Vite cache isn't documented behavior, so it could have changed in the past 2 years.

Unfortunately, this isn't something we have in our testing suite. The linked PR was manually tested against Bryce Wray's blog that displayed the issue.

To repro and test, I would:

Create 1000 MDX entries
Create a layout that globs for those 1000 MDX entries
Apply that layout to the MDX pages
Edit one of these MDX entries. See how long the dev server takes to update

If there's still a slowdown when the import is made synchronous, I would keep it where it is.

bluwy commented 1 month ago

I'll re-assign this to myself as we've got reports that this blocks the upgrade to @astrojs/mdx v3 (and a v3 specific problem). We'd want to get users upgraded to latest soon so it doesn't break when Astro 5 lands.

withastro / astro