RCS replaces all words that are the same as the selectors

[jpeer264]

RCS replaces all words that are the same as the selectors

[JPeer264]

Alright. I the next version 4+ a lot of bugs will be fixed. Yours should be fixed as well. To be sure I will add a test case specific to your problem ;)

[JPeer264]

First of all the HTML problem is resolved by updating rcs-core (ref: JPeer264/node-rcs-core#52 JPeer264/node-rcs-core#56)

The JavaScript problem is following. I haven't found a good way of determining if a string is used for selectors or just as text. I came up with detecting non word characters in the text (in regex \W) and if some got found no renaming happens on this text. You might have another or additional idea to this problem?

[Stephen-X]

Hi @JPeer264 thanks for following up! I think a simple way to deal with the JS problem would be to only find and replace word matches with a prefix of "." or "#" in all strings. Do you think that's sufficient enough?

[JPeer264]

Hm no that wouldn't be enough, as e.g. bootstrap is storing the selectors as plain strings sometimes: bootstrap.js#L405-L409, or jQuery has functions such as addClass, where selectors don't have any prefixes.

[Stephen-X]

Hmm you're right, there's no easy way to distinguish class names accurately without missing something or overkilling. There're three more complicated ways I can think of that may work though:

1. Allow users to wrap all CSS classes in JS and HTML inside something like css() during development, and they'll be automatically removed, selectors being renamed or not (since some of them might be excluded), during output. A simple regex of css\((.*?)\) should do the job. A downside of this method is that it adds RCS as a dependency, but we can probably add an -remove-css-wrapper option that simply outputs JS and HTML with css() wrapper removed so that those who no longer require RCS in their projects can sanitize their codes.

2. Make replacing an (optional) interactive process. RCS will act as greedy as possible (simply treat JS and HTML as a list of strings and try to find and replace all matches), and users will be able to confirm each replacement. This works better on small projects.

3. RCS will act as conservative as possible (as stated previously, it will only search for matches that have a prefix of "." or "#"), but users can add an include section in .rcsrc where they can put names of functions and objects in which they require replacing all occurrences.

[JPeer264]

Ya, those are good approaches and already had the same or similar approaches. In total I am more a fan of automation and less configuration. Here are my thoughts:

1. This was the first idea I came up with in the beginning of the project. There is already a similar way in rcs to achieve such a behavior. The mapping file was the solution. As the mapping file is a JSON you can simply add following in your code:

   const map = require('./mapping.json'); // or anything to load this JSON in browsers
   const myClass = map['.my-selector'];

   But the downside of this is, that it doesn't really save disk space, which is actually my goal of this lib.

   The downside of your approach (what I think) is, that the users (probably me as well) might forget about the wrapper and then the entire app won't work as expected. Which is also the same case with the mapping file.

2. This would be one of the better solutions, but I am a big fan of automation and having an interactive process might be really annoying over some time, especially when the code changes a lot. Which might be the case on smaller projects.

3. I think this would work the best with a low missing rate. But this is also a lot of configuration, especially when having a lot of objects and functions.

[Stephen-X]

I took a look at other selector obfuscation solutions such as Google Closure Tools, it would seem that it enforces users to mark CSS classes with goog.getCssName(): https://github.com/google/closure-stylesheets#renaming, https://github.com/google/closure-stylesheets/wiki/More-on-CSS-Renaming, so yeah, option 1 seems like a preferred solution: user should clearly mark CSS class names that they want RCS to process, otherwise RCS should just ignore them. This is a bit cumbersome (I agree that I myself might forget about the wrapper sometimes), but I think this might be the best balance between automation and code safety (it would be easier to debug the generated code by checking if you forgot to wrap a class name somewhere). The current solution (just replace all occurrences) is simply not safe enough to be used in production environment, as it could create unexpected outcomes that are hard to spot (such as changing the display content).

I'm not sure the use of mapping file is a good solution though. For starters, as you mentioned, the JS code will then contain a lot of redundant codes. The reference to the automatically generated mapping.json adds an unnecessary and fragile dependency to the source code, and will also add to the number of round trips made by browsers.

[JPeer264]

I totally agree that it is risky in production.

I guess this could work as you suggested in 1., replacing the code of rcs.css() with the selectors. In HTML however, just class and id are triggered, so adding those functions in JS are enough I guess.

However, your added documentation of the closure tools gave me an idea. I would like to add options for triggering classes in JS (so if I use another library, such as bootstrap, I am still able to rename those classes in an experimental way). I think following options for replacing selectors in JS would be neat:

• experimental - the current way
• closure (or similar) default - your suggested way of replacing all rcs.css(), without prefix # or ., or rcs.cssClass() | rcs.cssId(), which adds the prefix

I guess that works right?

[JPeer264]

Btw, the original issue with your HTML should be fixed in v3.0.0

[Stephen-X]

It's possible that if devs develop their own elements for styling, those won't be renamed? But yeah, only triggered by class and id seems enough already, at least for me :)

Adding an indirection layer like the Google Closure Tools did does seem like a better solution! In this way the css() marker is no longer just a piece of invalid marker code, and the original source code can run with RCS import. They can all be removed in the final output.

[JPeer264]

No those elements are not yet removed. But this is actually a good feature. As I use Parse5 for HTML and PostCSS for AST it is not a big deal. Custom elements in HTML are easy to find, as it is well structured and reliable (maybe I am also missing something).

Totally, I am also happy with that solution. It will just take some time I guess until this is implemented, as I also have to think about a good structure.

[klimashkin]

With css-modules I don't have such problem, because in my case all class names are generated in that way '[name]_[local]', like Button_primary.

