Skip to content
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

Broken auto prefixing #330

Open
victordidenko opened this issue May 6, 2024 · 5 comments
Open

Broken auto prefixing #330

victordidenko opened this issue May 6, 2024 · 5 comments
Assignees

Comments

@victordidenko
Copy link

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

stylis/src/Prefixer.js

Lines 78 to 79 in 0990417

case 4968:
return replace(replace(value, /(.+:)(flex-)?(.*)/, WEBKIT + 'box-pack:$3' + MS + 'flex-pack:$3'), /s.+-b[^;]+/, 'justify') + WEBKIT + value + value

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
Copy link
Author

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
Copy link
Collaborator

Andarist commented May 7, 2024

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
Copy link
Author

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
Copy link
Author

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
Copy link
Collaborator

Andarist commented May 7, 2024

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.

@thysultan thysultan self-assigned this Jun 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants