ClojureScript

"async" needs special munging since Feb2017 closure compiler

Details

  • Type: Defect Defect
  • Status: Closed Closed
  • Priority: Blocker Blocker
  • Resolution: Completed
  • Affects Version/s: None
  • Fix Version/s: None
  • Component/s: None
  • Labels:
    None
  • Patch:
    Code and Test

Description

I tried to release a new chromex library version and hit strange errors generated by new Closure compiler:

Feb 25, 2017 9:16:59 PM com.google.javascript.jscomp.LoggerErrorManager println
    SEVERE: /Users/darwin/code/chromex/test/.compiled/optimizations_advanced/cljs/core/async.js:1418: ERROR - Parse error. No newline allowed before '=>'
    var inst_18944 = async(inst_18943);
                                      ^
    
    Feb 25, 2017 9:16:59 PM com.google.javascript.jscomp.LoggerErrorManager printSummary

The problem is that "async" is kind of a reserved word in new Closure compiler. And code like this[1] will newly break advanced compilation.

I tried to fix it by adding "async" into list of js-reserved keywords in ClojureScript compiler, but this introduced another issue. Namespaces are subject of munging, so usage of goog.async.* stuff will break[2]. Also the compiler emitted a bunch of warnings about using reserved keywords in namespace names. I saw no easy way how to distinguish those cases. At least I created a dirty solution by hard-coding some extra logic into `munge` implementation just to fix my use-case. But in general this will probably require more precise solution.

[1] https://github.com/clojure/core.async/blob/d17eb8a50cfb022b83c0ba53715451a8fe2fd5fa/src/main/clojure/cljs/core/async.cljs#L270
[2] https://github.com/clojure/core.async/blob/d17eb8a50cfb022b83c0ba53715451a8fe2fd5fa/src/main/clojure/cljs/core/async/impl/dispatch.cljs#L29

Activity

Hide
Antonin Hildebrand added a comment - - edited

I marked this as a blocker because core.async is heavily used and it newly breaks due to this issue in advanced mode.

You might want to grab the test and patches from here:
https://github.com/clojure/clojurescript/compare/master...darwin:cljs-1954

Although that fixed the immediate problems in my scenario, I assume you want to handle namespace munging in a more robust way. In rare cases there might be user-specified closure modules with "async" in their names as well, not only goog.async.

Show
Antonin Hildebrand added a comment - - edited I marked this as a blocker because core.async is heavily used and it newly breaks due to this issue in advanced mode. You might want to grab the test and patches from here: https://github.com/clojure/clojurescript/compare/master...darwin:cljs-1954 Although that fixed the immediate problems in my scenario, I assume you want to handle namespace munging in a more robust way. In rare cases there might be user-specified closure modules with "async" in their names as well, not only goog.async.
Hide
Antonin Hildebrand added a comment - - edited

This is a dupe of CLJS-1953. Sorry, didn't see that.

Show
Antonin Hildebrand added a comment - - edited This is a dupe of CLJS-1953. Sorry, didn't see that.
Hide
Brandon Bloom added a comment -

This seems like a Google Closure Compiler bug: It shouldn't choke on "async" unless it precedes "function" or an arrow lambda (=>), since async isn't a reserved word (as far as I can tell), only a contextual keyword. Might get a quick response/fix if you file an issue over at https://github.com/google/closure-compiler

CLJS can probably workaround this, and arguably should, since if not invalid, using "async" as an identifier" is at least confusing with traditional syntax highlighting tools, etc.

Show
Brandon Bloom added a comment - This seems like a Google Closure Compiler bug: It shouldn't choke on "async" unless it precedes "function" or an arrow lambda (=>), since async isn't a reserved word (as far as I can tell), only a contextual keyword. Might get a quick response/fix if you file an issue over at https://github.com/google/closure-compiler CLJS can probably workaround this, and arguably should, since if not invalid, using "async" as an identifier" is at least confusing with traditional syntax highlighting tools, etc.
Hide
António Nuno Monteiro added a comment -

This has been reported and a fix is in progress. Here's the ticket: https://github.com/google/closure-compiler/issues/2336

Show
António Nuno Monteiro added a comment - This has been reported and a fix is in progress. Here's the ticket: https://github.com/google/closure-compiler/issues/2336
Hide
Len Weincier added a comment -

I see the closure-compiler issue is closed. Whats the process for this to hit Clojurescript ?

Show
Len Weincier added a comment - I see the closure-compiler issue is closed. Whats the process for this to hit Clojurescript ?
Hide
Dieter Komendera added a comment -

@Len as far as I know we first have to wait for a new release of the Closure compiler until we can bump it as done in http://dev.clojure.org/jira/browse/CLJS-1952

Show
Dieter Komendera added a comment - @Len as far as I know we first have to wait for a new release of the Closure compiler until we can bump it as done in http://dev.clojure.org/jira/browse/CLJS-1952
Hide
David Nolen added a comment -
Show
David Nolen added a comment - fixed by http://dev.clojure.org/jira/browse/CLJS-2006

People

Vote (8)
Watch (10)

Dates

  • Created:
    Updated:
    Resolved: