Hi,

I found out about this package because Magento 2 is using it in its build process. The build process is very time consuming, and part this time is spent compressing Javascript files with JShrink. When I looked at the code it occurred to me that a different approach might make it much faster. This approach would be based on the pivotal use of the PHP function preg_replace_callback, which would process the entire Javascript file at once.

Here's a bit of (very incomplete) code I used to test if this would work:

$exp = "~(
            /\*!.*?\*/ |                                             # /* license */
            /\*.*?\*/ |                                             # /* comment */
            //[^\n]* |                                              # // comment
            /(?:\\\\/|[^/])+/[dgimsuy]*[ ]*[,;\n)] |                 # erg exp: /(ape|monkey)\/banana/mi;
            \"(?:\\\\\"|[^\"])*\" |                                 # double quoted string
            '(?:\\\\'|[^'])*' |                                     # single quoted string
            (?P<negatives1>--?)\s+(?P<negatives2>--?) |             # a - --b     a-- - b
            (?P<positives1>\+\+?)\s+(?P<positives2>\+\+?) |         # a + ++b     a++ + b
            (?:return|var) |                                        # operator keyword
            [ \t\n]+                                                # whitespace
        )~xs";

        $normalized = str_replace(["\r\n", "\r"], ["\n", "\n"], $js);

        $result = preg_replace_callback($exp, function($matches) {

            $match = $matches[1];
            $first = $match[0];
            switch ($first) {
                case '"':
                    // remove line continuation
                    $string = str_replace("\\\n", "", $match);
                    return $string;
                case ' ':
                    return '';
                case "\n":
                    return '';
                case "\t":
                    return '';
            }
            $firstTwo = substr($match, 0, 2);
            switch ($firstTwo) {
                case '//':
                    return '';
                case '/*':
                    return '';
            }
            switch ($match) {
                case 'var':
                    return 'var ';
                case 'return':
                    return 'return ';
            }
            if (isset($matches['negatives1']) && $matches['negatives1'] !== "") {
                return $matches['negatives1'] . " " . $matches['negatives2'];
            }
            if (isset($matches['positives1']) && $matches['positives1'] !== "") {
                return $matches['positives1'] . " " . $matches['positives2'];
            }
            return $match;
        }, $normalized);

So, basically, the outermost loop is replaced by a single preg_replace_callback. It is much faster because it implements the inner loop with C code (the implementation of preg_replace_callback is written in C), rather than PHP code.

Before I work this out in detail, I was wondering if you are open to this complete rewrite. I am willing to perform the complete rewrite myself, but there are also some points in which the new code will not be backwards compatible with the existing code, like checking if a regular expression is well formed and throwing a RuntimeException. This new code will not do that. Doubtlessly there will be some other minor points that are not completely backwards compatible. So the question is: are you willing to loose some of the compatibility for a signification increase in processing speed?

If you are interested, I would like the help of some beta testers to debug my code. I just had this idea, it doesn't mean I make no mistakes working it out.

Of course I can start a repository in my own domain, but then it would have to start its user base from the start. By rewriting JShrink it may benefit all of your users.

What do you think?

The incentive was that with the xdebug profiler on I ran into timeout (30s) issues, tried to improve it a bit. You might be interested in this, i have tested a bit, all seems fine.

• strict equality tests
• regex /S optimize flag
• omit cast to bool in while
• empty instead of counting

with xdebug on, this got me 50% faster generation (10 instead of 20s) without xdebug its barely noticible

https://github.com/mrclay/minify/blob/master/min_unit_tests/_test_files/js/issue144.js https://github.com/mrclay/minify/blob/master/min_unit_tests/_test_files/js/condcomm.js https://github.com/mrclay/minify/blob/master/min_unit_tests/_test_files/js/issue132.js https://github.com/mrclay/minify/blob/master/min_unit_tests/_test_files/js/issue74.js

Supporting things like a - ++b required quite a patch in JSMin.

I'm still open to using this in Minify if it can handle all the same content and (ideall) faster.

(rev:b3d0a82) Cannot detect this regex and fails with 'Stray Regex Pattern' error.

        return /\?/ .test( url ) ? "&" : "?";

• This regex cannot get detected on line 233 because it doesn't have wrapping quotes.
• And it gets detected on line 248, but it catches the ending of regex, because it has question mark before getting closed. saveRegex() gets called (after the end of regex)
• So, saveRegex() function goes until the end of line and fails.

Since https://github.com/tedious/JShrink/pull/97 got merged, and https://github.com/tedious/JShrink/commit/ca8970bd84e78eabaf0daa30eb591161b4d71859 fix code wise incompatibility with PHP 8. Is there a new release coming soon?

It dies with: Uncaught exception 'RuntimeException' with message 'Unclosed string at position: 59'

given this string: "var regEx = /[a-z0-9!#$%&'+/=?^{|}~-]+(?:.[a-z0-9!#$%&'_+/=?^_{|}~-]+)@(?:a-z0-9?.)+(?:[A-Z])\b/;"

EDIT: The regEx is wrong, and shouldn't be like this. The start is /[a-z0-9!#$%&'+/ which normally completes a regex. The question is, I guess, whether JSShrink should still die or not.

This will allow for a production ready version of JShink when archived/exported for release(s) and still allowing a development version within the repository.

The export-ignore excludes un-needed files and directories when a branch is archived for release and these files are still included when the repo is cloned/forked.

This would be very useful because releases are intended for production use and when users are using a package (via composer or zip), they most likely are not interested in downloading the source code as a whole into production, thus saving server disk space and meeting security protocol. Think of it as a .gitignore for releases.

• e.g. /tests folder, or your .travis.yml file, etc. will not be included in the release zip(s)

See: git-scm.com - Exporting-Your-Repository

I've realized that if I minify the jquery plugin qtip2 (nightly built) with JShrink by using $js = JShrink\Minifier::minify($js); this will corrupt the file and its execution.

Here is the problematic part:

Original:

(i.innerHTML="<!--[if gt IE "+ ++t+"]><i></i><![endif]-->")&&
i.getElementsByTagName("i")[0];

will be:

(i.innerHTML="<!--[if gt IE "+++t+"]><i></i><![endif]-->")&&
i.getElementsByTagName("i")[0];

(!) it is the missing space after the first plus.

Any ideas how to solve this?

Not sure if this is more of a tinyMCE thing or a JShrink thing. When loading up content that loads tinyMCE.init(), a couple requests are made to:

www.(baseurl).com//langs/en.js

When the proper URL is something like:

www.(baseurl).com/en/asset/js/tinyMCE/langs/en.js

I don't mind the fact that it's making a request, as much as it's making a request to the wrong place. There are a pair of slashes and the directory is off. Maybe I'll need to hack the tinyMCE code to fix this? I've read in other places that people have had issues minifying tinyMCE.

Anyways, very good work. We're running a javascript-heavy site, and tinyMCE is the only thing that broke as a result of JShrink! ;)

Originally reported in https://github.com/matomo-org/matomo/issues/16707

Minimal testcase (using https://code.jquery.com/jquery-3.5.1.js):

<?php
ini_set('max_execution_time', 30);
include('vendor/autoload.php');
$js = file_get_contents("jquery-3.5.1.js");
$minifiedCode = \JShrink\Minifier::minify($js);
echo $minifiedCode;

takes 30 seconds to run when calling with php8 script.php (but weirdly enough outputs a minified js which is only missing a ; at the end).

using PHP 8.0.0RC2 (but the same issue also occurs in Matomo when using RC3)

Hi,

I've tried to go deeper into performance tweaks. Please review series of changes which I've prepared.

I've bench-marked every improvement separately before deciding whether to add it.

As a test I've run new and old version on JS library for 3D rendering (threejs-lib) which consists of 352 JavaScript files, 11MB. I compared MD5 of minified content for new and current-master versions of JShink - no changes found.

Current master: real 0m10,164s user 0m10,118s sys 0m0,036s

New version: real 0m9,469s user 0m9,454s sys 0m0,008s

New version is 0.695 sec faster, 6.8% improvement (on this specific JS lib). IMO not bad :-)

Andrey.

In certain cases nested template literals seem to cause some spaces being removed.

These seem to work fine, and the minified output is the same:

console.log(`aa ${`bb cc`}`);
console.log(`aa ${true ? `bb cc` : `bb cc`}`);
console.log(`aa ${`a bb cc`}`);

In these cases some spaces get removed:

console.log(`aa ${`, bb cc`}`);
console.log(`aa ${`. bb cc`}`);

When minified:

console.log(`aa ${`,bb cc`}`);
console.log(`aa ${`.bb cc`}`);

I tried to put into one .js file to minify count of file requests, first content of minified axios javascript, then some newline (comments etc) and then own code.

Axios is sure a modern and popular thing which should be considered important, and if it minified is buggy, then possibly other llibraries using the same minification may cause the same issues (webpack based minification)

Without shrinking it works fine, minified code from JSrink does not work anymore

To repeat, just find axios cdn, try copypasting content to local js file, then add newline and some own simple command, like

/* axios minified code first*/
{put here contents from https://cdnjs.cloudflare.com/ajax/libs/axios/0.27.2/axios.min.js}

/* own code */
console.log(1234);
document.body.innerHTML = 'WORKS!';

then just use script tag in some html, and after loading check console

Fix regex line: example line - {"use strict";t.exports=function(t){return/^([a-z][a-z\d+-.]:)?///i.test(t)}} after processing, the code was not cut correctly {"use strict";t.exports=function(t){return/^([a-z][a-z\d+-.]:)?/\

tinymce.min.js contains the following Javascript:

return/^[^:]+:\/\/\/?[^\/]+\//.test(t)

JSShrink does not call the saveRegex method for this Javascript string as it is not a recognized RegEx starting point.

By adding "n" to the strpos test before the saveRegex call, the return/RegEx/ situation is properly handled. In the above case.

When it is not properly handled, the \//.test(t) part of the RegEx is treated as a comment and is improperly truncated for tineymce.min.js.

In code i returning a url "https://player.vimeo.com/api/player.js"

getApiURL(){return "https://player.vimeo.com/api/player.js"}}

but after minifying code is removed after "https:" because you code consider a url as comment after double slash. code after minified: getApiURL(){return "https:;