Fork me on GitHub
#cljs-dev
<
2019-11-08
>
borkdude10:11:13

I'm looking at the slash import issue now (CLJS-3177, https://clojure.atlassian.net/browse/CLJS-3177), that we discussed a few days ago:

$ cat import.cljs
(ns test.imports
  (:import [goog.math Integer]))

(println (Integer/fromString "123"))
$ clj -m cljs.main -re node -i src/test/import.cljs
#object[Object 123]
It seems this already works?

borkdude10:11:57

Compiling also seems to work:

$ clj -R:cljs/dev -m cljs.main -t node -c test.imports
$ node out/main.js
#object[Object 123]

borkdude10:11:14

So maybe I can just close the issue? Or are there other problems?

borkdude11:11:43

^ @thheller you might know

thheller11:11:19

dunno. I guess its fine if it already works? I didn't actually test it 😛

borkdude11:11:49

maybe it doesn't work with things that are not inferred as a class using :ctr...

thheller11:11:46

I just tested in shadow-cljs and that has none of the new inference stuff since it still uses 1.10.520

thheller11:11:51

so seems fine

borkdude11:11:02

ah ok. a no-op issue then

borkdude11:11:35

I'll assign dnolen to let him be the judge to close it

Roman Liutikov11:11:20

Getting WARNING: Cannot infer target type in expression (. (g (inc 1)) catch identity) for this code. Looks like a bug? Happens in both master and 1.10.520

(defn f [g]
  (-> ^js/Promise (g (inc 1))
      (.catch identity)))

(f #(.resolve js/Promise %))

Roman Liutikov11:11:41

The warning is gone when g is replaced with core fn or any defn, or when inner expression (inc 1) is replaced with random value for example 1

thheller11:11:41

IIRC the .catch rewrite code loses :tag info. at least I have a vague memory of something like that.

thheller11:11:16

try if it warns for the other variant too

(defn f [g]
  (.catch ^js/Promise (g (inc 1)) identity)
  (. ^js/Promise (g (inc 1)) (catch identity)))

Roman Liutikov11:11:37

warns with any random method call, not just catch

Roman Liutikov11:11:46

also warns for both lines in the above sample

thheller11:11:04

hmm ok. may just be a missed case for externs inference

Roman Liutikov11:11:17

wtf, warns here as well

(defn f [g]
  (.-hello ^js/Promise (g (inc 1))))

thheller11:11:26

(defn f [g]
  (let [^js/Promise x (g (inc 1))]
    (.-hello x)))

thheller11:11:43

shouldn't right?

Roman Liutikov11:11:44

yeah this one is fine, except that it complains about hello prop not known on Promise type, but that’s expected

thheller11:11:49

I rewrote the . handling in shadow-cljs partly because of some externs inference quirks. I can't remember the details though.

Roman Liutikov12:11:03

@mfikes shouldn’t cljs version of cljs.core/str have string return tag? Its [x & ys] method has tag any (probably because of .toString call as return value)

mfikes12:11:37

Oh, maybe there is a bug there?

mfikes12:11:03

Ahh, a bug that you can see only with the apply feature you have?

mfikes12:11:21

(Otherwise the macro takes care of things.)

Roman Liutikov12:11:02

yeah, apply, comp and partial

mfikes12:11:27

Perhaps, with higher order inference, there will be core functions other than str that need adjustment as well.

Roman Liutikov12:11:29

I wonder if Closure Library type information added to cljs could solve the issue with str, since it’s return value is .toString?

Roman Liutikov12:11:41

otherwise we’d need return type hint

mfikes12:11:16

Yeah, that’s an interesting general thing: Anyone calling (.toString x)

mfikes12:11:07

Based on @roman01la’s focus: Addressing apply, comp, and partial might open up huge swaths of type inference where you have chains of function composition where the inference chain breaks with the current compiler implementation—this could fix that aspect 🙂

simple_smile 4
mfikes12:11:15

(This could be huger than we might initially think.)

Roman Liutikov12:11:27

Could be interesting to have compiler optimization that rewrites (apply str args) as (clojure.string/join args) or something along those lines?

Roman Liutikov12:11:38

I think with apply we end up with two iteration cycles, one in apply and other in str fn

mfikes12:11:40

In my opinion, yes, those are essentially intrinsics. There are a handful, now, done at the emission stage.

mfikes12:11:16

(If we end up with more, it might make sense to factor out an intrinsics templating engine of some sort?)

mfikes12:11:16

Here is where the intrinsics are implemented: https://github.com/clojure/clojurescript/blob/master/src/main/clojure/cljs/compiler.cljc#L1140 (There are more in JIRA tickets, IIRC.)

thheller17:11:03

@dnolen @mfikes before wrapping up the next release please consider including https://clojure.atlassian.net/browse/CLJS-3077

👍 4
borkdude17:11:56

talking about wrappers. would it be possible to make MetaFns less wrapped, so they become callable from JS?

🙏 8
dnolen18:11:12

I don't really see any way to make that work

dnolen18:11:27

but if somebody has a good idea let me know

dnolen18:11:40

but my opinion is that if you're using MetaFn interop is not your primary concern

dnolen18:11:17

@thheller the patch could use a better description

dnolen18:11:28

so you use the second pass when you know there won't be a recur?

thheller19:11:19

@dnolen it tracks whether it is in a loop or not. if in a loop it "captures" the locals in an extra wrapper fn, if not it doesn't need to.

thheller19:11:53

there is no added "second pass"?

dnolen20:11:10

@thheller but how can you know you're in a loop if the recur is deep in the body?

thheller20:11:49

the analyzer figures that out? the emit* :fn just checks later

thheller20:11:17

+                 (when (or in-loop (seq recur-params))
+                   (mapcat :params loop-lets))

thheller20:11:26

(seq recur-params) will be true-ish

dnolen20:11:41

oh right this is just during emission

dnolen20:11:51

ok will review it more closely when I have time

👍 4
Filipe Silva23:11:17

would be cool if https://clojure.atlassian.net/browse/ASYNC-230 was in the release as well

Filipe Silva23:11:27

was also thinking of adding to the guide how promise.all/race/allSettled can be done with go blocks tomorrow