Fork me on GitHub
#cljs-dev
<
2016-06-06
>
darwin00:06:40

@danielcompton: good point, I tried to look at figwheel’s implementation of the REPL integration, but didn’t see any logic related to this

darwin00:06:39

but I didn’t spend much time on it, my main focus was investigation if dirac middleware could be integrated with figwheel’s nREPL mode

danielcompton06:06:15

Would a patch to log a warning in dev mode when keywords were compared with identical? be accepted?

dnolen12:06:01

@danielcompton: no there’s a real reason why it doesn’t work

darwin13:06:52

@danielcompton: if you care about this particular one, you could implement it as another check in :sanity-hints of cljs-devtools, by monkey-patching cljs.core.identical_QMARK_

darwin13:06:43

it already does some dirty monkey patching, and is not enabled by default for that reason, people have to explicitly enable it: https://github.com/binaryage/cljs-devtools/blob/master/src/devtools/sanity_hints.cljs

darwin13:06:56

A side note, I’m playing with an idea to use source maps to map the "Cannot read property 'call' of null” type of errors back to original sources. And display it conveniently. That would be pretty nice I think.

danielcompton20:06:54

@dnolen: I think sanity hints was pretty much what I was thinking of here

dnolen20:06:39

@danielcompton: we could add a warning if there is a inline literal but to completely honest this is not something I’m super interested in

danielcompton20:06:55

sure, sanity hints would be as good a place to put it

dnolen20:06:57

there are very few reasons to prefer identical? over =

darwin20:06:46

what exactly is the problem with identical?, I must admit I used to write identical? for keyword checks

dnolen20:06:58

identical? is only for reference equality

dnolen20:06:03

idiomatic code should not be using

dnolen20:06:16

internally it’s only for the most perf critical stuff like the persistent data structures

darwin20:06:58

ok, but could I make mistake that two keywords would have false reference check? (them being created independently somehow)

darwin20:06:19

two same keywords

dnolen20:06:21

this is documented in the difference page

dnolen20:06:26

keywords are never going to be the same object

dnolen20:06:39

and you shouldn’t be using identical? anyway

dnolen20:06:44

if you weren’t using it you wouldn’t have this problem

darwin20:06:24

interesting, I would bet I was using identical? on them successfully, and that was probably just a conicidence

darwin20:06:40

they were just clones

dnolen20:06:44

the rule - don’t use identical?

dnolen20:06:58

unless you writing perf critical code at which point you’re on your own anyway

darwin20:06:26

ok, got it, just wanted to understand the problem

dnolen20:06:40

the very first thing = does is call identical?

darwin20:06:43

so I think we should wanr about it in cljs-devtools, when people install :sanity-hints

dnolen20:06:57

so if two things are the same thing you get the fast path

dnolen20:06:38

warning about something people shouldn’t be doing doesn’t seem like a very useful warning IMO

dnolen20:06:42

which is why I’m not interested in it

darwin20:06:50

but how should they know without understanding the internals? I used to use identical? in wrong places and didn’t know. For me it was like === and I thought rest of cljs works well with it

dnolen20:06:51

which is to say, everyone should be guided to use =

dnolen20:06:29

I believe there are existing Clojure linters that already do this

darwin20:06:59

btw. my route was javascript -> clojurescript, didn’t have chance to build that experience using more advanced tools in clojure land

darwin20:06:32

and clojurescript forced me to touch clojure and JVM, which I resisted with passion 🙂

dnolen20:06:33

I’m fine with higher level linters doing this kind of thing - less interested in ClojureScript getting involved in style linting

darwin20:06:38

ok, fine, I’m not advocating it at this point. Just wanted to know the details and if it would be a candidate for cljs-devtools to handle

darwin20:06:47

thanks for explaining it