-
-
Notifications
You must be signed in to change notification settings - Fork 8.7k
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
fix: insert site custom CSS with highest priority #6227
Conversation
Size Change: +214 B (0%) Total Size: 670 kB
ℹ️ View Unchanged
|
@@ -33,6 +37,7 @@ export interface DocusaurusConfig { | |||
// trailingSlash undefined = legacy retrocompatible behavior => /file => /file/index.html | |||
trailingSlash: boolean | undefined; | |||
i18n: I18nConfig; | |||
styling: StylingConfig; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this new styling.css
config a good API design @Josh-Cena ?
Is styling
a good name?
I'd like later to add new styling options like primary color leading to an attempt to auto-generate a color palette, that we could map to Infima/tailwind CSS vars in theme.
IMHO users shouldn't need to write any CSS to be able to tweak a bit the site colors. v1 had options for that too: https://v1.docusaurus.io/docs/en/site-config.html#colors-object
Note: we already have a config.stylesheets
API but it's not exactly the same: this just insert a raw CSS file into the HTML head, and this CSS is never processed/optimized. I suspect it's not widely used because it's documented only in one place: https://docusaurus.io/docs/api/docusaurus-config#stylesheets
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would have gone for styles
... but styling
, sure 👍
config.stylesheets
is not only rarely used but also very misleading: when I help build other sites (e.g. https://typescript-eslint.io/) I often hack this field for injecting <link />
tags...
Ah, clever, I didn't think of using a synthetic plugin to inject the client module... However this is still not strictly the "highest priority" because it can be overridden by global stylesheets imported in the theme components. I had just been wondering how we should solve it |
That didn't work actually 😅 client modules all use I can't figure a good way to have a proper ordering 😅
Our theme doesn't have global stylesheets atm (CSS modules probably don't quality) but I understand the concern. The thing is: routes are all lazy-loaded in dynamic imports, and CSS modules are only loaded inside those dynamic routes. Sometimes there are a few dynamic/static imports layers before we even reach the final CSS, like for I'm not sure how Webpack determines the order for anything dynamic, this is not very clear and looks quite random 😅 Let me know if you see any potential solution, because I can't. Maybe we should process such CSS outside of Webpack as a separate build pipeline 🤷♂️ |
Don't panic haha that was the same mental train I went down in #5987. We have several solutions if the mini CSS extract plugin can't do what we want:
|
According to this Remix blog post, Next.js has a similar and still unsolved CSS ordering problem: vercel/next.js#16630 It doesn't look so easy to implement correctly so I'd rather delay this breaking change after 2.0 |
@slorber What about we still push this change?
|
I don't think it's a good idea until we have found a a way to insert the CSS at the right place.
We don't exactly know yet what's the best way to do that Changing CSS ordering is annoying for our users and we don't want to recommend a new option that barely works as intended and then change it again to something that works better: this will break sites CSS even if it's an improvement.
That seems to be the best option to ensure CSS is at the end, but I don't really know how complex this would be to implement. Processing the CSS file separately can work but we must ensure that hot reloading of CSS still works in dev. |
closing since we shouldn't rely too much on CSS ordering provided by bundler implementation details, while CSS Cascade Layers exist and are widely supported now |
Deprecation notice
themeOptions.customCss
is deprecated in favor ofsiteConfig.styling.css
.customCss
will be removed so you'd rather migrate to this new option.Unlike before, site custom CSS will now be inserted last in the site final stylesheet, leading to site-specific rules being used in priority over Infima and theme styling.
This means that you can likely remove some
!important
usage from your site custom CSSIt can also lead to unwanted side-effects: a custom site CSS rule that was "inactive" and useless (overridden by an Infima/theme rule) would now become "active"
Motivation
Site CSS is often used for customizations
It should be able to override Infima and theme modules CSS easily, without the need for
!important
Site CSS should be inserted last, after Infima/theme modules CSS, so that site CSS rules with same specificity as Infima/theme rules can win and be applied in priority
Note: in some specific cases, site CSS rules might be optimized/deduplicated/merged and may appear at the top of the final stylesheet, but in general they would appear at the end (see discussions in #6222)
Have you read the Contributing Guidelines on pull requests?
yes
Test Plan
test CSS insertion order
Related