Fork me on GitHub
#cljs-dev
<
2018-03-22
>
dnolen00:03:20

@r0man yeah your change still doesn’t make any sense to me

john04:03:51

Some example websocket stuff to kick the tires on: https://github.com/johnmn3/repl-ws

john04:03:18

It's midway through implementation - probably going to move all the state in the http://cljs.repl.ws namespace into the WebSocketEnv object

dnolen09:03:12

@r0man the reason your patch doesn’t make sense to me is that we don’t use JSON for returning results to the server - it’s currently just for sending JS evals and for communication between iframes - if you can show where you think JSON is being introduced on the way back that would be helpful

r0man09:03:13

@dnolen I take a look later, need to finish something else first

mfikes12:03:03

There are some places in cljs.main where you can pass multiple things, separating them by colons. Another approach that could be nice is simply allowing the argument to be supplied more than once. One (arguably less important) example might be putting -co in an alias (via :main-opts), and then when you go to use that alias, if you pass -co that would be a second instance of -co on the command line that could be merged into the results of the first.

dnolen12:03:07

right for that we just need to consistently merge

mfikes12:03:06

Ahh @dnolen I think this is the way it actually works now.

mfikes12:03:31

Not intentionally, but it happens for -co

dnolen12:03:48

good, that was intentional

dnolen12:03:55

thought you found a case where it’s not true

mfikes12:03:55

Perhaps watch-opt could update-in, conj path

mfikes12:03:38

(A way to watch multiple paths, related to https://dev.clojure.org/jira/browse/CLJS-2681)

mfikes13:03:27

I also wonder a little about the use of colon as the delimiter for -co: Since you can pass paths to EDN files, it feels a little like a path delimiter as well, and if that's true, perhaps File/pathSeparator is natural (less problematic?) in the Windows case.

dnolen13:03:43

is that what clj does?

mfikes13:03:37

It appears that the only places clj accepts colons to delimit things is for aliases. (So paths don't come into play there.) The other place that might be -Scp in the future. Presumably on Windows, clj will accept a Windows (semicolon-delimited) classpath in that case. TL;DR perhaps clj has no precedent / guidance

Alex Miller (Clojure team)14:03:03

clj does not use colons as a delimiter - the alias opts take a concatenated set of keywords (which happen to be prefixed with colons)

Alex Miller (Clojure team)14:03:42

-Scp is expected to take a platform-appropriate path separator

dnolen13:03:57

Ah k, cross platform thing is fine by me

dnolen13:03:05

This is a minor sugary thing

dnolen13:03:46

@mfikes yes appropriate changes to make repeatable inits merge is fine by me

mfikes13:03:42

OK, I'll log a couple of tickets (one for File/pathSeparator and one for inits merging

juhoteperi17:03:46

@dnolen Any idea if goog.require("some.cljs.ns"); should work from some CJS/ES6 modules? Someone mentioned on Clojureverse this used to work, but after Closure update this breaks due to those goog.require calls we now generate. Fix might be simple, but not sure if this should work.

juhoteperi17:03:52

Looks like Closure will detect those goog.require calls during module processing and adds the required modules to the module dependency information.

thheller17:03:34

@juhoteperi goog.require is no longer allowed in ES6. but you can do import cljs from "goog:cljs.core"; cljs.assoc(null, "foo", 1);

thheller17:03:50

doesn't work with require though

juhoteperi17:03:48

Doesn't work with Cljs currently because our code tries to resolve import calls into node modules

thheller17:03:17

oh hmm that I don't know

juhoteperi17:03:19

We should filter goog: modules there, but I guess there would be other problems after that

juhoteperi17:03:32

Because we call Closure for module-processing with only the foreign-libs/node_module files, Closure can't resolve the requires to Cljs.core or other Cljs code (guess anyway)

juhoteperi18:03:13

Oh, nice, I was wrong. With simple change to filter code in our Node modules resolve logic, it seems to work.

juhoteperi18:03:43

Works for :optimization :none but :simple etc. fails 😕

juhoteperi19:03:27

And fix to remove unncessary goog base requires from module processed files: https://dev.clojure.org/jira/browse/CLJS-2691

juhoteperi19:03:23

After these, I think this works the same as in 1.9. Optimization still fails if module-processed files use Cljs namespaces that aren't required by any other Cljs namespace, but that happened before also.

mfikes20:03:10

When Clojure's range was converted from a lazy seq to a LongRange in CLJ-1515, LongRange was made to implement IChunkedSeq. On the surface it would seem that doing the same to ClojureScript's Range could result in some perf benefits (like map over a the result of a range call). I wonder if this has been explored and rejected as not being true.

mfikes20:03:23

I bet it would be faster. Compare

(time (apply + (filter even? (map inc (vec (range 1e6))))))
with the same but without converting to a vector to get the chunks
(time (apply + (filter even? (map inc (range 1e6)))))

mfikes20:03:44

This is enough to convince me to write a JIRA to try a potential patch.

Alex Miller (Clojure team)20:03:03

just a note that there were many dragons behind this change

mfikes20:03:25

Yeah, I see the change is fairly complex looking.

Alex Miller (Clojure team)20:03:59

you can reason from the large number of patch attempts :)

mfikes20:03:20

Yes. If I look into it, I'd definitely base work off Clojure master.

Alex Miller (Clojure team)20:03:47

balancing the perf concerns of reducible, sequence, and chunked sequence traversal (all of which are possible and may even interleave) while also avoiding potential overflow/underflow cases was a surprisingly narrow path

mfikes20:03:58

But that was the pattern with reducible sequence generators. You do the hard work so others can port it. 🙂

mfikes20:03:42

It sounds like you have harsh memories of this one, though.

Alex Miller (Clojure team)20:03:27

I did not expect to spend several months exploring ways to count

mfikes20:03:53

Just think of the electricity you saved though. Good for the planet.