Looks like Closure February release is out

Oh I was just yesterday thinking about when that would be due

I can try and upgrade it later today if no one gets to it first

cool

definitely won’t take a look until tomorrow

@dnolen turned out to be a very clean patch http://dev.clojure.org/jira/browse/CLJS-1952

of course, I spoke too early

cannot find modules now 🙂

OK got it

we need to (.setModuleResolutionMode compiler-options ModuleLoader$ResolutionMode/NODE)

probably worth discussing if we want to have a new compiler option for the module resolution modes. there exist LEGACY, NODE and BROWSER. more context: https://github.com/google/closure-compiler/wiki/Releases#february-18-2017-v20170218

I think we’ll mostly want NODE, not sure what BROWSER is for. I think only Microsoft Edge supports it

patch attached

I recently rewrote the whole node.js support in shadow-build as I was getting rather frustrated with the default node.js bootstrapping mechanism and its differences to optimized output

tried writing down my motivations and implementation

https://github.com/thheller/shadow-build/blob/master/doc/node.md

I'd be interested to hear some other opinions on the subject

currently setting up a proper demo so you can actually play with it

but at this point I have no idea if the things I "fixed" actually bother other people as well or just me

:none vs optmized and (js/require "./rel/path") not working as expected

@anmonteiro can you explain this patch a bit?

http://dev.clojure.org/jira/secure/attachment/16435/CLJS-1941.patch

@dnolen sure. We’re using compile-root which ends up calling that function. Given it’s a #?:clj only function, it shouldn’t be finding .cljc files if a .cljs file with the same name exists (since those are not really CLJS files but macros to be expanded in Clojure)

but how was that happening?

we saw the problem with om.dom, for example, where the compiler would end up compiling dom.cljc (with the wrong requires) instead of dom.cljs

that’s an or with .cljs first

yeah, but it ends up including both

but how?

there’s no explanation about that

oh

because it just calls file-seq

exactly

got it

awesome

@dnolen anyway, we’re using a version of CLJS with that patch now, in production

and it has been working great for the past 2 weeks (which means like 200 builds or more)

cool I understand the problem - just was trying to figure out the patch

awesome

might be related to that :parallel-build issue reported by @tonsky

going to try that next

@anmonteiro did you see the problem under :parallel-build or it was just an issue in general

interesting. I wouldn’t think they are connected

@dnolen nope, this is unrelated to :parallel-build

always happened

ah k

now would be a good time to try master

I’m surprised no one has been bitten by this one before https://github.com/clojure/clojurescript/commit/04541fbf8ec573d26bac4e4e61e69cb4c64a3dd7

I’ve seen different entries for the same NS after ordering deps, but somehow I thought that was normal

I can see how that would impact parallel build now 🙂

@dnolen i probably have

we've had some intermittent issues with parallel builds in the past and decided to turn them off, but never had any time to find a repro

@dnolen the fix for CLJS-1948 might fix CLJS-1921, but there still might be some weirdness going on, see ticket http://dev.clojure.org/jira/browse/CLJS-1921

i'll give parallel builds another try after that fix

@moxaj someone needs to confirm that master is still an issue for 1921

@anmonteiro yeah not normal 🙂

@dnolen can we expect a CLJS release today, or the parallel build issue doesn’t justify it?

let me check CLJS-1921 first

definitely looks like the same symptom of cljs-1948

let me try and repro cljs-1921 too, against current master

can’t repro CLJS-1921 with master here

I can't build master because i'm on windows 😞 But I suspect it would fix it. However, wouldn't it make more sense to eliminate the duplicates right as they are inserted? See the java.io.File - java.net.URL mismatch

(unless that is intentional)

too risky to change it there I think so I did the simplest thing I could think of

somebody else will need to spend some time to suss out the deeper issue and determine if it isn’t going to break the world

nitpick, but i'd argue that was the easiest 🙂

not if I can’t test every lib

de-duping inputs seems more clear

I mean, I understand your reasoning, i'm just riding on simple vs easy

right but I don’t think you’re quite clear what’s going wrong

it doesn’t have anything to do with file vs. url

but the fact that we use the compiler input as a key

so everything about that key has to be same

auditing for that is a massive pain when I just want to fix the bug

I can’t repro cljs-1921

but I also didn’t manage to repro it with and older cljs.jar, so I don’t know what that says

@anmonteiro there are some very specific conditions required to repro, I've tried my best to describe those in the ticket

I know how issues like these are not deterministic

it relies on set ordering etc.

I’ve tracked down 2 or 3 related ones

right but the set ordering issue goes back to what I said

which is my belief that using the the entire source input as the key was a bad idea to begin with

instead key on :provides

my rubber duck tells me it should be solved by the fix to cljs-1948

the same namespace appearing twice in a build can’t possibly be right ever

trying to fix this at :url and :file isn’t a good idea

nearly everything is sourced from the classpath anyway

oh, now I see what you mean - it is an issue, but shouldn't be solved directly

I agree that we shouldn’t mess with :url and :file

@moxaj yes 🙂

I’ll leave 1921 open for now and cut a release then