If you want to add some css('classname'), make it optional please, because currently everything works fine for me : )

[JPeer264]

@klimashkin good point. Then it is also not a breaking change 👍

[JPeer264]

Hey @Stephen-X .. I won't have much time the next few weeks. The implementation of this feature will take some time, I hope you'll understand.

[Stephen-X]

@JPeer264 No worries, take your time. I mostly use this utilities for personal projects 😄 Thanks for the great works so far!

[emilemil1]

Would it be safe to assume that if a string (after concatenation) contains any word that isn't a CSS selector, then every word in that string should be excluded from renaming? I can't personally think of any situation where you pair selector names with other random text, except possibly in some error description.

That would still miss a lot of cases, but it would solve the initial example and quite a few others.

[JPeer264]

I am not sure if I understood it correctly, but I assume you meant something like following: Let's say we have two string in JavaScript, one with CSS selectors and one with normal string:

const myString = 'Any string of my selector';
const mySelectors = 'selector another-selector';

And I have my CSS files:

.selector {}

.another-selector {}

In the variable myString there are Any, string, of and my included which are not defined in my CSS, so therefor this whole string can be excluded from renaming (which means selector in myString won't get minified, according to the other words).

If this is what you meant, then what about selectors which are not defined in any CSS, something like "JavaScript only selectors", such as #js-slider or .js-triggered-by-javascript. Then these would get trigger the logic when a whole string won't get minified, right?

[emilemil1]

Indeed, that's a destructive case I didn't consider. The aforementioned manual workarounds of adding a prefix/wrapper or similar would be needed to mark any JS-only selectors.

[X-Ryl669]

For me, in JS you shouldn't deal with CSS's class or ID everywhere. Typically, it'll boils down to querySelector/getElementByID/getElementsByTagName/addClass/removeClass/toggleClass...

The list is somehow sparse. Unluckily, you need to compute the AST for the complete JS script to figure out if whatever string is being used for such methods. I wonder if you can compile the complete JS code (with all modules), compile to AST, track the methods using some string, and mark/flag such strings for renaming (using the source map ?).
Obviously, code like var sel = 'mySelector'+i; whatever.toggleClass(sel); might not be renamed correctly, but it'll be detected as a candidate (sel is a variable that's used by toggleClass that's been modified in the AST). In that case, the code could (either):

1. Easier: Fill the exclusion list with mySelector (so it's not renamed), and output a warning on the console.
2. Create a new unmapped.json file storing the source line where such construct is detected and let the developer change his code.
3. Harder: Make sure the mapping is done in 2 steps (mySelector prefix is renamed to abc but anything after mySelector is kept unmodified, so a css rule .mySelector0 or .mySelector_down is renamed to .abc0 or .abc_down)

[JPeer264]

The only bad thing is that there are a lot of modules which needs selectors, so everytime a new module is imported, which needs selectors, it must be included into some list of available "replaceable" functions.

Those combined selectors are really hard to detect, as I already mentioned in here: JPeer264/node-rcs-core#85 (comment)

1. This might be the best solution I guess. You mean to output a console.log when there would be a potential selector?
2. That would be also a good hint to tell the developer such things are happening
3. Hm I think that cannot work like this as I don't know if the prefix is the correct class or the appendix. So it could also be that some developers make it the other way arround: .0mySelector or .down_mySelector. In this case I would rather disallow every change where such cases appear and would go for 1. and 2.

[X-Ryl669]

I guess with all the changes in rcs-core#95, this should be somehow fixed. At least, I had the same issues before and now it's working correctly on my project which is fairly large with many tricks (like Template, Id/Class mismatch, HTML using <b> and <i>, JS code making string from ID + iterator and so on.

Hopefully, you'll be able to test soon!

[waterplea]

Just tried this tool and ran into similar issue. I have error class used in my html, in js I have a constant error.password.invalid as an error code from backend. I get error in JS renamed :(

[JPeer264]

@waterplea which version do you use?

[JPeer264]

You could also exclude error globally for such things:

import rcs from 'rcs-core';

// following methods also allow array of strings and RegExp
rcs.selectorLibrary.setExclude('error'); 
// or rcs.selectorsLibrary.setExclude (if you use rcs-core@3+)

[waterplea]

I used the latest. Thanks for the suggestion, in my case I ended up with no need to process js, so excluded it.

[waterplea]

Hi @JPeer264, I've found another error. Now I have this case - I used this error key in data attribute and it got minified:
<div data-error="error.password.invalid">
And in CSS I have [data-error="error.password.invalid"]:before and it got minified to [data-error="error.ts.to"]. Your suggestion helped but it seems like even though I use 3+ and typings suggest selectorsLibrary with "S" - actual code is already without "S" on 3.3.0

[JPeer264]

Alright, thanks for the update, I will add this to my tests.

I think it is kinda confusing right now with the versions. Following should be true:

• rename-css-selectors version: 3.3.0
• rcs-core version: 2.6.3

I think you have installed [email protected]

I guess it will be better for the future to have a monorepo and namespace everything, that all versions match across the packages; but this is now another topic.

[heychazza]

I'm having

Hi @JPeer264, I've found another error. Now I have this case - I used this error key in data attribute and it got minified:
<div data-error="error.password.invalid">
And in CSS I have [data-error="error.password.invalid"]:before and it got minified to [data-error="error.ts.to"]. Your suggestion helped but it seems like even though I use 3+ and typings suggest selectorsLibrary with "S" - actual code is already without "S" on 3.3.0

Having a similar issue, but with EJS tags <%= some.thing %> any way to exclude these with regex even?