Closed shaunlebron closed 6 years ago
@shaunlebron this sounds great, I assume this doesn't have any impact on externs (i.e. they're still required)?
Externs are no longer required in this case. The CommonJS/AMD/ES2015 module gets converted into a Google Closure Compiler module and can be consumed as such.
Blocked by: http://dev.clojure.org/jira/browse/CLJS-1762 and probably other bugs will be found once Closure compiler has been updated.
Currently module-type support is not good for other than very simple tests cases. For example npm modules can't be used.
Now that there's :module-type
support in ClojureScript since 1.9.454 (https://clojurescript.org/reference/javascript-module-support) would the be interest to support this option in cljsjs?
Closure/ClojureScript doesn't currently support Node module resolution in classpath (e.g. Cljsjs JARs). Either files would need to be copied to filesystem in Cljs compiler, or something (Cljs, Cljsjs) should provide Closure node module resolution implementation which works with classpath.
I see, thanks.
Does the :npm-deps
option solve this now?
It doesn't solve this, it works around this.
I'd still prefer solution which doesn't involve calling npm
and instead packages the files to JARs.
Also, when using :npm-deps
, it doesn't make much sense to provide those in packages, it is easy for user to declare :npm-deps
in the project. But for this use case we could provide separate externs packages.
I don't think there is anything to do related to Cljsjs here. Deps.cljs can be generated with :module-type
entries, but I think ClojureScript compiler and Closure only work if those files are available in filesystem, not classpath. This will need to be fixed elsewhere.
I think it would be ideal to replace this (which doesn't even work on Node):
with this, by allowing an option
{:module-type :common-js}
:This option enables us to have the Closure compiler transform the library into a Closure module, rather than just pasting the library into the compiled output. Pending a future patch for a bug found in this process, I think supporting this new
module-type
option would be nice.Background references: