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

Manage specificity through rule insertion order #1

Closed
kripod opened this issue May 17, 2020 · 18 comments
Closed

Manage specificity through rule insertion order #1

kripod opened this issue May 17, 2020 · 18 comments
Labels
enhancement New feature or request

Comments

@kripod
Copy link
Owner

kripod commented May 17, 2020

Motivation

Currently, rule specificities are managed by repeating generated class names. However, responsive media queries can't be ordered like that, as screen sizes aren't known in advance.

Details

Similarly to the methods of React Native Web and StyleSheet, style rules should be tracked and ordered upon insertion.

@kripod kripod added the enhancement New feature or request label May 17, 2020
@eddyw
Copy link

eddyw commented May 18, 2020

What I've being doing in my personal project is to sort style tags instead of sorting rules. For instance:

<style>.c0 { color: red; } </style>
<style media="(max-width: 360px)">.c1 { color: red; }</style>
<style media="(max-width: 500px)">.c2 { color: red; }</style>

It's easier to insert a style tag before or after another style by just checking the media attr rather than sorting the rules. It also has the advantage of generating less CSS code since you can pretty much avoid writing @media(max-width: 500px) for every single atomic rule. Additionally, it makes it easier to read (without @media pollution everywhere)

For SSR is easier since you can concat all rules within a single <style> tag and concat all common media queries within a single @media(max-width: 500px) for example.

In my case, I know all breakpoints (media queries) ahead of time, so I create them always in the same order because I know those won't change.

Maybe this helps 😄

@kripod
Copy link
Owner Author

kripod commented May 18, 2020

Thank you for joining the discussion, that’s a great idea in fact! I’m not yet quite sure about managing multiple style tags at once, though. It should be an additional step towards more optimized code later on.

I would appreciate if you could share more of your findings from your personal project 😊

@eddyw
Copy link

eddyw commented May 18, 2020

@kripod I'm using emotion. So since I can't actually manage the style sheets myself, I need to create the emotion instances ahead of time before using them. The style tags order depends on the order you call/create an emotion instance. However, I was planning to maybe fork the @emotion/stylesheet (if I'm not mistaking the package name). What I currently do is monkey path the emotion.sheet._insertTag to add the media attributes to the style tags and this also works well when emotion inserts new style tags for dynamic styles in development (easier to debug) while in production it uses a fast mode which keeps about max 65k rules? 🤔 per style tag (difficult to debug since devtools don't show style content but faster). All keeping the pre-defined order of style tags.

Anyways, the only required config I have is to provide a list of breakpoints:

const breakpoints = {
  values: {
    xs: 0,
    sm: 600,
    md: 960,
    lg: 1280,
    xl: 1680,
  },
}

And I generate an instance (or style tag) for each breakpoint. Where xs: 0 would be no media queries. So it doesn't actually allow random media queries and makes it easier to modify them in one single place.

Something I'm not really a fan of is the pre-defined order for selectors such as :hover,:active, and :focus. I did also struggled to define a clear way to solve this. I even tried your approach of duplicating the class name to increase specificity. In the end, I opt for not fighting against it 😅

This is what I came up with:

const a = css({
  ':hover': { color: 'red' }, // .c0:hover
  ':focus': { color: 'yellow' }, // .c1:focus
  ':active': { color: 'lime' }, // .c2:active
})
const b = css({
  ':hover': { color: 'red' }, // .c0:hover <-- reused
  ':active': { color: 'lime' }, // .c3:active <-- new
  ':focus': { color: 'yellow' }, // .c4:focus < -- new
})

You can see that .c0 is reused because it's the first selector used in both. However, in b, the :active is second, so it creates a new rule.

It's kind of like:

// a
generateSelectorRule({ selector: ':hover', specificity: 1 }) // .c0
generateSelectorRule({ selector: ':focus', specificity: 2 }) // .c1
generateSelectorRule({ selector: ':active', specificity: 3 }) // .c2
// b
generateSelectorRule({ selector: ':hover', specificity: 1 }) // .c0 <--- same as 'a'
generateSelectorRule({ selector: ':active', specificity: 2 }) // .c3
generateSelectorRule({ selector: ':focus', specificity: 3 }) // .c4

Sure, it creates more atomic rules but it allows me, as a user, to define the order I want instead of being forced to have it in a pre-defined way. However, I'm still considering if it's worth it or not 😅

I'm also exploring a way to be able to parse, validate, and assign multiple selectors:

css({
  ':hover, :active, :hover': { ... },
})

Right now it's just a dirty key.split(',').map(s => s.trim()) kind of thing but what I aim to accomplish is to be able to do this in a safe way:

