Doesn't compile with Google Closure Compiler anymore #607

New Issue

2017-03-28T21:16:06Z

jscheid commented

2017-03-28 21:16:06 +00:00

(Migrated from github.com)

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:

70c48a74b9 (diff-6b0da2a0ba02b003f8c2e16a0e39cbdcR12)

The line mentioned in the error message is this one:
ab4617a863/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?

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 commented

2017-03-28 21:25:42 +00:00

(Migrated from github.com)

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 commented

2017-03-28 22:20:06 +00:00

(Migrated from github.com)

@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 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 :)

👍 1

jscheid commented

2017-03-28 22:25:33 +00:00

(Migrated from github.com)

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

🎉 2

SheetJSDev commented

2017-03-28 22:44:50 +00:00

(Migrated from github.com)

@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

@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

Sign in to join this conversation.