JPeer264 / node-rcs-core

Rename css selectors across all files
MIT License
37 stars 7 forks source link

Attribute selectors not replaced #33

Closed noscript closed 6 years ago

noscript commented 6 years ago

I'm trying to replace some attribute selectors, for example [class*=" cls-"] and [class^=cls-]. Tweaking ignoreAttributeSelectors doesn't make any difference.

The node-rcs-core Bootstrap example looks broken as well. Rule [class*="col-"] is unaffected while all other col-* selectors were replaced.

Before: https://github.com/JPeer264/node-rcs-core/blob/941a636f5aa2e023e97c61ac0f557ebc95d8bd3e/examples/replace/before/bootstrap.css#L903-L908

After: https://github.com/JPeer264/node-rcs-core/blob/941a636f5aa2e023e97c61ac0f557ebc95d8bd3e/examples/replace/after/bootstrap.replaced.css#L903-L908

noscript commented 6 years ago

Sorry, was meant to submit to https://github.com/JPeer264/node-rename-css-selectors but mixed up the browser tabs.

JPeer264 commented 6 years ago

Hi @noscript, thanks for your issue.

In the examples I wanted it this way, as I ignored it as it is just used for .no-gutters. Source: https://github.com/JPeer264/node-rcs-core/blob/2a9cb39a46d8e83b663ff91917a49607be7c2863/examples/replace/index.js#L5-L6

Do you have any code to reproduce this issue? Furthermore, do you have spaces within the attribute selectors: " cls-"? If so, this might be the reason why it does not work, as in this lib I include everything within these quotes.

JPeer264 commented 6 years ago

Btw, it is ok that you posted it here, as this repository is the correct one :+1:

noscript commented 6 years ago

I created a test repo https://github.com/noscript/rcs-attributes-test

There are two dist directories, the difference is whether ignoreAttributeSelector enabled or not.

I have some artificial style.css:

.lst-dot, .lst-dash {
  margin-left: 10px;
}
.board > [class*=" lst-"],
.board > [class^=lst-] {
  padding: 0;
}

With ignoreAttributeSelectors: false there is the first problem, having a single rule with unquoted attribute selector will break the rest of the code:

.r, .i {
  margin-left: 10px;
}
.s > [class*=" lst-"],
.s > [class^=lst-] {
  padding: 0;
}

I'm not sure if such syntax is valid, but some css post-processors will remove the quotes if there was no whitespace. I will make a bug report to the relevant post-processor in that case.


With ignoreAttributeSelectors: true there is a different problem, attribute selector values are not replaced.

.o, .u, .a, .f {
  font-family: monospace;
}
.l > [class*=" msg-"],
.l > [class^="msg-"] {
  padding: 10px;
}

Needless to say that .l > [class*=" msg-"], .l > [class^="msg-"] is a dead code, since there is no more msg-* anywhere else,

My expectation would be that rcs creates a prefix out of such selectors and does something like this:

.xo, .xu, .xa, .xf {
  font-family: monospace;
}
.l > [class*=" x"],
.l > [class^="x"] {
  padding: 10px;
}

However I'm not sure if that is easily achievable.


Furthermore, do you have spaces within the attribute selectors: " cls-"?

Yes, there is a space on purpose. Here is why https://stackoverflow.com/a/8588532/1005230

JPeer264 commented 6 years ago

Thanks for this repo. I am investigating on your issue now.

JPeer264 commented 6 years ago

Ok, the bug is the missing quote.

I'm not sure if such syntax is valid

It is still valid, but I forgot to think about that case. I'll make a fix

Needless to say that .l > [class=" msg-"], .l > [class^="msg-"] is a dead code, since there is no more msg- anywhere else

That is true, but I wanted to achieve that with ignoreAttributeSelectors. Maybe it is a bad option, as it produces, as you figured, dead code. I will also try to achieve your expectation out of the box.

JPeer264 commented 6 years ago

@noscript I got this bug removed in rcs-core@1.0.5 and in rename-css-selectors@1.3.5 - if you update you should no longer facing this issue.

Regarding to your expectation for the attribute selectors. I will open another issue for that.