search

thysultan / stylis

light – weight css preprocessor
https://stylis.js.org
MIT License
1.71k stars 82 forks source link

Broken auto prefixing #330

Open victordidenko opened 1 month ago

victordidenko commented 1 month ago

Hello!

I'm using Linaria, which uses Stylis under the hood for auto prefixing. Linaria is a zero-runtime CSS in JS, works by transforming it to CSS variables. CSS variables are generated.

After some updates my build become broken, it generates line like:

-webkit-box-pack:var(--b10wbfel-0);-mjustify;-webkit-justify-content:var(--b10wbfel-0);justify-content:var(--b10wbfel-0);

note that -mjustify; which is incorrect.

After hours of investigating I've narrowed issue to Styles, exactly this lines https://github.com/thysultan/stylis/blob/0990417cefe8a4d01115613ccf44f5ae86040cd5/src/Prefixer.js#L78-L79

First (inner) replace generates string

-webkit-box-pack:var(--b10wbfel-0);-ms-flex-pack:var(--b10wbfel-0);

Second (outer) replace generates this broken line

-webkit-box-pack:var(--b10wbfel-0);-mjustify;-webkit-justify-content:var(--b10wbfel-0);justify-content:var(--b10wbfel-0);

because regular expression /s.+-b[^;]+/ (which I assume should match space-between value) matches s-flex-pack:var(--b part of first string.

victordidenko commented 1 month ago

I've found an option variableNameSlug in Linaria, and set it to '_[componentSlug]_[index]', so variable names always starts with underscore. But this is hideous ._. I've spent eight hours to fix this, and this fix looks very unintuitive, so I have to write a explaining comment as well next to it.

I think this library should take into account, that there can be variables in place of values, with any variable name, and they should not match regular expressions.

Andarist commented 1 month ago

I think this library should take into account, that there can be variables in place of values, with any variable name, and they should not match regular expressions.

PRs are welcome.

victordidenko commented 1 month ago

PRs are welcome.

Of course, if I were fluent in CSS and differences in browser ._.

This library's code looks like maintainers put a significant efforts into its minimal size, and it is not always obvious what was ment by one thing or another. Regarding this issue, it is only my guess, that regular expression /s.+-b[^;]+/ ment to match space-between, but I'm not 100% sure. There are no any comments in the code, no any explanation and so on.

I dare say that this library is not very friendly for external contributors, who are unfamiliar with existing code base and chosen solutions.

victordidenko commented 1 month ago

I apologize, if my messages may looks like demands or requirements. They are not :) I just maybe a bit frustrated, but mostly on Linaria library, whose developers have chosen Stylis without proper investigation and without opt-in way to disable vendor prefixing. I'm sure this library is great, as well as Linaria, they maybe just don't fit each other completely, with a way Linaria works. And of course I understand that open source is not paid work.

Andarist commented 1 month ago

Well, Stylis is a popular choice for CSS-in-JS libraries. A dependent can't ever predict all the interactions with its dependencies so I wouldn't blame Linaria maintainer here for not doing "a proper investigation". This looks like a bug and nothing more than that - those will always happen in the software.

And of course I understand that open source is not paid work.

Yeah, this ☝️ I don't have great incentives to work on fixing this right now - my mind is just elsewhere. If your company cares about fixing this they could sponsor Sultan/me/Stylis to get this job done.