Fork me on GitHub
#cljs-dev
<
2017-04-07
>
thheller08:04:46

@dnolen about externs inference: what if every . function on a js tagged variable (defn foo [^js/foreign x] (.render x)) automatically inferred the return value as ^js/foreign also? your stuff currently infers Object instead. Just like we have not-native or number we could have foreign :tag that just auto propagates for all . calls.

thheller08:04:52

sure it doesn't work with type checking enabled ... but no one writes correct types for JS anyway

thheller08:04:37

then before optimizing we just generate one foreign externs object

thheller08:04:50

(defn x [^foreign x] (.. x (foo) (bar) (baz))) then works and given that Closure stops renaming properties on any property defined in externs anyway

thheller08:04:30

doesn't matter if you define A.prop1 and B.prop2, prop1 and prop2 will never be renamed on any object

thheller08:04:54

at least as far as I understand things

dnolen16:04:09

@anmonteiro finally got around to reviewing the patch on http://dev.clojure.org/jira/browse/CLJS-1497

dnolen16:04:31

@thheller we infer js actually but the type is Object

dnolen16:04:41

effectively js/Object

anmonteiro16:04:55

@dnolen did you review patch CLJS-1497-2?

anmonteiro16:04:32

so what’s not additive in that patch?

dnolen16:04:57

you cannot move an existing name to a different protocol

dnolen16:04:01

we have checks for stuff like that

anmonteiro16:04:35

I didn’t even see that

anmonteiro16:04:13

so weird, I thought I had done that at some point

anmonteiro16:04:30

we probably even have slack logs of you saying the method should be called -find

dnolen16:04:46

@anmonteiro I think we talked about it a long while ago, but probably got lost in your enthuasism for :npm-deps 😉

dnolen16:04:28

if people have other patches happy to look at them

dnolen16:04:45

@favila et al. OK, we should get that info into a ticket

thheller16:04:32

fixes a problem where calls to atom deref and the others are never fn-var and never properly called

thheller16:04:10

since declare overwrites the defn

dnolen16:04:35

@thheller thanks will add it to my queue

dnolen16:04:50

@anmonteiro the test in find seems a bit strange to me

dnolen16:04:01

it seems to me it ifind? should be inside

anmonteiro16:04:00

sorry I don’t follow

anmonteiro16:04:33

I just replaced associative? with ifind?

dnolen16:04:41

yeah so this is also going to break things 🙂

dnolen16:04:54

remember we don’t want to mess with collections we didn’t write and we don’t know about

anmonteiro16:04:04

ah custom collections

dnolen16:04:28

other than that, looks OK

anmonteiro16:04:45

does this look better?

(if (ifind? coll)
  (-find ...)
  (whatever-was-there-before))

anmonteiro16:04:03

cool, changing now

anmonteiro16:04:51

@dnolen OK attached CLJS-1497-4.patch addressing that comment too 🙂

dnolen17:04:17

applied, thanks!

favila23:04:45

@anmonteiro isn't find supposed to return nil on not-found?

anmonteiro23:04:48

@favila yeah, are we not doing that?

anmonteiro23:04:28

I’ll look into it during the weekend

favila23:04:00

oh, the outer find does a contains? check

favila23:04:20

I thought the contract with -find was the same as find

favila23:04:33

IOW I thought -find was trying to prevent double-lookup

favila23:04:40

the advantage of letting -find do lookup, maybe return nil is the collection has the chance to do k+v retrieval once, if it supports

favila23:04:06

but I'm not even sure Clojure bothers do to that

favila23:04:23

nm, looks like entryAt usually doesn't do double-lookup in Clojure