ndparker / rjsmin

Fast javascript minifier for Python
http://opensource.perlig.de/rjsmin/
Apache License 2.0
60 stars 15 forks source link

Space removed in template literals [Cause identified] #22

Open vonmaster opened 4 years ago

vonmaster commented 4 years ago

Hi,

Thank you for this very helpful library. I identified a strange bug which affects template literals.

Description

When I minify the following:

function renderReviewElement(result) {
    return `<span>
                ${result.return_rating.rating ? `${result.return_rating.rating}/5` : 'N/A'}
            </span>
            <button class="btn btn-sm likeButton" type="button" title="Like">
                <span class="fa fa-heart fa-lg" aria-hidden="true"></span>
            </button>`;

}

function initializeTinyMCE(form) {
    if (form) {
        activateTooltips(`${returnCSSPath(form)} *[title]`);
    }
}

Here is the ouput by JSMinFilter(js).output():

function renderReviewElement(result){return`<span>
                ${result.return_rating.rating ? `${result.return_rating.rating}/5` : 'N/A'}</span><button class="btn btn-sm likeButton"type="button"title="Like"><span class="fa fa-heart fa-lg"aria-hidden="true"></span></button>`;

}

function initializeTinyMCE(form) {
    if (form) {
        activateTooltips(`${returnCSSPath(form)}*[title]`);}}

As we can see, some expressions are wrongly formatted which can cause JS errors:

The culprit

After some investigation, my conclusion is that this line is causing this issue: ${result.return_rating.rating ? ` ${result.return_rating.rating}/5` : 'N/A'}

But why?

Solution 1: changing 'N/A' to 'N'

${result.return_rating.rating ? ` ${result.return_rating.rating}/5` : 'N'} does solve the issue. It seems that the slash / is misinterpreted by the parser.

Solution 2: changing 'N/A' to `N/A`

${result.return_rating.rating ? `${result.return_rating.rating}/5` : `N/A`} works as well. It seems that rJSmin requires us to use the same quote character in the ternary operator (in this situation, the backtick character `).

Conclusion

Please let me know what I can do to help. Should we write additional tests? Can this bug be fixed as per the current implementation?

ndparker commented 2 years ago

Hi,

thanks for your report.

as of now, nested template literals are a problem. Nesting in general is hard to come by with regular expressions. I do have some ideas how to solve this (based on splitting and parsing the pieces separately), but unfortunately I'm again out of cycles. Sorry.

Not exactly a duplicate, but related to #18.

Cheers,