Feat: setInclude | warn on already excluded values (#121)

.travis.yml

@@ -2,7 +2,6 @@ language: node_js
 sudo: true
 os:
   - linux
-  - osx
 node_js:
   - 10
   - 12

README.md

@@ -15,6 +15,12 @@ Some live projects:
 - Analyse ([https://analyse.org/](https://analyse.org/))
 - My personal webpage ([https://jpeer.at/](https://jpeer.at/))
 
+## Caveats
+
+Correctly using `rcs-core` or any of its [plugins](#plugins) on large project means few rules should be followed.
+
+[This document](docs/caveats.md) explains most of them.
+
 ## Installation
 ```sh
 $ npm install --save rcs-core
@@ -26,13 +32,17 @@ $ yarn add rcs-core
 
 ## Usage
 
+> **Note** couple of selectors are [excluded by default](./lib/helpers/excludeList.ts). You can activate them by using `.setInclude` **before** you fill the library
+
 1. Fill your library with all selectors (we assume there is just one CSS file)
 
 ```js
-rcs.fillLibraries(fs.readFileSync('./src/styles.css', 'utf8'));
-
 // excluding specific selectors
 rcs.selectorsLibrary.setExclude('selector-to-ignore');
+// include specific selectors which has been ignored by default
+rcs.selectorsLibrary.setInclude('center');
+
+rcs.fillLibraries(fs.readFileSync('./src/styles.css', 'utf8'));
 ```
 
 2. Optimize the selectors compression (optional)
@@ -50,6 +60,9 @@ const css = rcs.replace.css(fs.readFileSync('./src/styles.css', 'utf8'));
 const js = rcs.replace.js(fs.readFileSync('./src/App.js', 'utf8'));
 const html = rcs.replace.html(fs.readFileSync('./src/index.html', 'utf8'));
 
+// output some warnings which has been stacked through the process
+rcs.warnings.warn();
+
 fs.writeFileSync('./dist/styles.css', css);
 fs.writeFileSync('./dist/App.js', js);
 fs.writeFileSync('./dist/index.html', html);

__tests__/allWarnings.ts

@@ -16,8 +16,8 @@ it('create some warning without sources', () => {
 
   rcs.baseLibrary.get('a');
 
-  expect(Object.keys(rcs.warnings.warningArray).length).toBe(1);
-  expect(rcs.warnings.warningArray.a).toBeTruthy();
+  expect(Object.keys(rcs.warnings.list.compressed).length).toBe(1);
+  expect(rcs.warnings.list.compressed.a).toBeTruthy();
 
   rcs.warnings.warn();
 });
@@ -33,9 +33,9 @@ it('create some warning with sources', () => {
   rcs.baseLibrary.get('a');
   rcs.baseLibrary.get('a', { source: { file: 'nofile', line: 1, text: 'a' } });
 
-  expect(Object.keys(rcs.warnings.warningArray).length).toBe(1);
-  expect(rcs.warnings.warningArray.a[0].file).toBe('nofile');
-  expect(rcs.warnings.warningArray.a[0].line).toBe(1);
+  expect(Object.keys(rcs.warnings.list.compressed).length).toBe(1);
+  expect(rcs.warnings.list.compressed.a[0].file).toBe('nofile');
+  expect(rcs.warnings.list.compressed.a[0].line).toBe(1);
 
   rcs.warnings.warn();
 });
@@ -52,9 +52,23 @@ it('create some warning with common sources', () => {
   rcs.baseLibrary.get('a', { source: { file: 'nofile', line: 1, text: 'a'.repeat(501) } });
   rcs.baseLibrary.get('a', { source: { file: 'nofile', line: 1, text: 'a'.repeat(501) } });
 
-  expect(Object.keys(rcs.warnings.warningArray).length).toBe(1);
-  expect(rcs.warnings.warningArray.a[0].file).toBe('nofile');
-  expect(rcs.warnings.warningArray.a[0].line).toBe(1);
+  expect(Object.keys(rcs.warnings.list.compressed).length).toBe(1);
+  expect(rcs.warnings.list.compressed.a[0].file).toBe('nofile');
+  expect(rcs.warnings.list.compressed.a[0].line).toBe(1);
+
+  rcs.warnings.warn();
+});
+
+it('create some warning when selector has been ignored', () => {
+  rcs.baseLibrary.setExclude('move');
+  rcs.baseLibrary.set('move', 'a');
+
+  rcs.baseLibrary.get('move');
+  rcs.baseLibrary.get('move', { source: { file: 'nofile', line: 1, text: 'move' } });
+
+  expect(Object.keys(rcs.warnings.list.ignoredFound).length).toBe(1);
+  expect(rcs.warnings.list.ignoredFound.move[0].file).toBe('nofile');
+  expect(rcs.warnings.list.ignoredFound.move[0].line).toBe(1);
 
   rcs.warnings.warn();
 });

__tests__/baseLibrary.ts

@@ -164,6 +164,14 @@ it('setExclude | should enable excludes', () => {
   expect(rcs.baseLibrary.excludes.length).toBe(2);
 });
 
+it('setInclude | should enable includes', () => {
+  rcs.baseLibrary.setExclude('one-value');
+  rcs.baseLibrary.setInclude('second-value');
+
+  expect(rcs.baseLibrary.excludes.length).toBe(1);
+  expect(rcs.baseLibrary.includes.length).toBe(1);
+});
+
 /* ********** */
 /* ISEXCLUDED */
 /* ********** */

__tests__/mapping/load.ts

@@ -96,3 +96,19 @@ test('should generate attribute selectors mapping', () => {
   expect(rcs.selectorsLibrary.get('.a-test')).toBe('a');
   expect(rcs.selectorsLibrary.get('#a-selector')).toBe('tsen');
 });
+
+test('should include center (#119)', () => {
+  rcs.selectorsLibrary.setInclude('center');
+
+  rcs.mapping.load({
+    selectors: {
+      '.pv5-m': 'py-4',
+      '.w-100-m': 'w-full-m',
+      '.center': 'mx-auto',
+    },
+  });
+
+  expect(rcs.selectorsLibrary.get('.pv5-m')).toBe('py-4');
+  expect(rcs.selectorsLibrary.get('.w-100-m')).toBe('w-full-m');
+  expect(rcs.selectorsLibrary.get('.center')).toBe('mx-auto');
+});

__tests__/selectorsLibrary.ts

@@ -125,6 +125,67 @@ it('get | should not get excluded selector', () => {
   expect(testSelector).toBe('test');
 });
 
+it('get | should include already excluded selector', () => {
+  setSelectors();
+
+  rcs.selectorsLibrary.set('center');
+  rcs.selectorsLibrary.setExclude('test');
+
+  const ignoredSelector = rcs.selectorsLibrary.get('.test');
+
+  rcs.selectorsLibrary.setInclude('test');
+
+  const includedSelector = rcs.selectorsLibrary.get('.test');
+
+  expect(ignoredSelector).toBe('.test');
+  expect(includedSelector).toBe('a');
+});
+
+it('get | should include already pre excluded types', () => {
+  rcs.selectorsLibrary.set('.center');
+
+  const ignoredSelector = rcs.selectorsLibrary.get('.center');
+
+  rcs.selectorsLibrary.setInclude('center');
+  rcs.selectorsLibrary.set('.center');
+
+  const includedSelector = rcs.selectorsLibrary.get('.center');
+
+  expect(ignoredSelector).toBe('.center');
+  expect(includedSelector).toBe('a');
+});
+
+it('get | should include already pre excluded types as regex', () => {
+  rcs.selectorsLibrary.set('.center');
+
+  const ignoredSelector = rcs.selectorsLibrary.get('.center');
+
+  rcs.selectorsLibrary.setInclude(/cen/);
+  rcs.selectorsLibrary.set('.center');
+
+  const includedSelector = rcs.selectorsLibrary.get('.center');
+
+  expect(ignoredSelector).toBe('.center');
+  expect(includedSelector).toBe('a');
+});
+
+it('get | should include already pre excluded types as both regex', () => {
+  rcs.selectorsLibrary.setExclude(/tes/);
+
+  const ignoredSelector = rcs.selectorsLibrary.get('.test');
+
+  rcs.selectorsLibrary.setInclude(/te/);
+  rcs.selectorsLibrary.set('.test');
+  rcs.selectorsLibrary.set('.telescope');
+
+  const includedSelector = rcs.selectorsLibrary.get('.test');
+  const anotherIncludedSelector = rcs.selectorsLibrary.get('.telescope');
+
+  expect(ignoredSelector).toBe('.test');
+  expect(includedSelector).toBe('a');
+  expect(anotherIncludedSelector).toBe('b');
+});
+
 it('get | insure no mix if using existing selector', () => {
   rcs.selectorsLibrary.set('.test', 'renamed_test');
 

docs/api/baselibrary.md

@@ -99,6 +99,25 @@ rcs.baseLibrary.set('anything'); // will not set the minified one, as it matches
 rcs.baseLibrary.get('anything'); // anything
 ```
 
+### setInclude
+
+> To include values which has been already excluded before
+
+**rcs.baseLibrary.setInclude(value)**
+
+Parameters:
+- value `<String | RegExp>`
+
+Example:
+
+```js
+const rcs = require('rcs-core');
+
+rcs.baseLibrary.setInclude('center');
+
+rcs.baseLibrary.set('center'); // will be minified although it is ignored by default
+```
+
 ### isExcluded
 
 > Checks if the string is excluded, which was set by `rcs.baseLibrary.setExclude`

docs/caveats.md

@@ -0,0 +1,148 @@
+# Using rcs-core correctly and patterns to avoid
+
+The core is performing three steps on your files:
+1. Extracting CSS's class and id selectors, keyframes, variables and attributes selectors
+2. Assigning minified value for each entry it has found
+3. Trying to replace all elements that could match in your files with the minified version
+
+## Caveats for step 1
+
+> **tl;dr:** Selectors are just renamed if they appear in your CSS
+
+Currently, the first step is done on CSS syntax only (either from CSS files or extracted from HTML's `<style>` tags).
+This means that if you have some HTML code doing this:
+
+```html
+<div class='something'>[...]</div>
+<div class='something'>[...]</div>
+```
+
+without a CSS rule for class's `something`, it'll not be replaced/minified.
+In the future, the core might add a notice for these cases.
+
+HTML replacement should be quite solid for now, since the core can inspect attribute's name and only act upon `class` and `id` (and label's `for`).
+
+## Caveats for step 2
+
+> **tl;dr:** Do not name your selectors like standard HTML attributes. Such selectors are [excluded by default](../lib/helpers/excludeList.ts)
+
+It's very important that the assigned *minified* name for each entry does not collide with a valid CSS selector that you could encounter.
+Paradoxically, if your initial selectors are very large, you'll have less chances to encounter an issue. Using `rcs-core` or any of its [plugins](../README.md#plugins) in your pipeline should encourage more meaningful selectors in your file.
+
+By default, the core tries to minimize the length of the mapped minified name's selector. So if you have a CSS selector in your code like `.something`, it'll be mapped to (for example), `.a` if available.
+If you already have a small selector (like `.b`), the core is smart enough to map it to a similar length's selector (but not necessarly `.b`, it can be `.t`). When all first letter selectors are used, it's generating two letter selectors and so on.
+
+As you'll see in the next section, because of JavaScript's openness, some selectors can be understood differently.
+
+Imagine you have such document:
+```html
+<style>
+  .b { color: red; }
+  b { color: blue; }
+</style>
+
+<div class='b'>
+  <b>Yeah</b>
+</div>
+
+<script>
+  const a = 'b';
+  const c = document.querySelector(a);
+  const d = document.getElementsByClassName(a);
+</script>
+```
+Here, the *key* `'b'` in the JavaScript code can refer to the tag name `<b>` (when being called by `document.querySelector`) or to the class `.b` (when being called by `document.getElementsByClassName`.
+The core does not run your code (it only parses it) and, as such, can't figure out when a variable is being used for the former or latter case.
+Please notice that the core can't expect a *class* selection by looking at `getElementsByClassName` since you can be using any library that's abstracting this (like jQuery, React...).
+
+So, to avoid this caveat, you can either:
+* Avoid conflicting class name with potential HTML tag name (don't name your CSS class like `.b` or `.i` or `.span`, ...)
+* Avoid using `getElementById`, `getElementsByClassName` in your JavaScript code (and only fallback to `querySelector` and `querySelectorsAll`)
+* Reserve some mapping so the core can't use them (check the `config.reserve` array in the documentation about how to do that)
+* Exclude any selector that's conflicting with any HTML tag name (check the `config.exclude` array in the documentation about how to do that)
+
+
+### Warning:
+
+In the example above, `<div class='b'>` can be what the core is generating for your initial class `something`. So the example above could have been generated from this source:
+
+```html
+<style>
+  .something { color: red; }
+  b { color: blue; }
+</style>
+<div class='something'>
+  <b>Yeah</b>
+</div>
+<script>
+  const a = 'something';
+  const c = document.querySelector(a);
+  const d = document.getElementsByClassName(a);
+</script>
+```
+In that case, the initial code is semantically wrong since the `querySelector` should return `undefined`. The minified CSS selector code will thus return a different result in that case.
+
+If you had followed the advice above and excluded (or reserved) all potential HTML tag name (see below), then `something` won't be mapped to `b`, but, for example, `t` and the initial code behavior will be preserved.
+
+Similarly, if your initial code was:
+```js
+...
+const a = 'b';
+...
+```
+then the core will generate a warning telling you that `b` is already used in the renaming map and that you should fix the ambiguity.
+
+By default all HTML and CSS attributes are excluded, see [here](../lib/helpers/excludeList.ts)
+
+The incovenient being that such selectors won't be minified (think about `header` for example, it's quite common for a class name)
+
+## Caveat for step 3
+
+> **tl;dr:** Be careful with CSS selectors in JavaScript. Do not use string concatenation with your CSS selectors.
+
+Replacement is relatively safe for CSS and HTML (again, because such format are explicit enough to avoiding ambiguity).
+
+Replacements inside JavaScript code is much harder because a string for a selector can be generated from many small parts (like in `const a = '.' + objClass;`), can be used in functions that have different semantics (like `getElementById` and `querySelector`), etc...
+
+So, here's the way the core is trying to resolve ambiguities (but as seen above, not all ambiguities can be solved):
+
+1. ECMAScript template, like in ``const a = `some text ${jsConst}`;`` is parsed like HTML, looking for `class="X"` or `id='Y'` or `for="Z"`.
+   If you use template code to make your CSS selector from each part, it'll fail:
+   Don't write ``const myClass = 'something'; const sel = `.${myClass}`;``, but instead `const myClass = '.something'`.
+
+2. Each string literal is extracted from your JavaScript code and:
+    - If it contains some CSS specific selectors chars (like `#` or `.` or ` ` etc...), then it's parsed as a CSS selector. **This is safer and usually gives the expected result.**
+    - If it does not contain some CSS specific selectors (like a single word), then it's tried as an ID selector first and if not found as a class selector (or not replaced if not found in either one).
+    **This can lead to the ambiguity shown above.**
+
+So, to avoid such ambiguity, try to store your class or id with their respective prefix (`.` or `#`) in your code, and rely on `querySelector` et al.
+
+For example, here are some replacement selected by the library, with these CSS selector:
+
+```css
+.something {} /* Mapped to .a */
+#test {}      /* Mapped to #a */
+.test {}      /* Mapped to .b */
+```
+| Initial code | Replaced code | Reason |
+|---|---|---|
+|`const a = 'something';`|`const a = 'b';`| The core could not deduce the string literal is a selector, so fallback to searching for any rule with `something`, class had one|
+|`const a = 'test';`|`const a = 'a';`| The core could not deduce the string literal is a selector and in fallback to search for any rule, it selected #test=>#a since it was searched first|
+|`const a = '.something';`|`const a = '.a';`| The core deduced a class selector was required and replaced it|
+|`const a = '.test';`|`const a = '.b';`| The core deduced a class selector was required and replaced it|
+|`const a = ' something';`|`const a = ' something';`| The core deduced the string literal is a CSS selector, but could not find any match|
+|`const a = 'div[class=something]';`|`const a = 'div[class=a]';`| The core deduced the attribute selector was related to a class selector and replaced it|
+|`const a = 'input[name=something]';`|`const a = 'input[name=something]';`| The core deduced the attribute selector wasn't related to a class, id or for attribute, no replacement was done|
+|`const a = 'b #test, .something';`|`const a = 'b #a, .a';`| The core parsed all selectors and produced correct replacement for each of them|
+
+### Another remark:
+Some are writing CSS selector this way:
+```css
+div[class = something]
+{
+color: red;
+}
+```
+This is perfectly safe from JavaScript code (if you have `const a = 'div[class=something]';`), then this will be replaced correctly by the core (*to* `const a = 'div[class=b]';`).
+
+This however requires much more work from the developer so this is not a solution we recommend.