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

Closure Compiler Error 'Class names defined inside a function cannot be reassigned' #846

Closed
superstructor opened this issue Feb 21, 2021 · 9 comments

Comments

@superstructor
Copy link
Contributor

Require of NPM package @projectstorm/react-diagrams causes compiler errors 'Class names defined inside a function cannot be reassigned'.

e.g.

npm install @projectstorm/react-diagrams
npm install closest lodash react dagre pathfinding paths-js @emotion/core @emotion/styled resize-observer-polyfill
(ns example
  (:require
    ["@projectstorm/react-diagrams" :refer (DefaultLinkModel DefaultNodeModel DiagramModel)]
    ["@projectstorm/react-canvas-core" :refer (CanvasWidget)]))

Caused by Es6RewriteClass.java:489 which was subsequently removed as of release v20201006 in this commit with the comment:

Delete unnecessary CLASS_REASSIGNMENT check.
There shouldn't be a reason that specifically classes can't be reassigned, even if the original assignment is redundant. Additionally, the check had a bug related to shadowing of simple names.

Note I also found at least one other reference to the same issue when searching at rough-stuff/wired-elements#147

@thheller
Copy link
Owner

We currently cannot upgrade to the latest closure-compiler version since cljs.closure still references some stuff that was removed and causes exceptions on startup. This needs to be fixed in CLJS itself first.

@alexisvincent
Copy link

Is there a way to get around the issue with a hotfix somehow?

@superstructor
Copy link
Contributor Author

@alexisvincent The issue is that ClojureScript itself (not shadow-cljs) has bindings to a specific version of Google Closure Compiler that does not have the fix. So @thheller can not fix this in shadow-cljs.

What I may do, is fork Google Closure Compiler and release a shadow-cljs compatible JAR with that one check removed (i.e. 'backport' the upstream fix to an older Google Closure Compiler release). Then interested users can override the Google Closure Compiler dep to use that instead of the official release.

@thheller
Copy link
Owner

thheller commented Mar 1, 2021

With 2.11.19 it is possible to use the latest Closure Compiler version by putting a cljs/closure.clj in one of your source paths with just (ns cljs.closure). shadow-cljs itself doesn't use that ns so its fine if you just use a placeholder. Of course that will break stuff if you end up using code that actually needs that ns. Maybe some cider-nrepl middleware does, I'm not sure.

@alexisvincent
Copy link

alexisvincent commented Mar 1, 2021

@superstructor Thanks! But Thomas helped me with a fix. Do the thing he describes above with

com.google.javascript/closure-compiler-unshaded {:mvn/version "v20210202"}
thheller/shadow-cljs {:mvn/version "2.11.19"}}}}}

@superstructor
Copy link
Contributor Author

@thheller @alexisvincent Thanks for the workaround, that is a good option to have available for most users.

Unfortunately breaks on binaryage/cljs-oops because it has macros that use the cljs.closure ns.

In that case I found custom build of the Closure Compiler with that one upstream commit back-ported to it is an option that works for code that still depends on cljs.closure. I havn't published a JAR but the source can be found here.

@thheller
Copy link
Owner

thheller commented Mar 3, 2021

David Nolen is currently working on getting the Closure Library and Closure Compiler compatible with CLJS in the bump-closure branch. Once thats all sorted I'll make all the necessary adjustments to get shadow-cljs compatible as well ASAP. There might be some hidden ones lurking that I missed.

@alexisvincent
Copy link

@thheller Seems like bump closure has been merged, no release yet though

@thheller
Copy link
Owner

This should be ok now with 2.12.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants