Fork me on GitHub
#cljs-dev
<
2017-01-09
>
dnolen00:01:04

@ewen re: the other thing, return array-map instead - also sounds fine, open a minor ticket + patch for that if you like

ewen09:01:41

@dnolen what about subvec ? any chance we could get either the docstring updated, or subvec to check its argument type, or better, ranged iterator to only use the vector protocols ?

r0man14:01:17

Is this expected or a bug (undefined? (clj->js js/undefined)) => false?

tmulvaney14:01:52

@ewen i'm not sure if ranged-iterator is for public use

dnolen15:01:26

@ewen also this doesn’t seem particularly important to me - a trivial ticket is OK but it’s unlikely to ever get bumped up higher in priority

ewen16:01:43

@tmulvaney ranged iterator is not public but subvec is, and subvec uses ranged iterator. This means that using subvec with a custom vector implementation won't work

dnolen16:01:39

@ewen a patch that emulates what Clojure does would be acceptable

dnolen17:01:40

@r0man undefined isn’t a ClojureScript value - so that seems like a questionable thing to me at the moment

dnolen17:01:14

round tripping isn’t an expectation here either

r0man17:01:57

@dnolen I run into this while working with React. They use js/undefined as the value for uncontrolled inputs. I had a map like {:value js/undefined} and called clj->js on it to pass it to React. I would have expcted clj->js to return #js {:value js/undefined}, but got #js {:value nil}. You can't spot this very quickly because they print both as #js {:value nil} in the REPL. I'm also not quite sure if clj->js should convert it or not. Just wanted to mention it.

dnolen17:01:20

right, but not convinced that’s a good reason to handle it

dnolen17:01:50

we don’t really treat js/undefined at all

dnolen17:01:58

we consistently coerce it to nil

dnolen17:01:14

so so far, I have little interest in adding a special case here

dnolen17:01:01

especially for a goofy React-ism

dnolen17:01:16

broader use case would make it more convincing

dnolen17:01:24

but I think you’ll have a very hard time coming up with something

r0man17:01:33

I would argue that clj->js shouldn't be destructive or loose more information than necessary. Converting nil and js/undefined to both nil is loosing information. You can't figure out the difference afterwards. So that would be an argument for not converting. But I agree it's questionable. 🙂

dnolen17:01:29

yeah not convinced

rauh17:01:55

Is there a reason to-array and into-array don't prealloc the array size?

dnolen17:01:26

@rauh no evidence that it’s faster

dnolen17:01:19

and by faster I mean - benchmarks on 3-4 major JS engines that demonstrate gains

rauh17:01:07

I see. I mean sometimes in cljs.core the arrays are preallocated. Sometimes not

dnolen17:01:34

JS engines already prealloc, otherwise wish .push would be slow

rauh17:01:38

Also, a lot of the {object,long,int}-array function duplicate the code. Could this all be mapped to one function?

dnolen17:01:49

it could but it’s not important

rauh17:01:12

Yeah I understand they usually double the underlying memory to have amortized cost of O(1)

dnolen17:01:30

pre-alloc only really benefits one specific case in ClojureScript

dnolen17:01:53

PersistentHashMap needs dense arrays, so we prealloc + populate with nil

dnolen17:01:34

and that was when we tested a couple years ago

dnolen17:01:40

everything might have changed by now

dnolen17:01:46

all we knows is the benchmarks didn’t get slower

rauh17:01:09

Yeah I agree.

r0man17:01:43

@dnolen Do you think it makes sense to print js/undefined different than nil?

dnolen17:01:09

the answer to what should we do about js/undefined should be nothing

r0man17:01:34

ok, I guess it's called undefined for a reason then 😉

dnolen17:01:36

if it’s not nothing - then you need to come up with something that affects a wide variety of cases not just the one you are having

dnolen17:01:12

10 years of JS and 5 years of ClojureScript have convinced me the distinction in JS is abomination

dnolen17:01:22

the tradeoff is some friction in libs like React

dnolen17:01:32

but it’s not like React’s idiom is all that common

r0man17:01:06

it's not too important to me. I was just wondering if things like that could be made more user friendly.

dnolen17:01:30

you do realize that changing how js/undefined prints now is also pretty much going to be disaster

dnolen17:01:42

so no - remove this idea from your mind 🙂

r0man17:01:32

No, I think I don't understand.

dnolen17:01:43

printing is serialization

r0man17:01:32

Hmm, but aren't there other unreadable things things that we also print? Or do you mean the chance we break other code is too high?

r0man17:01:36

ok, just wanted to know the reasoning behind. thanks

rauh18:01:44

I just solved this one for CLJS: http://dev.clojure.org/jira/browse/CLJ-1402 ~5x speedup for a relatively simple :x keyfn, so can potentially speed up much more depending on how expensive keyfn is

rauh18:01:16

Only really worth it for large collections

rauh18:01:07

On a 10k element collection, the keyfn is called by Chrome's sort implementation ~270k times.

andrewhr18:01:51

@rauh for expensive keyfn, isn’t memoize good enough? https://clojuredocs.org/clojure.core/memoize

rauh18:01:04

@andrewhr Well given that avoiding the map lookup resulted in 5x speedup and memoize itself does a map lookup. Probably not

angusiguess23:01:25

Okay! I've written a patch for http://dev.clojure.org/jira/browse/CLJS-1453. I'd appreciate any feedback about the approach.

dnolen23:01:28

@angusiguess please remove all whitespace changes from the patch

dnolen23:01:10

cursory check, looks good - I’ll look at more closely when we get a cleaned up patch

angusiguess23:01:06

Done and done 🙂