Skip to content

[NEXT-983] app router CSS handling seems incompatible with CSS Cascade Layers - no way to guarantee that CSS that defines layers can be loaded before any usage of the CSS layers #47585

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

Closed
1 task done
philwolstenholme opened this issue Mar 27, 2023 · 9 comments · Fixed by #48244
Assignees
Labels
bug Issue was opened via the bug report template. linear: next Confirmed issue that is tracked by the Next.js team.

Comments

@philwolstenholme
Copy link
Contributor

philwolstenholme commented Mar 27, 2023

(I'm not 100% sure if this is a Next issue or a RSC/React core issue)

Verify canary release

  • I verified that the issue exists in the latest Next.js canary release

Provide environment information

See https://codesandbox.io/p/sandbox/next-js-css-cascade-layers-hkqesn

Which area(s) of Next.js are affected? (leave empty if unsure)

No response

Link to the code that reproduces this issue

https://codesandbox.io/p/sandbox/next-js-css-cascade-layers-hkqesn

To Reproduce

  1. Open the Codesandbox https://codesandbox.io/p/sandbox/next-js-css-cascade-layers-hkqesn
  2. Note that the disabled buttons are not all grey, the 'theme' colour is being applied instead of the 'state' colour
  3. Use devtools to re-order the CSS files in the <head> and note that the button background colours change. Now the 'state' styling has taken over the background colour – this is what we want!

image

image

Describe the Bug

CSS Cascade Layers are an exciting feature in CSS with strong browser support. An example use case would be to make sure that utility classes could always override component styles, or that local styles could override imported shared company-wide styles to allow modifications on a per-app basis.

The order that the browser discovers layers is very important. Users of CSS cascade layers and Next.js need a way to define their layers before any other CSS is encountered by the browser, or else the order of the layers that the browser picks may not be what the developer wanted.

This seems difficult at the moment as layout CSS appears in the DOM after per-page or per-component styles:

image

Workarounds

To make sure it comes first I’ve had to add a new CSS file containing the layer definitions (e.g. @layer default, theme, state;) to my public directory, and import it with a precedence prop that isn’t in the types :( This is a shame as it's a non-standard way of doing things and the file now isn't processed in the same way as my other CSS files:

image

I am also considering undoing the above, but using https://github.com/ramonvictor/postcss-prepend-imports to import a CSS file that defines the layers into every CSS file, so it doesn't matter which one the browser encounters first. I'd rather not do this because of the unnecessary bytes.

Expected Behavior

I'd love any of these:

  • a way to control the 'weight' of a CSS import so that I could give my CSS layer defining import a weight of -100 and have it always come first
  • an officially supported way to include a <link> element with a precedence prop value that will make it appear in the DOM order before any other link type="stylesheet" elements
  • an officially supported way to include some inline CSS (<style>@layer default, theme, state;</style> ) that will appear in the DOM order before any link type="stylesheet" elements

Which browser are you using? (if relevant)

N/A

How are you deploying your application? (if relevant)

N/A

NEXT-983

@philwolstenholme philwolstenholme added the bug Issue was opened via the bug report template. label Mar 27, 2023
@philwolstenholme philwolstenholme changed the title Next's CSS handling is incompatible with CSS Cascade Layers as there is no way to guarantee that CSS that defines the layers can be loaded before any usage of the CSS layers Next's CSS handling seems incompatible with CSS Cascade Layers as there is no way to guarantee that CSS that defines layers can be loaded before any usage of the CSS layers Mar 27, 2023
@philwolstenholme
Copy link
Contributor Author

@bencehusi
Copy link

Having the same issue.

How can I control/ensure that an imported css file always stays on the top?

I use tailwind with some component library.

When hot reload happens, the imported css will be always the last before the closing tag - wrecking havoc on my styles.

@gnoff
Copy link
Contributor

gnoff commented Apr 10, 2023

@philwolstenholme sorry for the delay, was travelling when we first engaged. back now and going to look at this

@philwolstenholme
Copy link
Contributor Author

No need to apologise @gnoff :) Thanks for looking into it.

@philwolstenholme philwolstenholme changed the title Next's CSS handling seems incompatible with CSS Cascade Layers as there is no way to guarantee that CSS that defines layers can be loaded before any usage of the CSS layers app router CSS handling seems incompatible with CSS Cascade Layers - no way to guarantee that CSS that defines layers can be loaded before any usage of the CSS layers Apr 10, 2023
@gnoff
Copy link
Contributor

gnoff commented Apr 10, 2023

cc @shuding source issue

@gnoff
Copy link
Contributor

gnoff commented Apr 10, 2023

@philwolstenholme the page css is supposed to be higher precedence than the layout css. we're looking into why it is out of order. The idea is that you would just define your layers in the root layouts css imports. the first import in that file should go first when we render the page and therefore you should have a stable order for your layers. For some reason the page css is being ordered first which is not expected. Once we fix the ordering I believe you should have all the tools necessary to stage the layers yourself without any special purpose APIs

Thanks for bringing to our attention

@philwolstenholme
Copy link
Contributor Author

The idea is that you would just define your layers in the root layouts css imports. the first import in that file should go first when we render the page and therefore you should have a stable order for your layers.

Perfect, that sounds like the behaviour I had expected/hoped for when I tried this out for the first time. Thanks for picking this up 🙂

@shuding shuding added the linear: next Confirmed issue that is tracked by the Next.js team. label Apr 11, 2023
@shuding shuding changed the title app router CSS handling seems incompatible with CSS Cascade Layers - no way to guarantee that CSS that defines layers can be loaded before any usage of the CSS layers [NEXT-983] app router CSS handling seems incompatible with CSS Cascade Layers - no way to guarantee that CSS that defines layers can be loaded before any usage of the CSS layers Apr 11, 2023
@shuding shuding self-assigned this Apr 11, 2023
shuding added a commit that referenced this issue Apr 17, 2023
### What?

This PR fixes misordered CSS `<link>` tags. CSS imports in inner layers
(e.g. a page) should always take precedence over outer layers (e.g. root
layout), but currently it's reversed.

### Why?

In layouts we usually define more general styles like globals, resets,
and layout specific things. And in inner layers and pages, things need
to be more detailed and override upper layers if there're any conflicts.

Previously we defined the component segment as

```tsx
<>
  <Component {...props} />
  {assets}
</>
```

which is necessary because of `findDOMNode` - if we put `assets` before
the component, we can't find the correct scrolling DOM and position for
that layer.

However, with `assets` being the last Float will receive the reversed
order of resources.

### How?

I changed the `createComponentTree` function to return a `Component` and
`assets` pair, so in the Layout Router they're no longer passed to the
scroll wrapper altogether but separately.

Closes NEXT-983
Fixes #47585, fixes #46347.

fix NEXT-656

---------

Co-authored-by: JJ Kasper <jj@jjsweb.site>
@philwolstenholme
Copy link
Contributor Author

Looks like this might have a follow-up: #48807

@github-actions
Copy link
Contributor

This closed issue has been automatically locked because it had no new activity for a month. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 30, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue was opened via the bug report template. linear: next Confirmed issue that is tracked by the Next.js team.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants