Closed hgl closed 9 years ago
Also while testing this option, the --
variable name prefix doesn't seem to be optional as the readme says. Removing them cause cssnext to complain the variables being undefined and need a fallback.
After thinking about it again, I think the solution really shouldn't be preprocessing the passed variables option, but outputting the final computed custom properties, no matter if a variables option is passed or not.
In other words:
:root {
color: #aaa;
}
body {
color: var(--color);
}
Preprocessing this should output the final css along with the computed custom properties, if an option is passed:
$ cssnext --custom-properties props.json file
props.json
{
"color": "#aaa"
}
Not sure if it's doable with postcss?
The plugin cannot return anything but just edit the CSS AST. Maybe we can offer a mutable option but I think we should instead just compute JS variables like it's done for others properties. PRs are welcome for this enhancement.
For the optional --, just check this commit that have a test in it https://github.com/postcss/postcss-custom-properties/commit/0691784ed2218d7e6b16da8c4df03e2ca0c4798c
but I think we should instead just compute JS variables like it's done for others properties.
I'm sorry but not quite sure what do you mean by this. Could you elaborate a bit?
For the optional --, just check this commit that have a test in it 0691784
I was running an older version of this plugin. Now it's working. Thanks for the heads up.
I mean resolve var() in JS variables. Should be pretty easy.
OK, do you mean we should resolve the passed in variables option directly without looking at the actual css source code?
This can fail if users do:
variables option
{ "border-color": "var(color)" }
css
:root {
"color": "#aaa";
}
div {
border: 1px solid var(--border-color);
}
Without the css source code, the passed variables option might be incomplete.
I think the mutable option is a viable approach. What if we modify the variables
option in place?
var opts = {};
postcss()
.use(customProperties(opts))
.process(css);
opts.variables // computed values
and then cssnext could use this property to output the result.
I can starting working on a PR if you think this approach is good.
This plugin works with 2 passes on the complete AST. If we try to resolve JS variables between the first resolution and the variables replacement, I think we should be good. Just here https://github.com/postcss/postcss-custom-properties/blob/0691784ed2218d7e6b16da8c4df03e2ca0c4798c/index.js#L85. Should take a second. I am not a fan of mutability :)
So after the JS variables are resolved, how do we output them?
Are you saying we let cssnext operate directly on json files?
variables.json
{ "--border-color": "var(--color)" }
style.css
:root {
"--color": "#aaa";
}
div {
border: 1px solid var(--border-color);
}
$ cssnext --compute-js-variables variables.json style.css
{ "--border-color": "#aaa" }
We are not in cssnext. This plugin will never handle file.
I just pushed a modification that should make you happy ;) I won't do anything more. Your first example in #22 with color won't be resolved by this plugin. If you want to play with color, you can do that before offering JS variables... using JS ! :p
Closed by a5aa2bfe92d7b5414a9037efa3da95609f2ffdff
OK. I see where our misunderstanding lies.
The bug you fixed actually doesn't exist.
{
"--border-color": "#aaa",
"--background-color": "var(--border-color)"
}
Using this variables option works fine before your fix. Because at the second resolution, the js variable will be recursively resolved.
My feature request was to output the resolved values of all custom properties. I created a commit at my branch. It currently only works with custom properties specified directly in the source code and whose values don't contain var()
. To make it work with custom properties specified with variables
option and those containing var()
, this plugin needs to offer an extra feature (we can discuss it once you have seen my commit).
Let me know if you want such feature in cssnext.
(And if I'm not wrong, this fix should be reverted.)
I will see if what I did was stupid. I don't know what you are trying to do, but you have weird needs. You should defined JS vars and use them as is... Don't make things complicated...
var color = "#aaa"
var props = {
"--border-color": color,
"--background-color": "color(" + color + " alpha(0.2))"
}
but you have weird needs
I need to use these variables in my app, for doing UI animations with web-animations for example:
var startColor = variables["start-color"];
var endColor = variables["end-color"];
elem.animate([{
color: startColor
}, {
color: endColor
}], 3000);
{
"--start-color": "#aaa",
"--end-color" : "color(var(--start-color) alpha(0.2))"
}
a {
color: var(--start-color);
}
a:hover {
color: var(--end-color);
}
The same custom properties will be used both in css and js. So I need the resolved value in order to make it work in most browsers.
So what do you think? Could cssnext offer a list of resolved custom properties?
As I said before, for me you can do that in JS first, without adding an extra step. If you want to share same variables from JS & CSS, just prepare your variables in JS first.
In your case you could do
var color = require("color")
var mainColor = color("#aaa")
module.exports = {
"start-color": mainColor.rgbString(),
"end-color" : mainColor.alpha(0.2).rgbString()
}
cssnext is in javascript so everything that is implemented come from JS. You can use JS directly.
Note: that what me and other people are doing.
Thanks for showing me the tip. Didn't realize this.
However, not super excited about this procedural approach. I'm worried the amount of boilerplate code it entails:
var color = require("color")
var mainColor = color("#aaa")
var pageWidth = 960
var sidebarWidth = pageWidth / 2
module.exports = {
"start-color": mainColor.rgbString(),
"end-color" : mainColor.alpha(0.2).rgbString(),
"page-width": pageWidth + "px",
"sidebar-width": sidebarWidth + "px",
"search-width": sidebarWidth * 0.8 + "px"
}
Comparing it with
{
"start-color": "#aaa",
"end-color" : "color(var(--start-color) alpha(0.2))",
"page-width": "960px",
"sidebar-width": "calc(var(--page-width) / 2)",
"search-width": "calc(var(--sidebar-width) * 0.8)"
}
I personally find the later less painful to write.
But I understand if you don't want the extraction feature in cssnext, and I'm thinking about writing an separate project to provide this feature, but I'd still like to leverage this project to resolve variables. Will you accept an extra option where custom-properties are preserved but their values are replaced with resolved ones?
If you do a correct PR with tests & docs, why not. But what you are trying to looks like a dangerous game.
This es6 code looks better to me, and more flexible:
var color = require("color")
const mainColor = color("#aaa")
const pageWidth = 960
exports default {
"start-color": mainColor.rgbString(),
"end-color" : mainColor.alpha(0.2).rgbString(),
"page-width": `${pageWidth}px`,
"sidebar-width": `${pageWidth / 2}px`,
"search-width": `${pageWidth * 0.8}px`
}
It's explicit.
search-width
depends on sidebar-width
, so it's actually
"search-width": `${pageWidth / 2 * 0.8}px`
Will send a PR soon.
Ideally this extra option should extends the existing preserve
option:
preserve: "specified"
preserve: "computed"
preserve: "both"
Since the feature I'm looking overlaps with the existing preserve
. true
can be aliased to both
to provide backwards compatibility. What do you think? Does preserve: "specified"
make sense?
preserve
is just a way to keep original output.
What you are trying to do is a complete hack since it's rely on the fact that all plugins are chained and resolve each of their responsibility (postcss-color-*, postcss-calc, etc..).
I think we can't accept such an option in this plugin since it's beyond its responsibility.
since it's rely on the fact that all plugins are chained
That's the reason I implement the extraction feature in cssnext. From cssnext point of view, relying on plugins being chained is not hack, and the separate project will mostly copy cssnext, but using only postcss plugins that can affect css properties values.
The feature I'm proposing to this plugin is that, comparing to the current preserve behavior:
:root {
--width: 100px;
--height: var(--width);
}
/* compiles to */
:root {
--width: 100px;
--height: 100px;
--height: var(--width);
}
It will do:
:root {
--width: 100px;
--height: var(--width);
}
/* compiles to */
:root {
--width: 100px;
--height: 100px;
}
That is, it will not leave the specified value. Instead, it replaces it with the resolved value. So at cssnext level, I can retrieve all custom properties without getting duplicate values.
I don't think this proposal is a hack. It doesn't rely on other plugins. It simply removes the specified declaration without the conditional as the code base currently does.
Hope that can change your mind.
So what about a simple other option preserveComputedOnly
?
Or maybe:
preserve: true || "all" (current behavior)
preserve: "original" // only preserve original authored values
preserve: "computed" // only preserve computed values
I'm incline towards the later, otherwise we need to make a decision if preserve
overrides preserveComputedOnly
or the other way around.
I was going to suggest "original", but in css spec, they use "specified". So I thought it might be nice to follow suit.
Let me know if you insist.
if specs say specified go for this
preserve: true || "all" (current behavior) preserve: "specified" // only preserve original authored values preserve: "computed" // only preserve computed values
Cool. Glad we have an agreement. :)
Should I start working from the 3.2.0 commit?
Please just remove useless lines in the module BUT keep the tests to ensure we are not going back :) Don't drop any commit, just iterate.
Writing it makes me realize what preserve: "specified"
does is to simply bypass this plugin. (Users only want specified value, so no resolution is needed at all. And since this plugin is bypassed, custom properties in :root
is preserved. Does exactly what the option says.)
Are you ok with this option value?
I think a better option name would be
computed: "add" // add the computed value before the specified value computed: "replace" // replace the specified value with computed value
The thing is you think about declaration. preserve
affect declaration + var() usage.
So indeed this should be moved to another option.
preserve will affect the "computed" option. Keep that in mind.
source
:root {
--color: #aaa;
--bg-color: var(--color);
}
body {
background: var(--bg-color);
}
{preserve: true, computed: "add"}
:root {
--color: #aaa;
--bg-color: #aaa;
--bg-color: var(--color);
}
body {
background: #aaa;
background: var(--bg-color);
}
{preserve: true, computed: "replace"}
:root {
--color: #aaa;
--bg-color: #aaa;
}
body {
background: #aaa;
}
{preserve: false, computed: "add"}
body {
background: #aaa;
background: var(--bg-color);
}
{preserve: false, computed: "replace"}
body {
background: #aaa;
}
I think computed
should be dominant over preserve
, because as can be seen from the examples, {preserve: false, computed: "add"}
doesn't make much sense.
What about we default computed
to replace
, preserve
to false
, when computed
is add
, preserve
is simply ignored and works like true
? This makes the plugin backward compatible and won't generate incorrect css code.
I was thinking computed
was only going to affect custom props definition.
Imo, this option should just not be enabled by default, so no issue for backward compatibility.
Maybe you can simply add an option like computedProperties
that will only handle --* definition ?
If we introduce an option only affects custom props definition:
{preserve: true, computedProperties: true}
:root {
--color: #aaa;
--bg-color: #aaa;
--bg-color: var(--color);
}
body {
background: #aaa;
background: var(--bg-color);
}
{preserve: true, computedProperties: false}
:root {
--color: #aaa;
--bg-color: #aaa;
}
body {
background: #aaa;
background: var(--bg-color);
}
This feels asymmetrical (also the name computedProperties
doesn't feel right in these examples.). If we keep background: var(--bg-color);
so it can override background: #aaa;
in supporting browsers, why don't we keep --bg-color: var(--color);
so it can override --bg-color: #aaa;
?
OK. I was drunk.
Do need a better name though.
What about preserveSpecifiedDefinition
? Doesn't work when preserve
is false
, defaults to true
when preserve
is true
.
I just realized that at cssnext level, i'm actually able to dedup the custom properties definitions (when there are multiple custom properties definitions with the same name, use the last one doesn't contain var()
). So a preserve: true
offer by this plugin is enough to achieve the goal.
But I also realized I need the same preserve feature from the custom media plugin (want to extract custom media queries also.)
Will write a doc to clearly explain the problem and my plan of solving it. Sorry for all the noises I generated.
Please check this out when you got the time. https://gist.github.com/hgl/77f0141eea04817ee0af
I realized a problem the current preserve
option could introduce while writing it.
Let me know what you think. :)
With this variables option:
It would be great if cssnext could give back a variables option that have all values preprocessed:
Otherwise it's very hard to consume the values for other js code.
I wonder what might be the best way to output this option? Should it be handled at cssnext level or this plugin here?