@bronsa any chance we could get a release just with that fix? I’m now seeing this whenever I start a ClojureScript REPL

@mfikes re default, are you thinking that we should default to the browser case? I’m fine that

My actual motivation was for building iOS apps, where plain output is needed, not targeting or assuming any particular environment. (This could also be said of the code compiled for Planck). Since those use cases are rare, perhaps this is an argument for the ability to indicate that you don't really want a target, but with that ability not being the default behavior.

so -t default doesn’t work?

Oh, I was unaware of that. Let me try.

clojure -m cljs.main -t default -o out/main.js -c foo.core

hrm I guess we don’t have what you want though? There’s never been a “default” target, since you at least have to be able to load files

or are you asking that just need something for Ambly?

Yes, it is fair that this covers the needs for something like Ambly, which makes it less important.

And for Ambly

clojure -e "(require 'cljs.build.api)" -e '(cljs.build.api/build "src" {:output-to "out/main.js"})'

oh ..

I guess what Ambly really needs is for the target bit to be open

so you can supply your own thing

So, I asked myself: "How can I use -c to produce the same thing that you get using that clojure line above?"

@mfikes hrm what’s in out/main.js?

Let me paste both for comparison... (they are both smallish)

clojure -e "(require 'cljs.build.api)" -e '(cljs.build.api/build "src" {:output-to "out/main.js"})' generates

goog.addDependency("base.js", ['goog'], []);
goog.addDependency("../cljs/core.js", ['cljs.core'], ['goog.string', 'goog.Uri', 'goog.object', 'goog.math.Integer', 'goog.string.StringBuffer', 'goog.array', 'goog.math.Long']);
goog.addDependency("../process/env.js", ['process.env'], ['cljs.core']);
goog.addDependency("../foo/core.js", ['foo.core'], ['cljs.core']);

clojure -m cljs.main -o out/main.js -c foo.core generates

if(typeof Math.imul == "undefined" || (Math.imul(0xffffffff,5) == 0)) {
    Math.imul = function (a, b) {
        var ah  = (a >>> 16) & 0xffff;
        var al = a & 0xffff;
        var bh  = (b >>> 16) & 0xffff;
        var bl = b & 0xffff;
        // the shift by 0 fixes the sign on the high part
        // the final |0 converts the unsigned value into a signed value
        return ((al * bl) + (((ah * bl + al * bh) << 16) >>> 0)|0);
    }
}

var CLJS_OUTPUT_DIR = "out";
load((new java.io.File(new java.io.File("out","goog"), "base.js")).getPath());
load((new java.io.File(new java.io.File("out","goog"), "deps.js")).getPath());
load((new java.io.File(new java.io.File(new java.io.File("out","goog"),"bootstrap"),"nashorn.js")).getPath());
load((new java.io.File("out","cljs_deps.js")).getPath());
goog.global.CLOSURE_UNCOMPILED_DEFINES = {"cljs.core._STAR_target_STAR_":"nashorn"};
goog.require("process.env");
goog.require("foo.core");

Adding -t default causes the above to change to:

var CLOSURE_UNCOMPILED_DEFINES = {"cljs.core._STAR_target_STAR_":"default"};
var CLOSURE_NO_DEPS = true;
if(typeof goog == "undefined") document.write('<script src="out/goog/base.js"></script>');
document.write('<script src="out/goog/deps.js"></script>');
document.write('<script src="out/cljs_deps.js"></script>');
document.write('<script>if (typeof goog == "undefined") console.warn("ClojureScript could not load :main, did you forget to specify :asset-path?");</script>');
document.write('<script>goog.require("process.env");</script>');
document.write('<script>goog.require("foo.core");</script>');

FWIW for Planck, I don't specify :target in the compiler opts map passed to cljs.build.api/build and then Planck loads the resulting main.js that consists of nothing but clean goog.addDependency calls. So perhaps this capability is only really useful for environments that take on the responsibility for bootstrapping the Closure environment themselves. This is a nice capability from a "flexibility" standpoint, but clearly not what most end-users will be taking advantage of.

To be honest, directly calling cljs.build.api/build is fine for this, and https://dev.clojure.org/jira/browse/CLJS-2532 could be declined until we hear from other users out there that there is truly a need for a clean "targetless" compile capability via cljs.main and -c.

Interestingly, if you look at the very first out/main.js generated on the Quick Start page, it is of this "targetless" variety, so if we tried to replace all of the commands on that page with updated ones using cljs.main and variations involving -c and -r we would perhaps run into the same problem there. (Or we could just go with revisions that immediately start using -t browser or somesuch.)

Ahh, I see what I'm doing... by specifying a namespace after -c this is the same as adding a :main compiler option, which causes the different out/main.js to be written.

I'll add a note to https://dev.clojure.org/jira/browse/CLJS-2532

ah right, so yeah maybe special casing :target :none is the way to go

@dnolen cut 1.2.2

Perhaps yeah. What is more important than figuring out anything for Planck or Ambly, is what the first build or two in Quick Start looks like and whether it uses cljs.build.api/build without :main or cljs.main with -c and a :target :none or a -c without a namespace or something else...

@bronsa thanks!

@mfikes well the tutorial is focused on doing stuff in browser / node - so I think we can just stick with that

I don’t see a need to cover that there - unless I missed something

Yeah, the very first build could end up looking like clojure -m cljs.main -o "out/main.js" -c hello-world.core, essentially jumping initially into the Less Boilerplate section.

@mfikes just pushed :none case, we just skip the main bits if you set -t none

ok

Nice. I confirmed that clojure -m cljs.main -t none -o out/main.js -c foo.core essentially provides what I asked for in https://dev.clojure.org/jira/browse/CLJS-2532

@dnolen I was thinking about digging into :analyze-path https://dev.clojure.org/jira/browse/CLJS-2535 but if you already have an idea on that one and were thinking of working on it, let me know. (Don't want to duplicate effort.)

@mfikes not working on that, go for it!

@juhoteperi hrm I get test failure now for a npm modules test when running ./script/test

seems odd since haven’t been working on anything related to that, the error I see over here is that using lodash nth got renamed to ii somehow, but the original fn name wasn’t changed

@dnolen Did you upgrade your dev box to Java 9? If so, perhaps you are now seeing https://dev.clojure.org/jira/browse/CLJS-2400

FWIW, I avoid this locally by having done brew cask install java8 to also have Java 8 on my box, and by additionally

export JAVA_HOME=`/usr/libexec/java_home -v 1.8`

@mfikes oh hrm so is a JDK 9 bug in Closure or something?

