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.

Sign up for GitHub

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

Jump to bottom

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

Closed
rdogmartin opened this issue Oct 7, 2015 · 7 comments
Closed

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

rdogmartin opened this issue Oct 7, 2015 · 7 comments

Comments

@rdogmartin
Copy link

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
Copy link
Owner

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
Copy link
Owner

See also madskristensen/BundlerMinifier#63 (comment)

@BorisMoore
Copy link
Owner

Closing, since this is not a JsViews issue.

@BorisMoore
Copy link
Owner

@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
Copy link
Author

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

@BorisMoore
Copy link
Owner

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 BorisMoore mentioned this issue Feb 18, 2016
Closed
BorisMoore added a commit to BorisMoore/jsviews.com that referenced this issue Mar 20, 2016
@BorisMoore
Commit 74 (v0.9.74 - Beta)
1955752 
Updates and new documentation topics:

- Important updates and improvements to $.views.settings APIs:
  settings.allowCode(), settings.delimiters(), settings.debugMode() plus
  new settings.advanced(). See new documentation at
  www.jsviews.com/#settings and www.jsviews.com/#jsvsettings

- Support for unobserve() is now complete (and simplified): See updated
  documentation topic: www.jsviews.com/#unobserve

- Support for namespaces associated with observable changes is
  now complete: See new documentation topic: www.jsviews.com/#namespaces

- Support for error handling and debugging improved and extended, with
  some small changes to APIs - including for $.views.settings.debugMode,
  with full documention at www.jsviews.com/#onerror

- {{>}} is now equivalent to {{>#data}}

Minor breaking changes:

- $.observe(arr, "length", callback) now only listens to length, not to
  array changes but $.observe(arr, arr, "length", callback) listens to
  both length and array changes

- Namespaces with observeAll:
  $.observable(objectOrArray).observeAll(namespace, handler) is now
  written: $.observable(namespace, objectOrArray).observeAll(handler) -
  and similarly for .unobserveAll()

- JsRender and JsViews no longer use the (0, eval)('this') expression
  to get the window object. This means that they can now be minified
  by the Visual Studio minifier, in spite of it not correctly minifying
  this expression. See BorisMoore/jsviews#323

Bug fixes:

- BorisMoore/jsviews#334 (computed
  observable - depends)

- Several additional small bug fixes

- This update also includes a security fix
BorisMoore added a commit that referenced this issue Mar 20, 2016
@BorisMoore
Commit 74 (v0.9.74 - Beta)
de0fd3f 
Updates and new documentation topics:

- Important updates and improvements to $.views.settings APIs:
  settings.allowCode(), settings.delimiters(), settings.debugMode() plus
  new settings.advanced(). See new documentation at
  www.jsviews.com/#settings and www.jsviews.com/#jsvsettings

- Support for unobserve() is now complete (and simplified): See updated
  documentation topic: www.jsviews.com/#unobserve

- Support for namespaces associated with observable changes is
  now complete: See new documentation topic: www.jsviews.com/#namespaces

- Support for error handling and debugging improved and extended, with
  some small changes to APIs - including for $.views.settings.debugMode,
  with full documention at www.jsviews.com/#onerror

- {{>}} is now equivalent to {{>#data}}

Minor breaking changes:

- $.observe(arr, "length", callback) now only listens to length, not to
  array changes but $.observe(arr, arr, "length", callback) listens to
  both length and array changes

- Namespaces with observeAll:
  $.observable(objectOrArray).observeAll(namespace, handler) is now
  written: $.observable(namespace, objectOrArray).observeAll(handler) -
  and similarly for .unobserveAll()

- JsRender and JsViews no longer use the (0, eval)('this') expression
  to get the window object. This means that they can now be minified
  by the Visual Studio minifier, in spite of it not correctly minifying
  this expression. See #323

Bug fixes:

- #334 (computed
  observable - depends)

- Several additional small bug fixes

- This update also includes a security fix
BorisMoore added a commit to BorisMoore/jsrender that referenced this issue Mar 20, 2016
@BorisMoore
Commit 74 (v0.9.74 - Beta)
f984e13 
Updates and new documentation topics:

- Important updates and improvements to $.views.settings APIs:
  settings.allowCode(), settings.delimiters(), settings.debugMode() plus
  new settings.advanced(). See new documentation at www.jsviews.com/#settings

- Support for error handling and debugging improved and extended, with
  some small changes to APIs - including for $.views.settings.debugMode,
  with full documention at www.jsviews.com/#onerror

- {{>}} is now equivalent to {{>#data}}

Minor breaking changes:

- 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 BorisMoore/jsviews#323

Bug fixes:

- Several small bug fixes

- This update also includes a security fix
@BorisMoore
Copy link
Owner

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

@BorisMoore BorisMoore mentioned this issue Mar 29, 2016
Closed
@BorisMoore BorisMoore mentioned this issue Jun 18, 2016
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@BorisMoore @rdogmartin