sylvainpolletvillard / postcss-grid-kiss

A PostCSS plugin to keep CSS grids stupidly simple
https://sylvainpolletvillard.github.io/grid-kiss-playground/index.html
MIT License
1.32k stars 53 forks source link

Support native CSS custom properties/variables directly for dimensions #18

Closed Monkatraz closed 3 years ago

Monkatraz commented 3 years ago

Currently, you can input something like:

"+-----------------------------+"
"|                             |"
"+-----------------------------+"
"| calc(var(--my-cool-length)) |"

to get CSS properties to work. This is pretty clumsy, and verbose, but it does work just fine.

Which probably means a syntax like:

"+-----------------------+"
"|                       |"
"+-----------------------+"
"| var(--my-cool-length) |"

// or:
"+---------------------+"
"|                     |"
"+---------------------+"
"| v(--my-cool-length) |"

// my favorite:
"+-------------------+"
"|                   |"
"+-------------------+"
"| v(my-cool-length) |"

should technically work without any issues. I did take a look at the dimension parser and it looks like basically a copy paste of the calc regex. It would probably be fine to enforce one var input pattern. This would be nice feature to have!

There is the matter of Sass, Less, and other non-persistent variables (which are using @ and '$', usually). I also figure these wouldn't be too difficult to add.

One thing you could potentially do to avoid all these issues is to simply allow an escaped dimension, maybe like:

"+----------------+"
"|                |"
"+----------------+"
"| /$cool-length/ |"

// or:
"+----------------+"
"|                |"
"+----------------+"
"| ($cool-length) |"

// or, perhaps resembling 'interpolation' the most:
"+----------------+"
"|                |"
"+----------------+"
"| {$cool-length} |"

That way, custom functions, variables, etc. would be easily supported.

sylvainpolletvillard commented 3 years ago

Hello, thanks for the suggestion ! Added in v2.4.0

No fancy syntax, just the standard var(--name)

I won't support Sass or Less which have not been designed to work along with PostCSS.

Monkatraz commented 3 years ago

Thanks for quick response and quick addition!

Also, I suppose fair enough for the $ and @ variables.

sylvainpolletvillard commented 3 years ago

if you want a more complete response about why I won't propose official support for sass-like variables: these syntaxes are used by several other tools with different behaviours. For example, just for PostCSS we have these three that use $var syntax : https://github.com/postcss/postcss-simple-vars https://github.com/jedmao/postcss-nested-vars https://github.com/jonathantneal/postcss-advanced-variables

The only thing I can possibly do is to pass any $variable as is in the output and hope that another plugin in the chain will be able to use it. But I can't promise that it will work with any of these tools

Monkatraz commented 3 years ago

Totally fair - the only reason I'd want to use them for this plugin is simply for shorthands. Like:

--layout-body-max-width: 45em
$l-body-mw: var(--layout-body-max-width)

Which fits in the row or especially column dimension zones a lot easier. You can do var($alias) now, as long as Sass variables are processed after grid-kiss does its thing. For what I was wanting, that's good enough. var($) isn't that many characters, and it's fairly clear what that dimension turns into when someone else is reading the source.

About compatibility: I suppose my philosophy is that when using something like PostCSS is that you should be aware of what exactly your tool is doing. If you feed in incompatible garbage and a plugin breaks that's your fault for not reading the documentation, or for being unaware of what exactly your plugins have to do to in order to output valid CSS.

I have a big plugin chain and I made sure to make things compatible, but trusting end users (even devs) is probably a bad idea, so I totally get it lmao

Monkatraz commented 3 years ago

OK actually, commenting again because I was wrong. This function in the dimension parser:

