cljs-node-io
cljs-node-io copied to clipboard
Seems not to be advanced optimizations safe
It seems the node-externs does not support enough info for advanced optimizations.
I do not think :advanced has ever been officially supported for nodejs because of how google closure handles native requires. This documented with GC users outside of the CLJS community as well. However, the way to get around this is to replace all requires with something that will dynamically bind to the native require at runtime (and therefore not be munged away by GC). Something along the lines of
(def my-require (js/eval "require"))
(def https (my-require "https"))
Ive had success monkey patching things like this in the past but YMMV
I´m surprised, I did not get any issues using node-js libs with defined externs. For my last project I had planned using this library. After noticing advanced optimizations munged functions of the lib, I wrote a little namespace myself with the io stuff I need without any problems. I may not get the point but I get my interop working with the following backgrounds:
- https://clojurescript.org/reference/advanced-compilation
- https://clojurescript.org/guides/externs
- https://lwhorton.github.io/2018/10/20/clojurescript-interop-with-javascript.html
- https://cljs.github.io/api/cljs.core/js-invoke
See the example code which works with advanced optimizations:
(ns jtk-dvlp.io
(:require
["path" :as path-api]))
(defn parent-via-direct-interop
[path]
(js-invoke path-api "dirname" path))
(defn parent-via-auto-gernerated-externs
[path]
(.dirname ^js/path-api path-api path))
ok thankyou for pushing back on this :pray:
I have the tests passing with advanced on master with the externs folder. give it a try with those
The externs are only as good as the coverage though, so if something is missing let me know. Alot of the stuff i found around the web is very out-of-date, so i had to add stuff by hand.
Mmh following example did not work in advanced for me but without optimization. Does this work for you in advanced?
(defn parent
[path]
(let [file (io/file path)
parent (.getParentFile file)]
(.getAbsolutePath parent)))
nor
(defn parent
[path]
(let [file ^File (io/file path)
parent ^File (.getParentFile file)]
(.getAbsolutePath parent)))
i think the semantics for getAbsolutePath were off so i tweaked it abit on master. it passes in advanced. If you still get an error could you post it
I have also encountered an optimization issue with cljs-node-io when using shadow-cljs to release a node-script. I am making good use of cljs-node-io and appreciate the good work. Thank you!
The code runs as expected in development, but calling cljs-node-io.async/readable-onto-ch
fails when running an optimized release, with an error reported as "Right-hand side of 'instanceof' is not an object".
I have isolated this issue to (instance? stream.Readable emitter)
in cljs-node-io.async. Changing this to (instance? (.-Readable stream) emitter)
makes the call to cljs-node-io.async/readable-onto-ch
complete as expected also in the optimized release.
Curiously, just including (.-Readable cljs-node-io.async/stream)
in my own code fixes the issue, even with no change to cljs-node-io.async. It seems like this could be a compiler/optimization issue.
I suggest changing (instance? stream.Readable emitter)
in cljs-node-io.async to (instance? (.-Readable stream) emitter)
. Also, perhaps adding ^js
type hints to aid externs inference could eliminate such optimization issues.
I'm glad it's useful. Yes for whatever reason, you used to be able to get away with the dot syntax but it seems something changed upstream awhile ago and I failed to keep up with it, so I appreciate the usage report
Before closing this I'd like to derive externs directly from the nodes docs so they are known to be exhaustive, and then I think ref them with a deps.cljs file? Then I'll add better tests for adv and add CI
Not going to happen this month but I'll get there
new jar "2.0.339" should let you adv compile without manually specifying externs. Pretty sure I swapped over all the problematic interop sites, if not we can re-open this. Thanks @jtkDvlp