Skip to content

postcss-custom-properties: Enable usage of function for preserve option #778

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
wants to merge 1 commit into from

Conversation

bjentsch
Copy link

postcss-css-variables has a neat configuration option that enables devs to preserve custom properties under certain circumstances, using a function.

What it basically does is that it passes a declaration object to the configured function, which does its filtering magic and then returns a boolean indicating if the custom property should be preserved alongside the compiled output. Possible criteria for the function could be the declarations content, filename, ... you name it.

This PR is my go at implementing this functionality as an addition to only specifying true/false on the preserve configuration option.

WDYT?

@romainmenke
Copy link
Member

Hi @bjentsch 👋
Thank you for your contribution!

What is the motivation for preserving some properties but not others?

@bjentsch
Copy link
Author

Hi @romainmenke ✌️

With postcss-css-variables, we used injection from the TypeScript/JS side to change the variables at runtime while still providing a fallback for older browsers that don't natively support custom properties yet. We would like to replicate this behaviour with postcss-custom-properties.

When preserving some of the variables, those can then be dynamically altered by scripting during runtime, e.g. for a darkmode switch or dynamic color state transitions.

@romainmenke
Copy link
Member

romainmenke commented Jan 12, 2023

But then why not preserve all CSS variables?
I am mainly curious why this much control is needed.

Is it to minify the CSS bundle?


My concern is that this plugin is intended as a fallback for browsers that have no support for CSS variables and not intended to reduce the size.

Personally I preserve both the CSS variables and the fallback for projects that still need to support ancient browsers like IE. For more modern projects the plugin is disabled.

@bjentsch
Copy link
Author

Yes, for this specific project, we only want to preserve variables that are to be mutated by scripting later on.

The other variables should be inlined.

@romainmenke
Copy link
Member

If the variables that should be inlined are more like constants it might be interesting to use something like design tokens : https://github.com/csstools/postcss-plugins/tree/main/plugins/postcss-design-tokens#readme (there are other plugins that do similar things) That plugin inlines final values and doesn't produce CSS variables.

You can also spin of your own variant of postcss-custom-properties dedicated to your specific needs.

But I don't think this feature request aligns with the current goal of the plugin, which is to polyfill custom properties in old browsers like IE. We try to keep these plugins focussed on a single use case.

We have just finished removing importFrom and exportTo from this plugin for this exact reason :)
People had major issues with these plugin options when their browserslist updated and postcss-preset-env stopped running the plugin. Then suddenly all their values were gone.

@bjentsch
Copy link
Author

Alright, thanks, I will dig into design tokens.

I think we would be able to go with these, although to be honest it will add a layer of complexity for daily dev work.

Up until now, we could just mix "static variables" (= design tokens) and "dynamic variables" (= custom properties which can be modified by scripting) into one variable declaration CSS file. This won't be possible with the design tokens as far as I can tell from looking at the design-tokens repo.

@romainmenke
Copy link
Member

romainmenke commented Jan 12, 2023

Yes, that is why a fork would be interesting.
It would allow you to specialize this plugin to suite your needs.

I am sure there will be others with the same needs and we are happy to advice on and share your version (if you decide to build and publish it).

@bjentsch
Copy link
Author

Closing the PR as I don't see a chance of this getting merged, thanks for your input @romainmenke. 🤗

@bjentsch bjentsch closed this Jan 20, 2023
@romainmenke
Copy link
Member

I was just about to comment here too 🤗

Out of curiosity is there anything in particular that would make it easier for you to maintain and publish a fork?

We sometimes have to say no to a feature request like this, but it is always a tough choice.
We do want you to have the feature even if we do not think it fits within our scope.

bjentsch added a commit to bjentsch/postcss-custom-properties that referenced this pull request Jan 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants