Fork me on GitHub
#cljs-dev
<
2017-07-30
>
anmonteiro00:07:06

the attached repro had weird bugs (in the repro itself) but importing and using material’s textfield works

Oliver George04:07:04

I think 854 has reintroduced a "relative path" issue on Windows again. I'll look into this on Monday.

anmonteiro04:07:49

@olivergeorge would be good to have it reported and fixed before the actual release then. I added some tests for relative-path so we probably need more

anmonteiro04:07:42

I did some cursory testing on my windows VM and didn’t see any issues

Oliver George05:07:19

I'll do my best to check it out tonight then. +5 hours or so.

Oliver George09:07:06

Confirming that the example from CLJS-2036 was fixed in 1.9.671 but fails again in 1.9.854

Oliver George09:07:20

I'll prepare a bug and investigate but if anyone is interested here's a quick repo (lein mies plus cljs-2036 steps to reproduce).

Oliver George09:07:53

Update project.clj to set the clojurescript version and run scripts\build.bat on windows.

Oliver George10:07:45

Includes a patch which fixes things (for me) and seems sensible.

Oliver George10:07:58

@anmonteiro I think you were right about the code. Not sure about the tests... seems they didn't catch the platform specific nature of File/separator

anmonteiro21:07:20

I’m thinking that if we decide in favor of @thheller ‘s suggestion of always enabling AMD processing, we can greatly simplify process-js-modules

anmonteiro21:07:37

like, we don’t need to know the :module-type anymore, or even have the multimethod

anmonteiro21:07:24

if the Closure Compiler is smart enough to figure all those things out, then I think it also eliminates the problem described in this comment: https://github.com/clojure/clojurescript/blob/master/src/main/clojure/cljs/closure.clj#L2340-L2343

anmonteiro21:07:49

I’m going to play around with it

Oliver George21:07:58

Hi Guys, I have a small window to help with CLJS-2285 but I'm not sure about testing and windows.

Oliver George21:07:25

I think perhaps it's not updated for node/npm

Oliver George21:07:44

95 tests, 32 failures running from Windows

Oliver George21:07:26

Actually, I can work with it. The last error is a relative-name report which I think shows the bug I was reporting on master.

Oliver George21:07:30

@dnolen With that patch there's one less error reported by lein test.

Oliver George21:07:19

perhaps we need File/separator bound to a dynamic variable to allow testing of windows behaviour without Windows running.

Oliver George23:07:18

Yeah, this file path manipulation stuff is the devil. @anmonteiro tests highlight another issue.

anmonteiro23:07:57

I saw some weird things in that function which is why I added the tests

Oliver George23:07:30

Thanks for that. It's nice that we can see them at test time.

Oliver George23:07:28

These functions are impure and surprising due to the platform specific calls to java functions. Should we create some interop helpers which smooths out the inconsistencies? It'd be nice to deal in data (e.g. path as vector of strings).

anmonteiro23:07:32

sure. we had too many regressions because of that

anmonteiro23:07:54

^ this was answering the previous statement

anmonteiro23:07:05

not sure about helpers, better ask David

Oliver George23:07:12

It might be a whole new can of worms. Only a gut feeling.

Oliver George23:07:27

Just found to-path and path-seq. Will see if I that suits.