Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue with complex RegExp (like those in fabricjs) #526

Closed
JSteunou opened this issue May 10, 2017 · 4 comments
Closed

Issue with complex RegExp (like those in fabricjs) #526

JSteunou opened this issue May 10, 2017 · 4 comments

Comments

@JSteunou
Copy link

JSteunou commented May 10, 2017

Hi,

I have an issue when using babili in my project that depends on fabricjs

After minification (preset env no extra params) I got a code like this

                var je = [1, 0, 0, 1, 0, 0]
                  , ye = {reNum: '(?:[-+]?(?:\\d+|\\d*\\.\\d+)(?:e[-+]?\\d+)?)'}
                  , Ne = ye.reNum
                  , ze = '(?:\\s+,?\\s*|,\\s*)'
                  , qe = '(?:' + ('(?:(matrix)\\\\s*\\\\(\\\\s*(' + Ne + ')' + ze + '(' + Ne + ')' + ze + '(' + Ne + ')' + ze + '(' + Ne + ')' + ze + '(' + Ne + ')' + ze + '(' + Ne + ')\\s*\\))') + '|' + ('(?:(translate)\\s*\\(\\s*(' + Ne + ')(?:' + ze + '(' + Ne + '))?\\s*\\))') + '|' + ('(?:(scale)\\s*\\(\\s*(' + Ne + ')(?:' + ze + '(' + Ne + '))?\\s*\\))') + '|' + ('(?:(rotate)\\s*\\(\\s*(' + Ne + ')(?:' + ze + '(' + Ne + ')' + ze + '(' + Ne + '))?\\s*\\))') + '|' + ('(?:(skewX)\\s*\\(\\s*(' + Ne + ')\\s*\\))') + '|' + ('(?:(skewY)\\s*\\(\\s*(' + Ne + ')\\s*\\))') + ')'
                  , Ke = new RegExp('^\\s*(?:' + ('(?:' + qe + '(?:' + ze + '*' + qe + ')*)') + '?)\\s*$')
                  , Ze = new RegExp(qe,'g');

with qe equals to

"(?:(?:(matrix)\\s*\\(\\s*((?:[-+]?(?:\d+|\d*\.\d+)(?:e[-+]?\d+)?))(?:\s+,?\s*|,\s*)((?:[-+]?(?:\d+|\d*\.\d+)(?:e[-+]?\d+)?))(?:\s+,?\s*|,\s*)((?:[-+]?(?:\d+|\d*\.\d+)(?:e[-+]?\d+)?))(?:\s+,?\s*|,\s*)((?:[-+]?(?:\d+|\d*\.\d+)(?:e[-+]?\d+)?))(?:\s+,?\s*|,\s*)((?:[-+]?(?:\d+|\d*\.\d+)(?:e[-+]?\d+)?))(?:\s+,?\s*|,\s*)((?:[-+]?(?:\d+|\d*\.\d+)(?:e[-+]?\d+)?))\s*\))|(?:(translate)\s*\(\s*((?:[-+]?(?:\d+|\d*\.\d+)(?:e[-+]?\d+)?))(?:(?:\s+,?\s*|,\s*)((?:[-+]?(?:\d+|\d*\.\d+)(?:e[-+]?\d+)?)))?\s*\))|(?:(scale)\s*\(\s*((?:[-+]?(?:\d+|\d*\.\d+)(?:e[-+]?\d+)?))(?:(?:\s+,?\s*|,\s*)((?:[-+]?(?:\d+|\d*\.\d+)(?:e[-+]?\d+)?)))?\s*\))|(?:(rotate)\s*\(\s*((?:[-+]?(?:\d+|\d*\.\d+)(?:e[-+]?\d+)?))(?:(?:\s+,?\s*|,\s*)((?:[-+]?(?:\d+|\d*\.\d+)(?:e[-+]?\d+)?))(?:\s+,?\s*|,\s*)((?:[-+]?(?:\d+|\d*\.\d+)(?:e[-+]?\d+)?)))?\s*\))|(?:(skewX)\s*\(\s*((?:[-+]?(?:\d+|\d*\.\d+)(?:e[-+]?\d+)?))\s*\))|(?:(skewY)\s*\(\s*((?:[-+]?(?:\d+|\d*\.\d+)(?:e[-+]?\d+)?))\s*\)))"

being a SyntaxError: Invalid regular expression: Unterminated group

Whereas when trying in online REPL I got a code like this