(You can easily switch to Java 9 by changing that to -v 9.0

Yeah, I wonder if we can form a minimal repro or if you need everything at play. Someone else encountered this in #clojurescript the other day https://clojurians.slack.com/archives/C03S1L9DN/p1518811207000651

maybe https://dev.clojure.org/jira/browse/CLJS-2529

yeah that’s probably it

yeah can verify that issue on JDK 9

Wow, looks like a bit of a mess.

(Apache Ignite appears to work around it by delving into JDK internals.)

@bronsa still more :refer issues with 1.2.2, you :refer :all your impl stuff in clojure.tools.reader but you are missing :excludes

that's intentional

it will only refer tagged-literal/`reader-conditional` & co if loading <=clojure-1-7-alpha5

otherwise they won't be defined

those forms are conditionally evaluated

which now that i think about it means the exclude stuff wasn't really necessary

so curious about how you're seeing this

oh hrm, now it makes sense, I’m using the AOTed artifacts, but they will have been compiled by some version of Clojure already

ah yes

that will be an issue

and I guess you can’t :exclude something that doesn’t exist?

I can but then it wouldn't work as monkey patch for <=1.7-alpha5

but I don’t really understand why you need to backport those defs into older version of Clojure?

I had no idea we were distributing aot

@dnolen because I have to conditionally use the c.c or the c.t.r.utils versions depending on which is available

ClojureScript has been AOTed for some time now - and I had Alex modify the tools.reader and data.json deps so I could grab those as well

I didn’t include them before directly because wanted to wait an ssee

but I’d never seen any problems before until now

doesn't cljs work only with > 1.7 anyway?

why are we compiling it with an older version

yes, I’m pretty sure rely on 1.8 features

oh so I guess that’s the problem?

I suppose

I should compile it with 1.9?

yeah

then those monkey patch functions won't ever get compiled

@bronsa but wait I guess I don’t see why this should be a problem since I’ve been compiling with 1.8?

yeah that shouldnt happen

i dont see how that's possible

hrm

can you double check something's not causing an older version of clojure to be used

i'll do some experimentation if you're sure you're using 1.8

@bronsa well I see that excluding names that don’t exist is not really a problem

so I don’t see how that affects the monkey-patching bit you’re talking about

@mfikes hrm the Java9Bridge thing there is pretty simple if yuck-y - I’m not sure we’re going to come up with better ideas

on <1.7, c.c doesn't have those vars defined, so what we need is effectively (require [.. util :refer [tagged-literal]]) so that in code when we use tagged-literal that's the var we'll be refering too

tools.reader uses tagged-literal in its code

altho I wonder if i'm not using the wrong versionn for one of tagged-literal or reader-conditional

let me check

k, also maybe you can just do this choice a bit more dynamically, always exclude, resolve then choose or something like that

doing :refer :all w/o exclude is a way to refer to tagged-literal when it doesn't exist in c.c, but not when it does

fwiw, feature detection seems more reliable for stuff like this

yes if this is a problem I could exclude by default and conditionally require

(instead of version detection)

@dnolen no good way to do this otherwise since we need to define types not just vars

ah k, surface is bigger than I thought poking around

yeah, I might drop support for clojure < 1.7.0 at some point but until then there's no other way to do this

other alternatives don't play well with reloading bc of classloaders

k going to leave this alone for now then - the load time benefit from AOTed tools.reader and data.json isn’t very big - that’s why I was bothering

or would prevent AOT at all

@mfikes https://github.com/elcergey/ignite/blob/711085963d5c13aa7aeef6c8d8fecf3f063352cf/modules/core/src/main/java/org/apache/ignite/util/Java9Bridge.java, could probably just pull the few relevants bits into our all-classpath-urls logic

Yeah, @dnolen we could port the relevant portions to Clojure. I could take a stab at it if you are not already.

there is always (System/getProperty "java.class.path") which is most likely good enough

@mfikes go for it!

To @thheller’s point, it looks like we currently get 26 items out of all-classpath-urls, but only 15 out of (System/getProperty "java.class.path") so perhaps a port is the right approach

yeah but the extra jars are jvm internals. you won't find .cljs in there so you don't need them

one thing that’s a little frustrating is that the CLJS uberjar is 3 seconds faster to do anything, and this appears to be due to everything in the uberjar being compiled and direct linked

@thheller I think what @mfikes is suggesting is probably the safest in terms of emulating precisely how it worked before

my experience is that the tiniest differences with stuff like this inevitable is a problem for someone

the only difference is that the classloader path will pick up custom classloaders

and people use those and put stuff where those things will find them 🙂

it might affect boot yes

I'll put together a straight port... it looks like very little code, and we can decide whether to use it.

should probably try/catch and fall back to the env property. the Java code doesn't look all that trustworthy and looks like it expects that you pass in an URLClassLoader ... which custom classloaders might not be.

for the time being the important is to just emulate the old behavior such that it works under Java 9 - we can enhance etc. some other time

I would just like Java 9 to work like Java 8 - nothing more

(talking about priorities for the next release)

@dnolen Bah! Oracle have secured access to that stuff. Specifically if you try to

(-> (Class/forName "jdk.internal.loader.BuiltinClassLoader") (.getDeclaredField "ucp") (.setAccessible true))

InaccessibleObjectException Unable to make field private final jdk.internal.loader.URLClassPath jdk.internal.loader.BuiltinClassLoader.ucp accessible: module java.base does not "opens jdk.internal.loader" to unnamed module @4c03a37  java.lang.reflect.AccessibleObject.checkCanSetAccessible (AccessibleObject.java:337)

@mfikes, hrm I guess in Java 9 crawling the classpath like this is just not really supported, I’m fine with going with @thheller’s suggestion

anybody that was expecting more will have their own issues now anyway

but I think this will probably slow Java 9 adoption

so we should support the old way - only look at classpath if URLClassloader isn’t present

Yeah, I found a couple other codebases out there messing with that "ucp" field, but interestingly I don't see commits that add --add-opens java.base/jdk.internal.loader=ALL-UNNAMED in those projects.

I just bumped ClojureScript version to 1.10, I’m going to do pREPL for the next release

Yeah, I can make it try to work based on (System/getProperty "java.class.path") if Java 9.

FWIW thats what I have in shadow-cljs. if the classloader returns items I use that, it not I use the fallback. https://github.com/thheller/shadow-cljs/blob/master/src/main/shadow/build/classpath.clj#L505-L513

Unit tests pass with that change 🙂

You can also check if it's supported:

(select-keys (bean (ManagementFactory/getRuntimeMXBean))
               [:bootClassPathSupported
                :bootClassPath
                :classPath])

@dnolen I know it is not a priority, but I was wondering if you could check CLJS-2492

@richiardiandrea I don’t understand the patch (but also maybe because the old thing had problems)

if there’s no name you use still used the munged name?

that you use (symbol mn) even if you don’t have a name seems strange to me

(it was strange before too)

we would throw away stuff when name is nil it seems

Uhm, that is something I haven't considered actually (the nil case)

well might as well fix that up if you’re there

Yes definitely, I will handle that case, I see that if there is no name a cljs-timestamp is used

I also want to mention that name is munged and used as source map key somewhere

Will ping you again when the new version is ready, thanks for checking

I have to say that having the test alias in deps.edn is a great win, and on top of that I added the rebel global alias for Bruce super powers 😃 Good job overall.

Draft release PR updated to reflect 1.10 https://github.com/clojure/clojurescript-site/pull/156

@dnolen as promised, ping for CLJS-2492, also added one more test

@mfikes patch fixed the Java 9 issue for me

@richiardiandrea applied

thanks!

Cool. I suspect ClojureScript 1.10 will be good to go with Java 9, with the only corner case being Nashorn not being able to run self-host, which is arguably not that big of a deal. https://dev.clojure.org/jira/browse/CLJS-2530