Fork me on GitHub
#cljs-dev
<
2018-06-01
>
dnolen12:06:08

@symfrog probably something simple since that’s exactly a case it’s supposed to handle

dnolen12:06:22

if we can’t find the var - no big deal we just trust it exists

symfrog13:06:17

@thheller thanks! That successfully reproduces it. The issue does not occur when setting the clojurescript dependency to 1.9.946 in deps.edn, so this seems to be a 1.10.238 (and master) issue. Do you know what about your example triggers the issue?

symfrog13:06:45

@dnolen The issue seems to cause an invocation of a function across module boundaries to fail at runtime when :advanced compilation is used, which means that :modules and :advanced compilation can effectively not be used together in 1.10.238 in conditions that trigger the issue? Should I open a Jira issue for this now that a minimal repro is available?

dnolen14:06:39

@symfrog yes please, just copy the contents of the repro into the ticket though - don’t link out

dnolen14:06:02

thanks much!

dnolen17:06:52

@symfrog thanks but the report is pretty problematic

dnolen17:06:01

I removed all the speculation for now

dnolen17:06:33

I’ve compiled the thing and I cannot understand what the problem is or what I should be looking for

dnolen17:06:46

probably because I don’t really see how @thheller’s example is reproducing anything

dnolen17:06:58

going to leave this alone for now until someone can provide more useful information

dnolen17:06:24

@mfikes trying to reproduce 2763 also seems to show that the git caching stuff seems broken

dnolen17:06:02

if you try to build that repro you get a out/cljs_deps.js file with relative paths to cache directory - that can’t be right

dnolen17:06:14

if I run the build again (blowing away out) things look good

mfikes17:06:09

@dnolen OK. I'll probably be able to look into it sometime over the next week. (If urgent, perhaps we could revert the git deps feature.)

dnolen17:06:45

@mfikes it breaks anybody reporting via master

dnolen17:06:56

I guess without disabling caching explicitly

dnolen17:06:37

@symfrog yep, I’m not convinced about your problem and I think @thheller thing is something different

dnolen18:06:22

@mfikes do you think just disabling the git cache test is sufficient?

mfikes18:06:31

@dnolen I don't know. I haven't been able to follow the backlog yet.

dnolen18:06:20

@mfikes it works, I’ve applied the quickfix to master for now

dnolen18:06:59

@symfrog re: your problem I would no longer follow your lines of speculation. metadata doesn’t matter, what’s in the compiler environment also doesn’t matter

dnolen18:06:46

there was a bug around a exists? now resolved on master - but I honestly don’t see how this applies to your issue

symfrog18:06:29

@dnolen I have added the problematic output for 1.10.238 to the issue as well as the correct output from 1.9.946. It is related to line 7 of out/demo/a.js where the metadata is incorrect when compiled with 1.10.238. I am not sure that this is the ultimate cause of my issues, but I am sure that the metadata is being set incorrectly by cljs.core/resolve due to the defs of demo.foo.b not being populated in cljs.env/*compiler* at the time of the compilation of demo.a.

dnolen18:06:49

@symfrog the metadata doesn’t matter though

dnolen18:06:00

stop speculating 🙂

dnolen18:06:26

metadata can’t possibly have any effect under any compilation mode

dnolen18:06:11

based on the 2764 fix, I no longer have a meaningful reproducer on that ticket

dnolen18:06:48

so to understand what we should be looking for need to step back and understand what resolve actually does

dnolen18:06:32

it simply puts a JS reference to something inside of a function closure

dnolen18:06:53

later when you invoke var that JS reference will hopefully to point to a thing that exists

dnolen18:06:18

all this stuff about metadata and compiler info is completely irrelevant for invoking across module boundaries

dnolen18:06:00

if some JS property foo.bar.baz exists when you deref the var, invoke across the module boundary will work

dnolen18:06:49

as to why it worked before and not now, that will probably lead us to the real problem

dnolen18:06:06

but we need a real minimal reproducer, now that the previous one turned out to be an unrelated thing

dnolen18:06:25

and the hypothesized causes reported on the ticket so far aren’t relevant as far I can see

symfrog19:06:21

@dnolen thanks, it helps that I can focus elsewhere then to find the cause for my actual problem, but since I noticed the metadata issue I thought I would chase that down first. I would say there are potentially 2 issues then, the metadata issue (confirmed) and whatever is causing my actual problem.

dnolen19:06:21

@symfrog the above is the generated code

dnolen19:06:59

the only way for you to have an issue is A) Closure believes that demo.foo.b.bar doesn’t exist

dnolen19:06:30

B) you didn’t load the module with that def

symfrog19:06:38

The module is loaded, and works with 1.9.946. I will continue digging and see if I can find the actual cause now that the metadata issue is confirmed not to be the cause.

dnolen19:06:00

let’s ignore 1.9.946 (it’s quite old now a lot has changed around modules)

dnolen19:06:12

is the module loaded in master

dnolen19:06:35

that’s the first thing to check

symfrog19:06:48

@dnolen master fails to compile om.next (beta 3) with the error Caused by: clojure.lang.ExceptionInfo: clojure.lang.PersistentList cannot be cast to clojure.lang.Named at line 2525 om/next.cljc {:file "om/next.cljc", :line 2525, :column 33, :tag :cljs/analysis-error}

dnolen19:06:48

@symfrog fixed and released 1.0.0-beta4

dnolen19:06:03

@symfrog cljs master also changed so bump that

dnolen19:06:38

@symfrog yeah there’s no way to deal with this problem

dnolen19:06:00

I guess we could do something different here

dnolen19:06:01

one second

dnolen19:06:25

A bit annoying that people are using exists? for general nil check

dnolen19:06:23

@symfrog try master now, exists? now restores some? behavior

dnolen19:06:05

JIRA thing

dnolen19:06:10

please refrain from changing the description for now

dnolen19:06:17

any new information should go in comments

symfrog19:06:28

@dnolen thanks! It compiles now, but I still have the same problem, will keep digging.

dnolen19:06:25

ok but please remember what I’m saying

dnolen19:06:51

there are only two possibilities here

dnolen19:06:15

A) Closure doesn’t know about that def B) module isn’t loaded

dnolen19:06:42

and A) is only possible if ns with that def isn’t part of the build somehow

symfrog19:06:33

ok, will keep it in mind, must be A since the callback of cljs.loader/load is invoked and I can resolve the produced js variables from the console. I have also run the compiler with additional log output to check that the ns is built.

dnolen19:06:02

@symfrog please add another bit of information to the ticket

dnolen19:06:11

build with :advanced + :pseudo-names true, :pretty-print true

dnolen19:06:51

then at least we can see what’s happening to your resolved var

symfrog20:06:47

@dnolen on my actual project I get Uncaught TypeError: Cannot read property '$cljs$core$IFn$_invoke$arity$2$' of null at runtime when invoking ((resolve 'target.module.ns/init) arg1 arg2) in the callback of cljs.loader/load. Everything works fine when I use :simple optimization, only breaks with :advanced. I will do the same for the minimal repro and add the outcome to the ticket.