css({
   ':hover::before, :hover::after': { ... },
   ':hover:not([disabled]):not([aria-disabled="true"])': { ... },
   ':hover:not([aria-disabled="true"]):not([disabled])': { ... }, // < should be same as previous
   '::before:not([disabled])': { ... }, // < should fail, ":not" is not allowed after pseudo-element ::before
})

I couldn't find a CSS validator written in JS that actually validates selectors, so I'm pretty far from making this last thing work reliably 😅

@kripod
Copy link
Owner Author

kripod commented May 18, 2020

These are very valuable thoughts, thank you for sharing them! Tracking the order of pseudos is an interesting concept and definitely worth considering.

@kripod
Copy link
Owner Author

kripod commented May 20, 2020

@eddyw The 0.3.0 release has introduced support for advanced selectors, including comma-separated lists. They are not ordered in any way right now, but I feel like your idea is the way to go, instead of e.g. raising specificities by repeating &.

As for simple pseudos, I think the default order should remain in place, keeping the amount of generated rules at a minimum. However, overrides should happen when the order of selectors don’t match up with the default. This requires some serious re-architecting, though.

@kripod
Copy link
Owner Author

kripod commented May 21, 2020

While trying to implement progressive enhancement for the :focus-visible property as suggested on MDN, I realized that it's not possible with otion as of today. However, without class repetition, a rule like &:focus:not(:focus-visible) can take place regardless the ordering of &:focus and &:focus-visible.

@namjul
Copy link

namjul commented Jun 6, 2020

Instead of opening a new bug issue i respond here because i believe its what is described.

You can see in the example that the (min-width: 400px) mediy-query in the second paragraph does not get applied.

Example: https://codesandbox.io/s/media-specifity-gehzo

@kripod i guess that is the situation you are trying to solve in this issue.

@kripod
Copy link
Owner Author

kripod commented Jun 6, 2020

@namjul That’s a very important example, thank you. Unfortunately, I’m not yet ready to prepare a fix, due to all the recent saddening events in the world.

@kripod
Copy link
Owner Author

kripod commented Jul 21, 2020

After thinking for a while, I'm trying to conclude all the thoughts above into a single solution. The primary goal is keeping the amount of inserted rules at a minimum, so each of them shall only be inserted to the CSSOM once.

Precedences are easy to enforce for properties and pseudo-classes. However, instead of duplicating class names, they should be ordered in the source by maintaining a pre-defined amount of property groups (count: new Set(PRECEDENCES_BY_PSEUDO_CLASS.values()).size * (Math.max(...cssProperties.map(([property]) => (property.slice(3).match(/-/g) || []).length)) + 1), which equals to 9 * 3 = 27 as of today), isolated from each other.

Custom selectors should take precedence over anything else, allowing complex rules (e.g. &:focus:not(:focus-visible)) to override the effects of simple ones.

As for @media and @supports queries, there are a lot of ordering conventions available, so

In the end, I opt for not fighting against it 😅

As each conditional rule is inserted, their custom-specified order should be tracked upon insertion and rules shall be re-ordered with the CSS injector if a mismatch happens. This would allow both mobile-first and desktop-first approaches to be used with minimal overhead and support for the use of media query range contexts.

Rule ordering summary:

  1. Properties
  2. Pseudo-classes
  3. Media and feature queries, primarily ordered by the precedence of developers, and then by the pre-defined order for properties and pseudo-classes
  4. Custom selectors, ordered by the precedence of developers

@kripod
Copy link
Owner Author

kripod commented Jul 23, 2020

This has been addressed by v0.5.0. The only caveat is that a conditional rule may get inserted into the stylesheet more than once, but that shouldn't cause size issues with gzip or Brotli compression in place.

@kripod kripod closed this as completed Jul 23, 2020
@kripod
Copy link
Owner Author

kripod commented Jul 23, 2020

@all-contributors please add @eddyw for ideas and @namjul for bug reporting!

@allcontributors
Copy link
Contributor

@kripod

I've put up a pull request to add @eddyw! 🎉

@kripod
Copy link
Owner Author

kripod commented Jul 23, 2020

No, not like that.

@all-contributors please add @eddyw for ideas!
@all-contributors please add @namjul for bug reporting!

@allcontributors
Copy link
Contributor

@kripod

I could not determine your intention.

Basic usage: @all-contributors please add @Someone for code, doc and infra

For other usages see the documentation

@kripod
Copy link
Owner Author

kripod commented Jul 23, 2020

@all-contributors please add @eddyw for ideas!

@allcontributors
Copy link
Contributor

@kripod

I've put up a pull request to add @eddyw! 🎉

Repository owner deleted a comment from allcontributors bot Jul 23, 2020
@kripod
Copy link
Owner Author

kripod commented Jul 23, 2020

@all-contributors please add @namjul for bug reporting!

@allcontributors
Copy link
Contributor

@kripod

I've put up a pull request to add @namjul! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants