wp-media / wp-rocket

Performance optimization plugin for WordPress
https://wp-rocket.me
GNU General Public License v2.0
701 stars 218 forks source link

Missing semicolons at the ends of lines in source Javascript aren't restored during minification #5512

Open ethanclevenger91 opened 2 years ago

ethanclevenger91 commented 2 years ago

Before submitting an issue please check that you’ve completed the following steps:

Describe the bug Missing semicolons at the ends of lines in source Javascript aren't restored during minification, causing JS errors. For example:

var fulfilled = true;
var deferred = 'foobar';
var result = false;
if(fulfilled) {
    deferred.splice(i--, 1)
    var r = fn();
    if (r !== undefined) result = r;
}

Note the missing semicolon after the line beginning with deferred, which is valid Javascript.

When JS minification is enabled in WP Rocket, it chokes on this and doesn't restore the semicolon when putting this all onto one line. The resulting minified code looks like this:

if(fulfilled){deferred.splice(i--,1)var r=fn();if(r!==undefined)result=r}

Note that there's still a missing semicolon at )var. This throws a console error and crashes JS on the page.

To Reproduce Steps to reproduce the behavior:

  1. New WP install w/ WP Rocket (WP 6.0.3, WP Rocket 3.12.1.1)
  2. Install and activate Twenty Twenty
  3. Add the above code to assets/js/index.js
  4. Activate JS minification
  5. View site in incognito/private (or otherwise ensure you're viewing cached version of the site)
  6. See console error Uncaught SyntaxError: unexpected token: keyword 'var'

Expected behavior Valid Javascript when minification is enabled.

Backlog Grooming (for WP Media dev team use only)

piotrbak commented 1 year ago

@wp-media/qa Could you check if this is still happening with the following PR? https://github.com/wp-media/wp-rocket/issues/5617

vmanthos commented 1 year ago

Could you check if this is still happening with the following PR?

I'm on this.

vmanthos commented 1 year ago

Using the provided snippet, there were console errors related to i and fn not being defined. So, I used this instead:

    var fulfilled = true;
    var deferred = 'foobar';
    var result = false;
    if(fulfilled) {
        console.log("This is super!")
        var r = true;
    }

This is working fine both with the current and new minification library. Having the var keyword allows the browser to detect this is a new line, even though no ; exists.

piotrbak commented 1 year ago

@ethanclevenger91 Could you confirm that you're on the latest WP Rocket version?

We made some tests and the minification library detects where to make the breaks: image

ethanclevenger91 commented 1 year ago

@piotrbak let me do a little extra testing. I think I just encountered this bug on a site running the latest WP Rocket, so I believe it's still an issue, but lemme test a couple things.

iruzevic commented 1 year ago

Any updates on this issue? I have similar thing with the latest version (3.12.5.3) only when I turn on minification of JS. I use WebPack in my project and there is something like this:

// install a JSONP callback for chunk loading
/******/        var webpackJsonpCallback = (parentChunkLoadingFunction, data) => {
/******/            var chunkIds = data[0];
/******/            var moreModules = data[1];
/******/            var runtime = data[2];
/******/            // add "moreModules" to the modules object,
/******/            // then flag all "chunkIds" as loaded and fire callback
/******/            var moduleId, chunkId, i = 0;
/******/            if(chunkIds.some((id) => (installedChunks[id] !== 0))) {
/******/                for(moduleId in moreModules) {
/******/                    if(__webpack_require__.o(moreModules, moduleId)) {
/******/                        __webpack_require__.m[moduleId] = moreModules[moduleId];
/******/                    }
/******/                }
/******/                if(runtime) var result = runtime(__webpack_require__);
/******/            }
/******/            if(parentChunkLoadingFunction) parentChunkLoadingFunction(data);
/******/            for(;i < chunkIds.length; i++) {
/******/                chunkId = chunkIds[i];
/******/                if(__webpack_require__.o(installedChunks, chunkId) && installedChunks[chunkId]) {
/******/                    installedChunks[chunkId][0]();
/******/                }
/******/                installedChunks[chunkId] = 0;
/******/            }
/******/        
/******/        } // Missing ;
/******/        
/******/        var chunkLoadingGlobal = self["webpackChunk_eightshift_infobip"] = self["webpackChunk_eightshift_infobip"] || [];
/******/        chunkLoadingGlobal.forEach(webpackJsonpCallback.bind(null, 0));
/******/        chunkLoadingGlobal.push = webpackJsonpCallback.bind(null, chunkLoadingGlobal.push.bind(chunkLoadingGlobal));

on output I get this:

[chunkId][0]()}installedChunks[chunkId]=0}}var chunkLoadingGlobal=self.webpackC

missing ; breaks my projects JS

iruzevic commented 1 year ago

any update on this issue?

iruzevic commented 1 year ago

any update on this issue?

DahmaniAdame commented 1 year ago

Another one that can be reproduced:

$('body').on('click', '.single-people.has-modal', function() {
    $(this).next('.modal').toggleClass('open');
    $('#c-mask').toggleClass('is-active');
});

Getting turned to this:

$('body').on('click', '.single-people.has-modal', function() {
    $(this).next('.modal').toggleClass('open');
    $('#c-mask').toggleClass('is-active')
});

It's related to this - https://github.com/matthiasmullie/minify/issues/287

iruzevic commented 1 year ago

any update on this issue?

emont88 commented 1 year ago

Any update here? Having the exact same issue on WP 6.3 and WP Rocket 3.14.3

ethanclevenger91 commented 5 months ago

I'm still encountering this issue with WP Rocket 3.16. Not sure if it's helpful or not, but I'm attaching the source file causing issues. Line 2790 is where the problem occurs once minified. main.js.zip

iruzevic commented 5 months ago

I gave up on this issue ever being fixed 👎

piotrbak commented 4 months ago

This issue is related to the minification library. We'll see if we can contribute there to get it fixed.

suzoutlet commented 1 week ago

Related -> https://secure.helpscout.net/conversation/2737981260/518480/

let error = { test: [  /\sedg\/ ] };
alert( 'test' );

After minification is turned to

let error={test:[/\sedg\/]};
alert('test')

In the browser, it throws this error: Uncaught SyntaxError: Invalid regular expression: missing /