Fork me on GitHub
#cljs-dev
<
2016-11-05
>
anmonteiro14:11:40

I'm starting to look http://dev.clojure.org/jira/browse/CLJS-1622, but I'm starting to ask myself if the ticket makes sense

anmonteiro14:11:18

I'll add a comment in JIRA

anmonteiro15:11:21

(def a (sorted-map-by > 1 :a 2 :b))
(get a "a")

anmonteiro15:11:44

this fails because it ends up using the function version of <, which returns false for both (< "a" 1) and (< 1 "a")

anmonteiro15:11:55

so the comparator returns 0

anmonteiro15:11:51

I'm not sure what the right fix is. Something that comes to mind is a runtime check that all the arguments passed to < are numbers

anmonteiro17:11:34

appreciate your feedback

Yehonathan Sharvit19:11:43

@anmonteiro since 1.9.293, I see that (my.ns$macros/xx) evaluates to (my.ns/xx)

Yehonathan Sharvit19:11:41

It causes me a problem with the port of core.match

anmonteiro19:11:56

there's probably a solution you're not seeing 🙂

anmonteiro19:11:06

can you point me to the specific line in the code?

Yehonathan Sharvit19:11:23

As a temporary workaround I referred the cljs.core.match$macros namespace explicitely https://github.com/viebel/core.match/blob/master/src/main/clojure/cljs/core/match.cljc#L81-L91

Yehonathan Sharvit19:11:35

backtrack-expr and backtrack-sym are injected into the code

Yehonathan Sharvit19:11:43

Therefore they are needed at run time

Yehonathan Sharvit19:11:44

The clean solution would be to ensure cljs.core.match is loaded when one requires the cljs.core.match$macros

Yehonathan Sharvit19:11:58

and to define backtrack-expr and backtrack-sym inside cljs.core.match

Yehonathan Sharvit19:11:54

Actually, I think this is the case already

Yehonathan Sharvit19:11:11

in core.match README, the advised way to load core.match is

(ns foo.bar
  (:require [cljs.core.match :refer-macros [match]]))

Yehonathan Sharvit19:11:38

It cause both cljs.core.match and cljs.core.match$macros to be loaded

Yehonathan Sharvit19:11:04

In my test code, I used

(ns my.match·
        (:require-macros [cljs.core.match :refer [match]]))

Yehonathan Sharvit19:11:12

So the bug is in my test code 🙂

anmonteiro19:11:34

@viebel it seems to me that you just need to (:require-macros [cljs.core.match]) in your :cljs branch

anmonteiro19:11:43

I don't see you doing that anywhere?

anmonteiro19:11:14

it's the use case that 1.9.293 allows

Yehonathan Sharvit19:11:22

but then all the functions defined inside the code will pollute cljs.core.match namespace!

anmonteiro19:11:41

which functions?

Yehonathan Sharvit20:11:28

all of the functions defined in match.cljc are needed by the macro namespace

anmonteiro20:11:44

sorry I don't understand what you're saying

Yehonathan Sharvit20:11:00

I mean that for the purpose of macro expansion

Yehonathan Sharvit20:11:10

there are a lot of functions defined

anmonteiro20:11:17

have you tried doing what I said?

Yehonathan Sharvit20:11:22

those functions are not needed at “run-time"

Yehonathan Sharvit20:11:34

What you said will probably work

Yehonathan Sharvit20:11:49

My concern is: is it the proper solution?

anmonteiro20:11:51

so what is the problem? adding size to the build?

anmonteiro20:11:08

it is the proper solution for it to work in regular ClojureScript as well

Yehonathan Sharvit20:11:23

It works like a charm !