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

Update Closure Compiler to the latest version (v20160822) #444

Closed
wants to merge 2 commits into from

Conversation

kranich
Copy link
Member

@kranich kranich commented Aug 24, 2016

This is the upcoming release of Closure Compiler.

--rewrite_polyfills is now on by default. This caused some problems with CindyGL that I could fix by explicitly deactivating --rewrite_polyfills for CindyGL.

I have tested some examples of CindyJS core, Cindy3D, and CindyGL, all of which seem to work now.

@gagern
Copy link
Member

gagern commented Aug 24, 2016

Can you be specific about the kinds of problems polyfill rewrites caused for you? So that we know what to look for in the future to see whether these issues have been fixed? I tried the code without the flag, and could't see any issues immediately.

@kranich
Copy link
Member Author

kranich commented Aug 25, 2016

Can you be specific about the kinds of problems polyfill rewrites caused for you? So that we know what to look for in the future to see whether these issues have been fixed? I tried the code without the flag, and could't see any issues immediately.

For example, in examples/cindygl/08_julia.html (probably in all CindyGL examples), the following errors are thrown in Firefox:

TypeError: invalid 'in' operand ba CindyGL.js:2:408
Plugin CindyGL not found Cindy.js:465:1
Called undefined function colorplot (as colorplot$2) Cindy.js:203:474

The last one many, many times. And in Chromium (possibly a bit more helpful):

polyfill] :36Uncaught TypeError: Cannot use 'in' operator to search for 'String' in undefined(anonymous function) @ polyfill] :36(anonymous function) @ ShaderProgram.js:177
Operators.js:3899 Plugin CindyGL not found
809Setup.js:754 Called undefined function colorplot (as colorplot$2)

@kranich
Copy link
Member Author

kranich commented Aug 25, 2016

Maybe @montaga wants to investigate this further?

@gagern
Copy link
Member

gagern commented Aug 25, 2016

I looked at the code. The relevant portion can be cleaned up to something like this:

var ba;
if ("undefined" != typeof window && window === this) ba = this;
else if ("undefined" != typeof global) ba = global;
else ba = this;
var ca = ["String", "prototype", "repeat"];
for (da = 0; da < ca.length - 1; da++) {
    var ea = ca[da];
    ea in ba || (ba[ea] = {});
    ba = ba[ea]
}
var fa = ca[ca.length - 1];
var ga = ba[fa];
var ha = ga ? ga : function(a) {  }

So this is an attempt to install the String.prototype.repeat method, but it fails because our wrapper does not maintain this. So out this is undefined, not the global object. I'll propose an alternate PR shortly, addressing this issue.

gagern added a commit to gagern/CindyJS that referenced this pull request Aug 25, 2016
This is required to make polyfills work, as discussed in
CindyJS#444 (comment)
@kranich
Copy link
Member Author

kranich commented Aug 28, 2016

Superseded by #450.

@kranich kranich closed this Aug 28, 2016
@kranich kranich deleted the closure-compiler branch September 5, 2016 19:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants