Fixes #3023: Add style support for css selectors type [foo~=bar].

[mormubis]

It was a problem with css separators. It was just looking for ~ as separator but there is a possibility to use ~ as part of an attribute css selector.

element[myattribute~=partial] {
    // some visuals
}

Edit: http://www.w3.org/TR/css3-selectors/

[googlebot]

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.

---

• If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
• If you signed the CLA as a corporation, please let us know the company's name.

[mormubis]

I signed it!

[googlebot]

CLAs look good, thanks!

[samccone]

@adelarosab would you mind adding a test for this functionality?

@nazar-pc would you mind reviewing this?

Thanks!!!! ✨

[mormubis]

Yeah, you caught me adding test for this.

[mormubis]

I'm having a problem with my nodejs version. I cannot test in my local machine.

Sorry.

[nazar-pc]

LGTM 👍

[TimvdLippe]

Correct me if I am wrong, but I think this breaks the following cases:

.foo[bar=baz] {
}
.foo[bar=baz] > * {
}

[TimvdLippe]

This regex seems to catch this edge-case:

(^|[\s>+~]+)([^\s>+~=\[]*(?:\[.+?\])?)

E.g. A CSS seperator followed by anything that is not a seperator followed by (optionally) a selector between squared brackets.

[mormubis]

Hi @JeremybellEU,

Nope, it doesn't break anything. You can test exactly same case you did with the jsbin from #3023.

Regards

Edit: Your first screenshot shows regexp is fine. I think what we are looking with this regexp is try to find split points. As you can see in the screenshot, you see 2 "selectors" which is okay.

Your regexp is totally accurate but I guess they've chosen to simplify it for performance issues.

[TimvdLippe]

Hm I checked your PR locally and I am indeed unable to reproduce what I expected.

When I output the value of m at https://github.com/adelarosab/polymer/blob/master/src/lib/style-transformer.html#L165 for the test-case

:host[attribute="true"] {
  background: white;
}

the console output is :host[attribute as expected by the updated regex. However, this does not seem to effect the output of the CSS.

After some more checking, it replaces :host[attribute with my-element[attribute and then substitutes this back into the selector. Therefore the case is fixed, but the regex does not parse everything. If the regex is not supposed to parse everything, this is fine. If it however should parse everything, then the update breaks the existing selectors.

[mormubis]

Hi @JeremybellEU,

I haven't made this code. I'm not 100% sure if the purpose of regexp was match everything. The only one who can answer that question is @sorvell.

I suppose @samccone or @nazar-pc, could bring light to this.

I dare to guess you are right. The code still works because they analyse selectors just for host replacement purposes.

Edit: One more correct regexp would be (^|[\s>+~]+)((?:\[.+?\]|[^\s>+~=\[])+) avoiding matches in empty styles.

Edit2: I'm not sure what happens with the performance once this regexp is added. @samccone, is there a stress test?

[dfreedm]

This change seems ok to me, but @sorvell should really take a look

[sorvell]

I don't think this regex change will be sufficient. The problem is that ~ has 2 meanings in css: it's a sibling combinator and part of an attribute selector [attr~=value]. The regex needs to match the first case and not the second.

We also don't have a sibling combinator test or this change would have made it fail! So we need to add a test for this as well.

Sorry for the delay. If you're still willing to work on this that'd be great. Otherwise we'll close this PR and put #3023 back in our queue.

[mormubis]

I'll do it ;)

Also i'm worry about performance because the regex we are looking for increase complexity a lot (^|[\s>+~]+)((?:\[.+?\]|[^\s>+~=\[])+), I think.

[nazar-pc]

Well, talking about performance it is better to write some simple benchmark to measure what difference we are talking about. And how it affects component's performance in general.

[sorvell]

Fixed via #3413. Thanks!