Sheetjs: Doesn't compile with Google Closure Compiler anymore

[jscheid]

Hi,

I've been minifying xlsx using Google Closure Compiler for months now, always worked fine. But since upgrading from 0.9.3 to 0.9.6 I'm getting the following error.

stdin:20562: ERROR - constant self assigned a value more than once.
Original definition at externs.zip//browser/window.js:70
        if(typeof self == 'undefined' && typeof app != 'undefined') self = app;

It seems to be caused by this change:

https://github.com/SheetJS/js-xlsx/commit/70c48a74b9efb61b46293ef35f6babe5022e94df#diff-6b0da2a0ba02b003f8c2e16a0e39cbdcR12

The line mentioned in the error message is this one:
https://github.com/google/closure-compiler/blob/ab4617a86346e659253593b11d2570fa26300415/externs/browser/window.js#L68

As far as I gather, Closure Compiler thinks that self here refers to https://developer.mozilla.org/en/docs/Web/API/Window/self which is a read-only property.

This might be a bug in Closure Compiler, but I wonder if it would be possible to use a different variable name?

[SheetJSDev]

Sorry about that! We had to make some changes to the shim to support Adobe products (see https://github.com/SheetJS/js-xlsx/issues/603) and we left the line in. Removing it does not affect the result in extendscript so we will remove the line in the source and push a new version of the module momentarily.

[SheetJSDev]

@jscheid we just released 0.9.7 with the fix. There are 14 warnings which are intentional (including currently-disabled functions with the if(0) pattern). To verify on the web interface:

https://closure-compiler.appspot.com/home#code%3D%252F%252F%2520%253D%253DClosureCompiler%253D%253D%250A%252F%252F%2520%2540compilation_level%2520SIMPLE_OPTIMIZATIONS%250A%252F%252F%2520%2540output_file_name%2520default.js%250A%252F%252F%2520%2540code_url%2520http%253A%252F%252Foss.sheetjs.com%252Fjs-xlsx%252Fxlsx.core.min.js%250A%252F%252F%2520%253D%253D%252FClosureCompiler%253D%253D%250A%250A%252F%252F%2520ADD%2520YOUR%2520CODE%2520HERE%250Afunction%2520hello(name)%2520%257B%250A%2520%2520alert('Hello%252C%2520'%2520%252B%2520name)%253B%250A%257D%250Ahello('New%2520user')%253B%250A%250A

We currently run jshint / jscs / flow checks, but in the future we will be adding a closure check :)

[jscheid]

I've also just tested with 0.9.7, works perfectly now. That was a really quick turnaround, many thanks!

[SheetJSDev]

@jscheid closure actually revealed a bug in flow/jshint! We were getting ready to make a release this week anyway :)

https://github.com/jshint/jshint/issues/3116

https://github.com/facebook/flow/issues/3601