search

BorisMoore / jsviews

Interactive data-driven views, MVVM and MVP, built on top of JsRender templates
http://www.jsviews.com/#jsviews
MIT License
855 stars 130 forks source link

jsViews breaks when minified by Visual Studio Bundler & Minifier extension #323

Closed rdogmartin closed 8 years ago

rdogmartin commented 8 years ago

I don't know if this is a bug in the minifier in the Visual Studio Bundler & Minifier extension or the way jsViews is written, so I'm posting here and in the minifier's discussion area. Please see https://visualstudiogallery.msdn.microsoft.com/9ec27da7-e24b-4d56-8064-fd7e88ac1c40/view/Discussions for my post dated today. It includes a very small repro.

Cheers!

BorisMoore commented 8 years ago

This looks like a minifier bug to me.

For an explanation of the JsViews code, see

http://stackoverflow.com/a/14120023/1054484

In particular, see

It will execute eval() in the global scope because an indirect call to eval() will set its scope to global, as opposed to the calling environment's scope.

To achieve an indirect call, you have to invoke eval() indirectly, i.e. you can't just call it with eval(). You can use (0, eval) to invoke it.

So replacing (0, eval)('this') by eval("this") won't be the same in all environments, but may return the same result in some. (So may give misleading test results).

BorisMoore commented 8 years ago

See also https://github.com/madskristensen/BundlerMinifier/issues/63#issuecomment-147234018

BorisMoore commented 8 years ago

Closing, since this is not a JsViews issue.

BorisMoore commented 8 years ago

@rdogmartin: FYI - I'm not sure whether or when the VS minifier will get a bug fix (or new minifier) to correctly minify (0, eval)('this'). So I have looked at ways to eliminate that line of code from JsViews and JsRender. The next update should have a fix where that eval expression will no longer be used.

rdogmartin commented 8 years ago

Thank you. It's unfortunate you have to spend time on this.

BorisMoore commented 8 years ago

Yes, also it increases the size of JsRender. And if they don't fix it they are stuck with a minifier that is being used in production by VS users, but is unmaintained. And any other framework that uses an indirect call to eval to get the this from global context (which can be a common way of adding a namespace object to window from within a self-executing function) will also be broken for minification.

BorisMoore commented 8 years ago

The latest update (0.9.74) includes a change in order to workaround this minifier issue in VS.

  • JsRender no longer uses the (0, eval)('this') expression to get the window object. This means that it can now be minified by the Visual Studio minifier, in spite of it not correctly minifying this expression. See https://github.com/BorisMoore/jsviews/issues/323