adamgian / minify-selectors

Post-processor to minify class and ID selector names.
Apache License 2.0
8 stars 0 forks source link

Support JS dot and bracket notation #36

Open kimbauters opened 1 year ago

kimbauters commented 1 year ago

In some cases the analysis minifies a selector in the wrong way. Unfortunately, because this is for internal code, I cannot post the full HTML on GitHub. If you want it though, I am happy to send it to you privately.

Hopefully I can describe the issue enough to make it reproducible. Take an HTML form that calls some javascript:

<form action="#" onsubmit="return validate(this);">
    <input type="url" id="url" class="custom-search-input" onkeyup="check(this);" placeholder="https://en.wikipedia.org/wiki/Minification_(programming)">
    <textarea name="message" id="message" class="custom-search-textarea" placeholder="Add a message here"></textarea>
    <button class="custom-search-botton" id="button" type="submit">do it</button>
</form>

The problem now occurs for id url and message. Assume I have this as my JS:

function check(input) {
    ...
    let url = document.getElementById("url");
    ...
}

then url gets minified. However:

function validate(form) {
   next_step(form.url.value, form.message.value);
}

the use of url and message here will not be detected. As a result message does not get minified, while url gets minified but the above code line would not be updated.

edit: sorry, I hit submit too early. Updated to finish the explanation.

edit 2: a further minification could be to also rename JavaScript functions.

adamgian commented 1 year ago

Thank you so much @kimbauters for taking the time to lodge such a detailed issue report. Much appreciated.

Right now, due to the limitations of the regex-based implementation, it's difficult to discern between an actual ID property and an object property. Will have a think about this and see how far it could be pushed. Though I suspect it'll eventually need to be done with an AST implementation down the line.

I was going to suggest maybe doing form.elements["url"].value, but embarrassingly I've missed that and that needs to be added (triaged it into #38). Perhaps form.querySelector("#url").value for the time being as a stop-gap solution, unfortunately it's not as elegant as how you have it right now.

kimbauters commented 1 year ago

No problem at all. It is definitely something that can be worked around. This particular bug just caught me a bit by surprise as it was otherwise working well.

I'm fully aware of the challenge here though. You basically have to juggle 4 languages at the same time, each with their own dialects, so it is by no means an easy feat!

adamgian commented 1 year ago

Oh and you've correctly observed that it would not minify the url ID in your markup until it was used in your scripts as well. I've mulled over this and it's probably better to always minify IDs regardless (tracking in #39).