slimit removes parentheses from ternary expression, causes syntax error in jQuery #42

Closed
hallvors opened this Issue Jan 23, 2013 · 6 comments

Projects

None yet

3 participants

@hallvors

There is a bug in the Slimit JSParser / serializer, which we hit when trying to convert some files including jQuery. It removes some parentheses it shouldn't remove, causing a syntax error in jquery.js.

e.isPlainObject(d) ? (a = [c.createElement(j[1])], e.fn.attr.call(a, d, !0))

becomes

e.isPlainObject(d) ? a = [c.createElement(j[1])], e.fn.attr.call(a, d, !0)

which breaks the ternary expression.

Probably another variant of #21 ?

The full line of code is:

d=d instanceof e?d[0]:d,k=d?d.ownerDocument||d:c,j=m.exec(a),j?e.isPlainObject(d)?(a=[c.createElement(j[1])],e.fn.attr.call(a,d,!0)):a=[k.createElement(j[1])]:(j=e.buildFragment([g[1]],[k]),a=(j.cacheable?e.clone(j.fragment):j.fragment).childNodes);

@rspivak rspivak was assigned Jan 24, 2013
@rspivak
Owner

Thanks a lot for the bug report. I should be able to look into it in the next couple days.

@rspivak
Owner

What version of Slimit do you use?

@hallvors

Seems to be slimit 0.7.4

@hallvors

Another snippet of code broken by the same bug:

null!=this._locales&&(this._defaultLocale=b.default_locale||"en",this._currentLocale=null!=a&&""!=a?a.slice(0,2).toLowerCase():this._defaultLocale,this._messages={},a=this._getLocaleMessages(this._currentLocale),null!=a&&(this._messages[this._currentLocale]=a),this._currentLocale!=this._defaultLocale&&(this._messages[this._defaultLocale]=
this._getLocaleMessages(this._defaultLocale)))

Turns into

null != this._locales && this._defaultLocale = b.default_locale || "en", this._currentLocale = null != a && "" != a ? a.slice(0, 2).toLowerCase() : this._defaultLocale, this._messages = {
    }, a = this._getLocaleMessages(this._currentLocale), null != a && (this._messages[this._currentLocale] = a), this._currentLocale != this._defaultLocale && (this._messages[this._defaultLocale] = this._getLocaleMessages(this._defaultLocale));
@hallvors

Maybe the simplest test case is

(function(){ /* code */ }());

which (according to a colleague) becomes

function(){ /* code */ }();
@miketaylr

Maybe the simplest test case is
(function(){ /* code */ }());

Yet (function(){ /* code */ })(); is handled just fine.

@miketaylr miketaylr pushed a commit to miketaylr/slimit that referenced this issue Mar 20, 2013
Mike Taylor Unittests for the dropping parens bug (Issue #42) ceb77c3
@rspivak rspivak closed this Mar 23, 2013
@acatton acatton added a commit to acatton/slimit that referenced this issue Jan 26, 2015
Mike Taylor Unittests for the dropping parens bug (Issue #42) a0886ec
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment