csstools / postcss-plugins

PostCSS Tools and Plugins
https://preset-env.cssdb.org/
MIT No Attribution
888 stars 71 forks source link

`postcss-light-dark-function` generates bloat css in `:root` #1454

Closed vovsemenv closed 4 weeks ago

vovsemenv commented 1 month ago

What would you want to propose?

I try to use do this

:root {
    --ad-other-focus: light-dark(var(--system-blue-200), var(--system-blue-300));
    --constant-bg-base-primary: light-dark(var(--gray-neutral-900), var(--constant-bg-base-primary-night));
}

but since i have 150+ colors every light-dark produce new selector like this i end up in the selector hell. ( even chrome devTools stops working

:root {
    --csstools-light-dark-toggle--0: var(--csstools-color-scheme--dark) var(--system-blue-200);
    --ad-other-focus: var(--csstools-light-dark-toggle--0, var(--system-blue-300));
    --csstools-light-dark-toggle--1: var(--csstools-color-scheme--dark) var(--gray-neutral-900);
    --constant-bg-base-primary: var(--csstools-light-dark-toggle--1, var(--constant-bg-base-primary-night));
}

:root {
    & * {
        --csstools-light-dark-toggle--1: var(--csstools-color-scheme--dark) var(--gray-neutral-900);
        --constant-bg-base-primary: var(--csstools-light-dark-toggle--1, var(--constant-bg-base-primary-night));
    }
}

:root {
    & * {
        --csstools-light-dark-toggle--0: var(--csstools-color-scheme--dark) var(--system-blue-200);
        --ad-other-focus: var(--csstools-light-dark-toggle--0, var(--system-blue-300));
    }
}

Suggested solution

// @postcss-light-dark-no-subselector
:root {
    --ad-other-focus: light-dark(var(--system-blue-200), var(--system-blue-300));
    --constant-bg-base-primary: light-dark(var(--gray-neutral-900), var(--constant-bg-base-primary-night));
}

that lead to this result without additional selectors

:root {
    --csstools-light-dark-toggle--0: var(--csstools-color-scheme--dark) var(--system-blue-200);
    --ad-other-focus: var(--csstools-light-dark-toggle--0, var(--system-blue-300));
    --csstools-light-dark-toggle--1: var(--csstools-color-scheme--dark) var(--gray-neutral-900);
    --constant-bg-base-primary: var(--csstools-light-dark-toggle--1, var(--constant-bg-base-primary-night));
}

Additional context

No response

Validations

Would you like to open a PR for this feature?

romainmenke commented 1 month ago

Hi @vovsemenv,

Thank you for reaching out about this.

Unfortunately there isn't much we can do about this. The generated CSS is correct and the smallest/simplest it can be.

We test this extremely thoroughly and pass all the same tests that browsers do.

The :root * {} blocks are needed because of how inheritance works with light-dark() and custom properties.

You can see this in action in this codepen: https://codepen.io/romainmenke/pen/wvLGwyb

Notice how the output for light-dark() is computed for each child element and not only once at the root.


Maybe it is too soon to use light-dark() with so many colors and also in a setup where everything is a custom property?

Fallbacks/polyfills can never be as performant and efficient as native implementations. Using a feature this intensive with a fallback/polyfill might not be ideal?


i end up in the selector hell

How do you mean?

These are all very low specificity selectors and shouldn't conflict with anything you wrote yourself.

Do you simply mean that there is a lot of generated CSS?

even chrome devTools stops working

That seems like an issue with dev tools, not with postcss-preset-env?


Is there an actual problem when rendering pages? Or only a large CSS file and buggy chrome dev tools?

romainmenke commented 1 month ago

🤔 We can simplify one thing.

We can output a single & * {} rule per parent rule instead one per light-dark().

:root {
    & * {
        --csstools-light-dark-toggle--1: var(--csstools-color-scheme--dark) var(--gray-neutral-900);
        --constant-bg-base-primary: var(--csstools-light-dark-toggle--1, var(--constant-bg-base-primary-night));
    }
}

:root {
    & * {
        --csstools-light-dark-toggle--0: var(--csstools-color-scheme--dark) var(--system-blue-200);
        --ad-other-focus: var(--csstools-light-dark-toggle--0, var(--system-blue-300));
    }
}

->

:root {
    & * {
        --csstools-light-dark-toggle--1: var(--csstools-color-scheme--dark) var(--gray-neutral-900);
        --constant-bg-base-primary: var(--csstools-light-dark-toggle--1, var(--constant-bg-base-primary-night));
        --csstools-light-dark-toggle--0: var(--csstools-color-scheme--dark) var(--system-blue-200);
        --ad-other-focus: var(--csstools-light-dark-toggle--0, var(--system-blue-300));
    }
}
vovsemenv commented 1 month ago

🤔 We can simplify one thing.

We can output a single & * {} rule per parent rule instead one per light-dark().

This is actually can help a lot too!! Let me know if i can contribute that myself

romainmenke commented 1 month ago

You can! 😄

The contributing guides are here : https://github.com/csstools/postcss-plugins/blob/main/CONTRIBUTING.md#quick-start

The relevant code here is : https://github.com/csstools/postcss-plugins/blob/66f7becd050ab94d36594de6f0eda1b85316eaef/plugins/postcss-light-dark-function/src/index.ts#L87-L97

Keeping track of state and data can be done in the prepare block: https://github.com/csstools/postcss-plugins/blob/66f7becd050ab94d36594de6f0eda1b85316eaef/plugins/postcss-light-dark-function/src/index.ts#L15-L18

I would likely use something like a Map() and only create one variableInheritanceRule for each unique decl.parent.


Test config is here : https://github.com/csstools/postcss-plugins/blob/main/plugins/postcss-light-dark-function/test/_tape.mjs

Maybe best to create a new test CSS file : tests/common-patterns-1.css. A separate test file is likely more representative of your use case.

And add an entry for it in _tape.mjs :

    'common-patterns-1': {
        message: 'support common patterns',
    },
vovsemenv commented 4 weeks ago

@romainmenke thank you very much for amazing work!!!

romainmenke commented 4 weeks ago

@vovsemenv This has been released and you can also try it in the playground: https://preset-env.cssdb.org/playground/#JTdCJTIyc291cmNlJTIyJTNBJTIyJTNBcm9vdCUyMCU3QiU1Q24lMjAlMjAlMjAlMjAtLWFkLW90aGVyLWZvY3VzJTNBJTIwbGlnaHQtZGFyayh2YXIoLS1zeXN0ZW0tYmx1ZS0yMDApJTJDJTIwdmFyKC0tc3lzdGVtLWJsdWUtMzAwKSklM0IlNUNuJTIwJTIwJTIwJTIwLS1jb25zdGFudC1iZy1iYXNlLXByaW1hcnklM0ElMjBsaWdodC1kYXJrKHZhcigtLWdyYXktbmV1dHJhbC05MDApJTJDJTIwdmFyKC0tY29uc3RhbnQtYmctYmFzZS1wcmltYXJ5LW5pZ2h0KSklM0IlNUNuJTdEJTIyJTJDJTIyY29uZmlnJTIyJTNBJTdCJTIyYnJvd3NlcnMlMjIlM0ElNUIlMjIlM0UlMjAwLjIlMjUlMjBhbmQlMjBub3QlMjBkZWFkJTIyJTVEJTJDJTIybWluaW11bVZlbmRvckltcGxlbWVudGF0aW9ucyUyMiUzQTAlMkMlMjJzdGFnZSUyMiUzQTIlMkMlMjJsb2dpY2FsJTIyJTNBJTdCJTIyaW5saW5lRGlyZWN0aW9uJTIyJTNBJTIybGVmdC10by1yaWdodCUyMiUyQyUyMmJsb2NrRGlyZWN0aW9uJTIyJTNBJTIydG9wLXRvLWJvdHRvbSUyMiU3RCU3RCU3RA==

Thank you again for reporting this!