Import CSS at beginning of files

[fregante]

This change is unlikely to break or fix anything at this point, it's just defensive.

Import order matters in CSS, so when importing a JS component with its own CSS before the parent’s CSS, the result will be unexpected and we're more likely to use !important, fighting CSS.

Also if JS is run before CSS it will have access to the DOM before any CSS, which will also be unexpected.

Hopefully a lint rule will automate/enforce this in the future.

Related:

• Use existing components for brick and node lists in editor #2300 (comment)
• Allow import/order to accept a glob/regex import-js/eslint-plugin-import#674

[twschiller]

Given that we've either used something BEM-like or are now using CSS modules/utility classes, I don't think this has been ever caused problems for us in practice?

In the past, I've actually intentionally ordered things the opposite way (with the components CSS class at the bottom). But, I think I understand your point that that'd bad because then it'd override the styles of it's dependencies, even for other components

I'm fine merging this in, but would want a lint rule in place.

[fregante]

Specifically because we're using BEM/modules we might have a rule with a single .jwlktnf class that comes before bootstrap's CSS .btn class while it should be the opposite. The selectors have the same specificity so .btn overrides our style.

It happened in the Related PR I linked. I'm sure it happened other times but I assume developers just append a !important to make it work, so it's not really a visible issue.

I also found CSS modules adding styles to the same element other non-CSS modules files were, like Panel.scss and ElementWizard.module.css. The order in which these two files are imported is somewhat relevant (I'd expect a CSS module to come later)

Unfortunately a lint rule isn't yet available so this can be considered "best effort".

[twschiller]

@BLoe, @BALEHOK thoughts?

Specifically because we're using BEM/modules we might have a rule with a single .jwlktnf class that comes before bootstrap's CSS .btn class while it should be the opposite.

I agree our classes should be coming after bootstrap and our bootstrap theme. Isn't that already taken care of by the top-level import order? http://github.com/pixiebrix/pixiebrix-extension/blob/0f5b1fabeaad883cae625c242015dfaec7c7d0e5/src/options.tsx#L18-L18

Where are there cases that .jwlktnf is coming before the bootstrap CSS? Is there something re-importing bootstrap somewhere?

I also found CSS modules adding styles to the same element other non-CSS modules files were, like Panel.scss and ElementWizard.module.css

Agree having the mix isn't good. Post 1.5.4 we need to take stock of where there's non-module CSS and convert it to modules

[BLoe]

I agree our classes should be coming after bootstrap and our bootstrap theme

+1

Sorry if this is a dumb question, but why does putting the import styles ... at the top of the files make a difference in the css precedence behavior here?

[fregante]

Isn't that already taken care of by the top-level import order?

Yeah, but not everywhere, which is what this PR tries to fix. This is on main currently:

pixiebrix-extension/src/ephemeralForm.tsx

Lines 22 to 24 in 22293c9

	import EphemeralForm from "@/blocks/transformers/ephemeralForm/EphemeralForm";
	import "bootstrap/dist/css/bootstrap.min.css";

Where are there cases that .jwlktnf is coming before the bootstrap CSS?

See above. If ephemeralForm.tsx uses CSS modules then its CSS will appear before bootstrap’s

why does putting the import styles ... at the top of the files make a difference in the css precedence behavior here?

If we were using CSS modules exclusively and we were avoiding all compound selectors (e.g. .root { a:hover {}}) then it would not make a difference since style would always come for exactly one .module.css… but that's not the case, we're using both.

To be fair this should be a rare occurrence, but it's not like nobody writes bugs, 🙂 hence the "defensive programming" approach that is this PR.

[BALEHOK]

The CSS modules are meant to solve such issues, they define "atomic" styles that influence only the elements of the component.

I also found CSS modules adding styles to the same element other non-CSS modules files were, like Panel.scss and ElementWizard.module.css. The order in which these two files are imported is somewhat relevant (I'd expect a CSS module to come later)

I think ElementWizard.module.css overrides default Bootstrap styles, not Panel.scss. Placing BS import before our styles should eliminate the need for !important modifiers in the module.

[fregante]

The CSS modules are meant to solve such issues

As mentioned in my previous comment, they do not (always). We're using compound selectors:

pixiebrix-extension/src/devTools/editor/sidebar/Sidebar.module.scss

Lines 66 to 70 in 361ba23

	.actions {
	display: flex;
	justify-content: space-between;
	button {

pixiebrix-extension/src/devTools/editor/tabs/editTab/editorNodeLayout/EditorNodeLayout.module.scss

Lines 18 to 29 in 361ba23

	.actions {
	height: 0;
	display: flex;
	place-content: center;
	place-items: center;
	gap: 0.2em;
	position: relative;
	z-index: 3; // Above Bootstrap’s .list-item.active
	--size: 22px;
	button {

This generates the selector .abc button and .zyx button. The order in which they appear in the file does matter and they can very well end up targeting the same button

---

In any case, whether CSS modules fix the issue or not, I don't see why we're discussing this, the fix is easy and requires no effort.

[BLoe]

I'd be fine with merging in this change, but also agree with Todd that we need to go through and clean up the non-modules CSS code everywhere at some point. Otherwise we risk running into quirky issue like this one.

[fregante]

clean up the non-modules CSS code everywhere at some point

It's next on my list 👌

Edit: #2704