'use strict';
var fabric = {
        reNum: '(?:[-+]?(?:\\d+|\\d*\\.\\d+)(?:e[-+]?\\d+)?)'
    },
    iMatrix = [1, 0, 0, 1, 0, 0],
    number = fabric.reNum,
    commaWsp = '(?:\\s+,?\\s*|,\\s*)',
    skewX = '(?:(skewX)\\s*\\(\\s*(' + number + ')\\s*\\))',
    skewY = '(?:(skewY)\\s*\\(\\s*(' + number + ')\\s*\\))',
    rotate = '(?:(rotate)\\s*\\(\\s*(' + number + ')(?:' + commaWsp + '(' + number + ')' + commaWsp + '(' + number + '))?\\s*\\))',
    scale = '(?:(scale)\\s*\\(\\s*(' + number + ')(?:' + commaWsp + '(' + number + '))?\\s*\\))',
    translate = '(?:(translate)\\s*\\(\\s*(' + number + ')(?:' + commaWsp + '(' + number + '))?\\s*\\))',
    matrix = '(?:(matrix)\\s*\\(\\s*(' + number + ')' + commaWsp + '(' + number + ')' + commaWsp + '(' + number + ')' + commaWsp + '(' + number + ')' + commaWsp + '(' + number + ')' + commaWsp + '(' + number + ')\\s*\\))',
    transform = '(?:' + matrix + '|' + translate + '|' + scale + '|' + rotate + '|' + skewX + '|' + skewY + ')',
    transforms = '(?:' + transform + '(?:' + commaWsp + '*' + transform + ')*)',
    transformList = '^\\s*(?:' + transforms + '?)\\s*$',
    reTransformList = new RegExp(transformList),
    reTransform = new RegExp(transform, 'g');

with transform equals to

"(?:(?:(matrix)\s*\(\s*((?:[-+]?(?:\d+|\d*\.\d+)(?:e[-+]?\d+)?))(?:\s+,?\s*|,\s*)((?:[-+]?(?:\d+|\d*\.\d+)(?:e[-+]?\d+)?))(?:\s+,?\s*|,\s*)((?:[-+]?(?:\d+|\d*\.\d+)(?:e[-+]?\d+)?))(?:\s+,?\s*|,\s*)((?:[-+]?(?:\d+|\d*\.\d+)(?:e[-+]?\d+)?))(?:\s+,?\s*|,\s*)((?:[-+]?(?:\d+|\d*\.\d+)(?:e[-+]?\d+)?))(?:\s+,?\s*|,\s*)((?:[-+]?(?:\d+|\d*\.\d+)(?:e[-+]?\d+)?))\s*\))|(?:(translate)\s*\(\s*((?:[-+]?(?:\d+|\d*\.\d+)(?:e[-+]?\d+)?))(?:(?:\s+,?\s*|,\s*)((?:[-+]?(?:\d+|\d*\.\d+)(?:e[-+]?\d+)?)))?\s*\))|(?:(scale)\s*\(\s*((?:[-+]?(?:\d+|\d*\.\d+)(?:e[-+]?\d+)?))(?:(?:\s+,?\s*|,\s*)((?:[-+]?(?:\d+|\d*\.\d+)(?:e[-+]?\d+)?)))?\s*\))|(?:(rotate)\s*\(\s*((?:[-+]?(?:\d+|\d*\.\d+)(?:e[-+]?\d+)?))(?:(?:\s+,?\s*|,\s*)((?:[-+]?(?:\d+|\d*\.\d+)(?:e[-+]?\d+)?))(?:\s+,?\s*|,\s*)((?:[-+]?(?:\d+|\d*\.\d+)(?:e[-+]?\d+)?)))?\s*\))|(?:(skewX)\s*\(\s*((?:[-+]?(?:\d+|\d*\.\d+)(?:e[-+]?\d+)?))\s*\))|(?:(skewY)\s*\(\s*((?:[-+]?(?:\d+|\d*\.\d+)(?:e[-+]?\d+)?))\s*\)))"

which is a valid RegExp.

I'm still searching what are the options differences between the online REPL and default configuration to point out the faulty plugin.

@JSteunou
Copy link
Author

The root of the issue is the escaping. My minification got to much \\\\ instead of just \\

@JSteunou
Copy link
Author

ok seems to be fixed by #440 related to #499

How can I avoid this bug until the fix is released?

@boopathi
Copy link
Member

boopathi commented May 10, 2017

Thanks. I'm not sure if it has any workaround. Planning to release a patch version this week after a few more bug fixes.

Closing this as it's fixed in master.

@JSteunou
Copy link
Author

JSteunou commented May 10, 2017

How can I avoid this bug until the fix is released?

I will answer for those having the same issue: you should deactivate minify-constant-folding plugin either via presets or via option evaluate: false

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants