Fork me on GitHub
#cljs-dev
<
2016-11-12
>
thheller11:11:06

just found a problem in shadow-build that I'm pretty sure cljs will have as well, don't have time to verify

thheller11:11:23

ns caching will lose the ns metadata

thheller11:11:09

since it is only available via the [::ana/namespaces ns :name] meta and that is not written to disk

thheller11:11:45

maybe someone else is interested in verifying

thheller11:11:26

so (ns my.ns {:fancy true}) will not have the metadata when loading from cache

thheller11:11:56

not a widely used feature but docstrings are lost as well

anmonteiro12:11:45

Submitted a number of patches today that address some bugs I found out while giving current master a spin

dnolen17:11:06

@thheller seems like a simple thing to fix, we should add :ns-meta to the analysis data

dnolen17:11:13

feel free to file an issue

anmonteiro17:11:02

@dnolen easier to answer you question in CLJS-1848 here. If you look at how the :js-module-index map is structured, you'll get the bug: https://github.com/clojure/clojurescript/commit/1f2c0a922130769a3da398aa501b3621fd1f8cd5#diff-d32ae4da93f4291162e396e053e5ff0dR57

anmonteiro17:11:24

particularly, I was getting a huge number of undeclared var/NS warnings when doing the Om stuff I mentioned in the #om channel

dnolen17:11:09

@anmonteiro but why didn’t we see this with the circle-demo?

anmonteiro17:11:15

hrm let me investigate

anmonteiro18:11:43

@dnolen because it doesn't reach that test

anmonteiro18:11:49

the and clause bails out earlier

dnolen18:11:42

where at js-loaded-ns?

anmonteiro18:11:40

@dnolen though there's something weird

dnolen18:11:56

yeah I’d like to understand what’s going on here

dnolen18:11:02

if there’s something to clean up we should do it

anmonteiro18:11:21

in the circle color project, the prefix is module$resources$public$js$libs$react, in the Om case I tried it was React

anmonteiro18:11:20

so my understanding is the following: when a (processed) JS module calls React, the prefix is module$resources$public$js$libs$react, when we call from a ClojureScript namespace, the prefix is React

dnolen18:11:26

I didn’t encounter what you are saying

anmonteiro18:11:01

oh that's right, I didn't read my full output

anmonteiro18:11:06

my conclusion above is wrong

anmonteiro18:11:15

the problem was in defui, so the problem occurs when analyzing a macro expansion

anmonteiro18:11:37

which leads me to believe the solution should be different from the one in my patch

anmonteiro18:11:53

we probably should add the {React React} case, like we do for namespaces aliases

anmonteiro18:11:03

and keep checking the values in js-module-exists?

dnolen18:11:36

right whatever patch we come up with - we should provide a detailed explanation

anmonteiro18:11:54

does the above sound good?

anmonteiro18:11:06

I can try that and check both projects again

anmonteiro18:11:19

(and provide an explanation as to why we need to do it, of course)

dnolen18:11:32

I’m assuming you’re saying that we always provide an ns as an alias to itself?

dnolen18:11:37

so we need to in this case as well?

anmonteiro18:11:13

when we require any namespace, say cljs.test we add a {cljs.test cljs.test} entry to requires map

anmonteiro18:11:47

which I think we should do in the :js-modules-index too for macroexpansion to work

anmonteiro18:11:50

yeah, I just confirmed the bug happens as a result of macroexpansion

anmonteiro19:11:00

@dnolen updated the issue with the actual fix for the problem http://dev.clojure.org/jira/browse/CLJS-1848

dspiteself20:11:50

We have a use case that makes equiv on a defrecord a hotspot. Would you entertain a patch that makes the IEquiv implimentation that emit-defrecord emits call equiv on all field inline and only do equivmap on extmap ?

dspiteself23:11:01

great either I or @favila will make a ticket and patch soon