thysultan / stylis

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

Separate prefixed pseudos in nested rules to avoid CSSOM injection failures #315

Closed quantizor closed 1 year ago

quantizor commented 1 year ago

Based on my understanding of rulesheet, I think the @media {::placeholder{}} combo should be split into separate rules.

quantizor commented 1 year ago

@thysultan if you have any pointers for solving this, I'd be happy to try. Just haven't had much luck so far.

Andarist commented 1 year ago

I think that the fix would have to be applied here and somehow you'd have to recursively copy the parent rules through their .root (if there are any parent rules) and put that single prefixed rule as the "leaf" .children

Andarist commented 1 year ago

Almost managed to fix it - there is just one failing test case: https://github.com/thysultan/stylis/blob/5687d854384bb0285130581e951a1d620128f63f/test/Middleware.js#L84-L87

This test tests the output and not what is passed to rulesheet (rulesheet is just a plugin), values passed to rulesheet (I tested this manually) are OK.

I'm not sure how to resolve this conflict because prefixing of those pseudo rules happens transitively here: https://github.com/thysultan/stylis/blob/5687d854384bb0285130581e951a1d620128f63f/src/Serializer.js#L35

As we can see - this serializes children and uses that as part of the value. This happens when stringifying @media rule and the serialized children land within { ... }. The problem is that if we return the prefixed stuff here: https://github.com/thysultan/stylis/blob/5687d854384bb0285130581e951a1d620128f63f/src/Middleware.js#L57 then we end up with outputs like:

@media (min-width: 500px){a:-moz-read-only{color:red;}}
@media (min-width: 500px){@media (min-width: 500px){a:-moz-read-only{color:red;}}a:read-only{color:red;}}

We can see here that the prefixed and stringified top-level rule becomes part of the children of that top-level rule which is not what we want to achieve here.

This problem originates in the assumption that the prefixed things always become siblings of what they "cloned" but that's no longer true.

cc @thysultan

quantizor commented 1 year ago

Ping @thysultan, could you spare some time to look at this one? Thank you!

thysultan commented 1 year ago

Yes, i'll take a look at this, this weekend.

thysultan commented 1 year ago

@Andarist So the latest commit fixes the issue, but the order of rules are: changed to unprefixed then prefix for ::placeholder.

thysultan commented 1 year ago

Ok, i think i managed to preserve the prefix-first order now.

quantizor commented 1 year ago

Thank you @thysultan, your code wizardry is appreciated as always

joacub commented 11 months ago

this changes cause this error:

image
Andarist commented 11 months ago

Please share a repro case