node-rename-css-selectors icon indicating copy to clipboard operation
node-rename-css-selectors copied to clipboard

Published 20 hours ago • verified publisher icon JPeer264

getElementById string being replaced by class selector name instead of id

Open istng opened this issue 2 years ago • 0 comments

Hi! I found the following issue that I tried to track down into this example:

<html><body>
    <div class="overlay-container">
        <div class="overlay-symbol" id="overlay-symbol"></div>
    </div>

    <div id="button-container">
        <button>
            <img id="button-picture" src="https://images.unsplash.com/photo-1621839673705-6617adf9e890?ixlib=rb-4.0.3&ixid=MnwxMjA3fDB8MHxwaG90by1wYWdlfHx8fGVufDB8fHx8&auto=format&fit=crop&w=1632&q=80">
        </button>
    </div>

    <style type="text/css">
        .overlay-container {
            background-color: yellow;
        }
        .overlay-symbol {
            background-color: grey;
        }
        #button-picture {
            background-color: red;
        }
        #button-container {
            background-color: black;
        }
    </style>

    <script>
        var overlaySymbol = document.getElementById("overlay-symbol");
        var buttonContainer = document.getElementById("button-container");
    </script>
</body></html>

I run rcs with the following script:

const rcs = require('rcs-core')
rcs.fillLibraries(fs.readFileSync('./test.html', 'utf8'),{codeType: 'html'});
console.log(rcs.mapping.generate())
rcs.optimize();
const test = rcs.replace.html(fs.readFileSync('./test.html', 'utf8'));
// output some warnings which has been stacked through the process
rcs.warnings.warn();
fs.writeFileSync('./dist/rcs_test.html', test);

The result:

    <div class="t">
        <div class="n" id="overlay-symbol"></div>
    </div>

    <div id="n">
        <button>
            <img id="t" src="https://images.unsplash.com/photo-1621839673705-6617adf9e890?ixlib=rb-4.0.3&amp;ixid=MnwxMjA3fDB8MHxwaG90by1wYWdlfHx8fGVufDB8fHx8&amp;auto=format&amp;fit=crop&amp;w=1632&amp;q=80">
        </button>
    </div>

    <style type="text/css">
        .t {
            background-color: yellow;
        }
        .n {
            background-color: grey;
        }
        #t {
            background-color: red;
        }
        #n {
            background-color: black;
        }
    </style>

    <script>
        var overlaySymbol = document.getElementById("n");
        var buttonContainer = document.getElementById("n");
    </script>
</body></html>

The mappings generated by optimize:

  selectors: {
    '.overlay-container': 't',
    '.overlay-symbol': 'n',
    '#button-picture': 't',
    '#button-container': 'n'
  }
}

There are two things happening here:

  1. fillLibrary does not make a mapping for overlay-symbol id, I think this is because there is no actual rule on the style section.
  2. replace is replacing getElementById("overlay-symbol") with its mapping for the class overlay-symbol. Now, given that optimize utilizes the same letters for ids and classes, they collide on the script section when they get replaced.

I understand that the first problem could be rather an implementation decision: only automatically map selectors present on css rules. Although, ids and classes can also de used as means for javascript to interact with html elements.
The second issue I take as the actual bug, I think it should not replace an id string for a class string.

Although probably this is not a breaking problem if I provide my own mapping file.

Thanks for the module!

istng avatar Nov 09 '22 17:11 istng