Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

getElementById string being replaced by class selector name instead of id #88

Open
istng opened this issue Nov 9, 2022 · 0 comments
Open

Comments

@istng
Copy link

istng commented Nov 9, 2022

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant