tedious / JShrink

Javascript Minifier built in PHP
http://www.tedivm.com
BSD 3-Clause "New" or "Revised" License
749 stars 152 forks source link

RuntimeException in version 1.6.x #123

Closed calcosta closed 1 year ago

calcosta commented 1 year ago

Just updated to version 1.6.

Shrinking several libraries now breaks with Error: Unclosed string at position: or Error: Unclosed regex pattern at position.

One example is jquery-ui.js (see https://pastebin.com/NwEXPtAb): breaks e.g. on line 12958

I added $this->echo('|error|'); before throw new \RuntimeException('Unclosed regex pattern at position: ' . $this->index); and removed the exception to get an idea where the error occurs. See https://pastebin.com/AX1XFgS3 (starts on line 1074).

Thanks for help!

tedivm commented 1 year ago

Thanks for narrowing it down- honestly that is the hardest part of a lot of these bugs, just figuring out where it's breaking. Looking at the line in question I've got a good idea of what's happening, so once I'm done with work today I should be able to tackle it.

manavo commented 1 year ago

Not sure if this helps (or if it's the same), but I was able to track down a similar exception with Error: Unclosed regex pattern at position to having the combination g/ but not as part of a regex: var value = currentRating/other;

Failing test:

    public function testUnclosedRegexExceptionDoesNotHappenWithVariablesEndingWithG()
    {
        $this->expectNotToPerformAssertions();
        \JShrink\Minifier::minify('var value = currentRating/other;');
    }
tedivm commented 1 year ago

Test cases are always super super helpful! I normally put them directly into the test suite when I'm working on fixes.

manavo commented 1 year ago

Yeah, I'd include it in a pull request normally, but haven't had a chance to dig into why this is happening. And then I saw an open PR and this issue, so wasn't sure if it's already mostly resolved 🙂

tedivm commented 1 year ago

I think I'll be able to resolve it with about an hour of work, but unfortunately last night I had a migraine so I didn't have a chance.

manavo commented 1 year ago

No worries at all! Thanks for looking into this as quickly as you are!

attrib commented 1 year ago

I'm running into this issue as well.

Maybe my reduced test case helps when trying to test it:

<?php

require 'vendor/autoload.php';

// https://github.com/salesagility/SuiteCRM/blob/hotfix/themes/SuiteP/js/style.js#L183
var_dump(JShrink\Minifier::minify('if (spans[jj].className.match(/currentTab.*/)) {
    tabNum = ii;
}'));

Returns: Unclosed regex pattern at position: 49

I also tried to apply the fix from #124 but the issue remains the same.

attrib commented 1 year ago

Actually noticed the above error is the error with the #124 applied.

Here an example when I do it without #124

<?php

require 'vendor/autoload.php';

var_dump(JShrink\Minifier::minify('if(D>(F/2){console.log("a")}'));

Works if I add spaces around the /, but doesn't minify it

<?php

require 'vendor/autoload.php';

var_dump(JShrink\Minifier::minify('if(D>(F / 2){console.log("a")}'));

Results in if(D>(F / 2){console.log("a")} Expected: if(D>(F/2){console.log("a")}

Hope this helps while debugging the issue.

Downgrading to JShrink 1.5.0 for the meanwhile.

tedivm commented 1 year ago

A fix has been pushed up! I've incorporated all of the examples from this thread into the test suite as well to prevent future regressions.

Thanks for everyone's help!

maciej-orba commented 1 year ago

Hello

After using updated version number of issues is less but still exist (before hundreds, now few)!

ex.


Compilation from source /var/www/magento/lib/web/jquery/jquery-ui.js failed
RuntimeException: Unclosed regex pattern at position: 420835 in /var/www/magento/vendor/tedivm/jshrink/src/JShrink/Minifier.php:634
Stack trace:
#0 /var/www/magento/vendor/tedivm/jshrink/src/JShrink/Minifier.php(300): JShrink\Minifier->saveRegex()
#1 /var/www/magento/vendor/tedivm/jshrink/src/JShrink/Minifier.php(162): JShrink\Minifier->loop()
#2 /var/www/magento/vendor/tedivm/jshrink/src/JShrink/Minifier.php(137): JShrink\Minifier->minifyToString('/*! jQuery UI -...', Array)
#3 /var/www/magento/vendor/magento/framework/Code/Minifier/Adapter/Js/JShrink.php(27): JShrink\Minifier::minify('/*! jQuery UI -...')
#4 /var/www/magento/vendor/magento/framework/View/Asset/PreProcessor/Minify.php(60): Magento\Framework\Code\Minifier\Adapter\Js\JShrink->minify('/*! jQuery UI -...')
#5 /var/www/magento/vendor/magento/framework/View/Asset/PreProcessor/Pool.php(77): Magento\Framework\View\Asset\PreProcessor\Minify->process(Object(Magento\Framework\View\Asset\PreProcessor\Chain))
#6 /var/www/magento/vendor/magento/framework/View/Asset/Source.php(152): Magento\Framework\View\Asset\PreProcessor\Pool->process(Object(Magento\Framework\View\Asset\PreProcessor\Chain))
#7 /var/www/magento/vendor/magento/framework/View/Asset/Source.php(105): Magento\Framework\View\Asset\Source->preProcess(Object(Magento\Framework\View\Asset\File))
#8 /var/www/magento/vendor/magento/framework/View/Asset/File.php(159): Magento\Framework\View\Asset\Source->getFile(Object(Magento\Framework\View\Asset\File))
#9 /var/www/magento/vendor/magento/framework/App/View/Asset/Publisher.php(74): Magento\Framework\View\Asset\File->getSourceFile()
#10 /var/www/magento/vendor/magento/framework/App/View/Asset/Publisher.php(62): Magento\Framework\App\View\Asset\Publisher->publishAsset(Object(Magento\Framework\View\Asset\File))
#11 /var/www/magento/vendor/magento/module-deploy/Service/DeployStaticFile.php(97): Magento\Framework\App\View\Asset\Publisher->publish(Object(Magento\Framework\View\Asset\File))
#12 /var/www/magento/vendor/magento/module-deploy/Service/DeployPackage.php(189): Magento\Deploy\Service\DeployStaticFile->deployFile('jquery/jquery-u...', Array)
#13 /var/www/magento/vendor/magento/module-deploy/Service/DeployPackage.php(133): Magento\Deploy\Service\DeployPackage->processFile(Object(Magento\Deploy\Package\PackageFile), Object(Magento\Deploy\Package\Package))
#14 /var/www/magento/vendor/magento/module-deploy/Service/DeployPackage.php(102): Magento\Deploy\Service\DeployPackage->deployEmulated(Object(Magento\Deploy\Package\Package), Array, true)
#15 [internal function]: Magento\Deploy\Service\DeployPackage->Magento\Deploy\Service\{closure}()
#16 /var/www/magento/vendor/magento/framework/App/State.php(187): call_user_func_array(Object(Closure), Array)
#17 /var/www/magento/generated/code/Magento/Framework/App/State/Interceptor.php(68): Magento\Framework\App\State->emulateAreaCode('adminhtml', Object(Closure), Array)
#18 /var/www/magento/vendor/magento/module-deploy/Service/DeployPackage.php(103): Magento\Framework\App\State\Interceptor->emulateAreaCode('adminhtml', Object(Closure))
#19 /var/www/magento/vendor/magento/module-deploy/Process/Queue.php(354): Magento\Deploy\Service\DeployPackage->deploy(Object(Magento\Deploy\Package\Package), Array, true)
#20 /var/www/magento/vendor/magento/module-deploy/Process/Queue.php(267): Magento\Deploy\Process\Queue->execute(Object(Magento\Deploy\Package\Package))
#21 /var/www/magento/vendor/magento/module-deploy/Process/Queue.php(248): Magento\Deploy\Process\Queue->executePackage(Object(Magento\Deploy\Package\Package), 'adminhtml/Magen...', Array, false)
#22 /var/www/magento/vendor/magento/module-deploy/Process/Queue.php(176): Magento\Deploy\Process\Queue->assertAndExecute('adminhtml/Magen...', Array, Array)
#23 /var/www/magento/vendor/magento/module-deploy/Strategy/QuickDeploy.php(84): Magento\Deploy\Process\Queue->process()
#24 /var/www/magento/vendor/magento/module-deploy/Service/DeployStaticContent.php(115): Magento\Deploy\Strategy\QuickDeploy->deploy(Array)
#25 /var/www/magento/setup/src/Magento/Setup/Console/Command/DeployStaticContentCommand.php(136): Magento\Deploy\Service\DeployStaticContent->deploy(Array)
#26 /var/www/magento/vendor/symfony/console/Command/Command.php(298): Magento\Setup\Console\Command\DeployStaticContentCommand->execute(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#27 /var/www/magento/vendor/symfony/console/Application.php(1040): Symfony\Component\Console\Command\Command->run(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#28 /var/www/magento/vendor/symfony/console/Application.php(301): Symfony\Component\Console\Application->doRunCommand(Object(Magento\Setup\Console\Command\DeployStaticContentCommand), Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#29 /var/www/magento/vendor/magento/framework/Console/Cli.php(116): Symfony\Component\Console\Application->doRun(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#30 /var/www/magento/vendor/symfony/console/Application.php(171): Magento\Framework\Console\Cli->doRun(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#31 /var/www/magento/bin/magento(23): Symfony\Component\Console\Application->run()
maciej-orba commented 1 year ago

Ref file: https://github.com/magento/magento2/blob/2.4-develop/lib/web/jquery/jquery-ui.js

maciej-orba commented 1 year ago

Please reopen

tedivm commented 1 year ago

@maciej-orba - check out the latest PR, #126, it should resolve this. I've included the jquery-ui test in the test suite itself to confirm that no errors are being triggered.

maciej-orba commented 1 year ago

I confirm, Static Content Deploy for Magento works fine, with no issues on latest JShrink Thank you!

adminhtml/Magento/backend/en_US         1825/2923           =================>---------- 62%    6 secs              
frontend/Magento/luma/en_US             1007/2199           ============>--------------- 45%    6 secs
frontend/Magento/blank/en_US            2102/2183           ==========================>- 96%    11 secs             
adminhtml/Magento/backend/en_US         2678/2923           =========================>-- 91%    11 secs             
frontend/Magento/luma/en_US             2099/2199           ==========================>- 95%    11 secs
frontend/Magento/blank/en_US            2183/2183           ============================ 100%   11 secs             
adminhtml/Magento/backend/en_US         2923/2923           ============================ 100%   11 secs             
frontend/Magento/luma/en_US             2199/2199           ============================ 100%   11 secs
Execution time: 14.963996887207
tedivm commented 1 year ago

Thank you for confirming that- if you have any other issues please feel free to open up an issue here!

hendralin commented 1 year ago

Hello,

I'm still using version 1.6.0 and PHP7, but Error: Unclosed regex pattern at position...still appear.

because since 1.6.1 it dropped PHP 7 support.

is there anyway to patch this error?

I'm now downgrade to 1.5.0

Thank you

tedivm commented 1 year ago

I should have dropped php7 support in 1.6 but didn't as an error. You should lock to 1.5 or even 1.4 if you're having trouble and want to stay on the no longer supported php7. You should really upgrade to php8 though, as php7 isn't getting security updates anymore.

tedivm commented 1 year ago

I'm going to lock this topic for now- if people find their way here please just open up a new ticket with any issues you're having.