function cleanupDimInput(input){
  return input
    .replace(/[^a-zA-Z0-9.()\-+/*\s%,<>]/g, "") // remove anything that is not part of a dimension value
    .replace(/^[-+\s]+|[-+\s]+$/g, "") // remove remaining '-' '+' segments but preserve range dimensions
}

is ran against dimensions. This function will remove both $ and @ from dimensions, meaning that Sass or Less variables simply do not work at all anywhere within a grid-kiss string. Replacing the value inside of the string before its processed will just be inherently problematic and likely to fail in hilarious and catastrophic ways.

Considering your view on PostCSS's scattered and janky static variables, I'm not sure if you want to actually change this - your call. I already have a (local) solution - so it's not a big problem for me anyways.

sylvainpolletvillard commented 3 years ago

I can change this function behaviour to keep $ characters. This will prevent any future usecase for a dimension that use $, but I think it is unlikely to happen in future versions of CSS due to the popularity of the $var syntax in the world of CSS preprocessors.

For @var however, I'm more concerned because @keywords are already a thing in CSS

Monkatraz commented 3 years ago

Personally, I like the {stuff} (or whatever syntax) for escaping values. They would be most flexible treated as a length I suspect - but they don't have to be. Using a special escaping syntax makes it so that in the source and in the docs it's clear what it's doing, because big scary interpolation brackets implies custom behavior.

However, your fix is simple and easy, and will probably solve 90% of use cases. The only missed use cases I can think of is when using JS custom functions, like say pow() or whatever. These can be pretty handy.

sylvainpolletvillard commented 3 years ago

Inventing another syntax for escaping leads to the same problems ; brackets and antislash may already be used by something else, for example if we add these rules through a template string in JavaScript or in a Mustache template system.

I might consider adding a dimensionParser option next to the existing selectionParser that let you provide a custom function to parse dimensions value. Yet I'm not fully convinced that this is actually needed. Do you have any example of the 10% use cases I would miss after preserving $ characters ?

Monkatraz commented 3 years ago

EDIT: As I wrote this I realized that Sass variables are just too much of a mess for compatibility, and I had to edit this a lot - so sorry if this is hard to follow

Usage of Sass variables requires that they be processed:

Sass variables have to be processed before nesting, which means grid-kiss has to as well. grid-kiss is technically compatible with nesting, but it doesn't output nested selectors. I don't think that specifically matters for dimensions as far as I can tell? Regardless, grid-kiss (for me) has not handled nested at-rules at all, and to get my own plugin chain to be reliable I had to make sure grid-kiss only ran after nesting. Which basically means you can't use Sass variables without sacrificing like, one of the main points of their existence. Not sure if it's even worth supporting, to be honest. Compatibility with nesting would have to be basically... idk, "native."


Alright, without Sass variables, if we want static the next best thing is probably custom functions, and that was the 10% I was wanting to talk about anyways. If I had defined a custom function, say vr($number), I would not be able to use it like:

"+----------+"
"|          |"
"+----------+"
"| vr(1234) |"

However, I believe this works even right now:

"+----------------+"
"|                |"
"+----------------+"
"| calc(vr(1234)) |"

For me, these custom functions are usually shortening a really long, parameterized calc expression into something palatable. A grid-kiss string could get really mangled if you're forced to input a long string into a dimension zone.

What's neat about custom functions is that they allow for basically anything you'd want Sass variables for most likely, and it even allows you to do proper escaping. Here is my idea: Instead of checking for calc( ) and var( ) separately, just look for anything that looks like a single function (like foo() by itself). This by itself I think this is a lot better, because this would naturally support custom functions, calc, var, and probably anything else that might be added to CSS in the future. In order to not break something, you would have to avoid filtering any text inside a ( ) block.

The only edge case to this that I can think of is the foo(bar)px situation, which I have seen done a few times. It's good for math functions - like pow(5, 2)px or something akin to that.

By not filtering inside. you can do any escaped text by kind of brute forcing it - a user could create a esc($string) function and make that function literally just do nothing but spit out whatever was fed into the $string argument.

To add escaped text more naturally, you could maybe make the function detection be valid for functions with empty names, like ( stuff ). This would unfortunately add a special case to the function parser, as it would have to spit out the text content inside of the ( ) block rather than just returning the whole string like it normally does. Probably not worth it, but it's a neat idea.

sylvainpolletvillard commented 3 years ago

We moved from variables to functions. We can go over many custom non-standard CSS syntaxes that could be used inside a grid-kiss dimension, even some that conflicts with already existing custom syntax in grid-kiss.

This can be reduced to the question of what to do with unrecognized syntax in dimensions. Passing everything as-is without any cleaning is not possible, first because we already do some cleaning stuff for characters that are here for the purpose of grid readability (typically | - / \), and secondly because it would prevent any future addition of custom syntax for grid-kiss that could break another custom syntax used by a specific user (except maybe if we push new major version with breaking change notices, but it's not really convenient)

So the only way to enable these custom syntaxes would be to add another dimensionParser option that let you opt out of this cleaning process, and add your own transforms when unrecognized syntax is found in dimensions. This should meet your needs without compromising the maintainability and scalability of grid-kiss in the future.

Monkatraz commented 3 years ago

Providing it through the grid-kiss options is probably best, and it easily allows for neat syntax to be added without any hassle.

I do wanna make clear I'm just spouting off ideas - as it stands the plugin can now do whatever good ol' CSS can natively do for lengths and dimensions, so it'd be fine to leave the plugin alone even.

sylvainpolletvillard commented 3 years ago

Fine, I went with the minimal clean-up / dimensionParser option approach. Just published v2.5.0

This should enable custom vars and functions. See https://github.com/sylvainpolletvillard/postcss-grid-kiss/blob/master/test.js#L54

It might break layouts for some users that have exotic characters in their dimensions that grid-kiss used to clean up for them before. But since it has never been documented, this is not technically a breaking change, and hopefully these users will be able to quickly figure out what's going on.