Fork me on GitHub
#cljs-dev
<
2018-07-04
>
kommen13:07:35

but I keep getting

kommen13:07:58

Error building classpath. Failed to read artifact descriptor for com.google.javascript:closure-compiler-unshaded:jar:1.0-SNAPSHOT
org.eclipse.aether.resolution.ArtifactDescriptorException: Failed to read artifact descriptor for com.google.javascript:closure-compiler-unshaded:jar:1.0-SNAPSHOT
Caused by: org.apache.maven.model.resolution.UnresolvableModelException: Could not find artifact com.google.javascript:closure-compiler-parent:pom:1.0-SNAPSHOT in central ()
errors

kommen13:07:22

when trying to use self built closure snapshot

mfikes13:07:35

@kommen Are you running

mvn -DskipTests -pl externs/pom.xml,pom-main.xml,pom-main-unshaded.xml
in your closure-compiler clone?

kommen13:07:11

that ends with

[INFO] --- maven-install-plugin:2.4:install (default-install) @ closure-compiler-unshaded ---
[INFO] Installing /Users/kommen/work/closure-compiler/target/closure-compiler-unshaded-1.0-SNAPSHOT.jar to /Users/kommen/.m2/repository/com/google/javascript/closure-compiler-unshaded/1.0-SNAPSHOT/closure-compiler-unshaded-1.0-SNAPSHOT.jar
[INFO] Installing /Users/kommen/work/closure-compiler/pom-main-unshaded.xml to /Users/kommen/.m2/repository/com/google/javascript/closure-compiler-unshaded/1.0-SNAPSHOT/closure-compiler-unshaded-1.0-SNAPSHOT.pom
[INFO] Installing /Users/kommen/work/closure-compiler/target/closure-compiler-unshaded-1.0-SNAPSHOT-sources.jar to /Users/kommen/.m2/repository/com/google/javascript/closure-compiler-unshaded/1.0-SNAPSHOT/closure-compiler-unshaded-1.0-SNAPSHOT-sources.jar
[INFO] ------------------------------------------------------------------------
[INFO] Reactor Summary:
[INFO] 
[INFO] Closure Compiler Externs 1.0-SNAPSHOT .............. SUCCESS [  1.261 s]
[INFO] Closure Compiler Main .............................. SUCCESS [  0.735 s]
[INFO] Closure Compiler Unshaded 1.0-SNAPSHOT ............. SUCCESS [ 41.465 s]
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 44.342 s
[INFO] Finished at: 2018-07-04T15:35:05+02:00
[INFO] ------------------------------------------------------------------------

kommen13:07:06

ok, looks like it works now after running mvn -DskipTests once in closure

mfikes13:07:10

clj -Sdescribe yields :version "1.9.0.381" for me

mfikes13:07:26

OK… yeah, perhaps it is some other side artifact

mfikes13:07:43

(`closure-compiler-parent`)

mfikes13:07:25

Curious to see if the JavaScript related to slate ends up being loaded in a usable way 🙂

kommen13:07:53

doesn’t look like it

kommen13:07:26

getting

Uncaught TypeError: (0 , module$Users$kommen$work$cljs_issues$slatenpm$node_modules$slate$node_modules$debug$src$browser.default.load) is not a function
    at browser.js:15
Uncaught TypeError: module$Users$kommen$work$cljs_issues$slatenpm$node_modules$slate$node_modules$debug$src$debug.default.enabled is not a function
    at Object.createDebug$$module$Users$kommen$work$cljs_issues$slatenpm$node_modules$slate$node_modules$debug$src$debug [as default] (debug.js:16)
    at slate.es.js:374

mfikes13:07:41

Yeah. If, in a REPL as per the ticket you do something like (require '[slate :refer [Changes]]) things ostensibly look good but some disconnect is still occurring.

kommen14:07:04

@mfikes but I guess, while slate would still not load, the problem as per the ticket is resolved, and this is now some follow up problem?

mfikes14:07:10

Strictly speaking yes. When ClojureScript ultimately ships with some new build of Closure Compiler, the ticket as written would no longer be reproducible.

kommen14:07:33

@mfikes so for a working slate example one would also need slate-react as per https://docs.slatejs.org/walkthroughs/installing-slate. slate-react however also throws an exception when required in the repl

$ clj -m cljs.main -co '{:npm-deps {"react" "15.5.4" "react-dom" "15.5.4" "slate" "0.33.6" "slate-react" "0.12.6" "immutable" "3.8.2"} :install-deps true :repl-verbose true}' -d out -r
cljs.user=> (require 'slate-react)
path.js:26
function assertPath(path) {
^

TypeError: Path must be a string. Received false
    at assertPath (path.js:26:1)
    at Object.resolve (path.js:1156:12)
    at Deps.anonymous ([eval]:204:40)
    at emitNone (events.js:104:1)
    at Deps.emit (events.js:156:31)
    at endReadableNT (/Users/kommen/work/clojurescript/node_modules/readable-stream/lib/_stream_readable.js:1010:12)
    at _combinedTickCallback (internal/process/next_tick.js:129:3)
    at process._tickCallback (internal/process/next_tick.js:151:3)

kommen14:07:54

not sure though if this is maybe just a follow up problem from the one above

mfikes14:07:26

@kommen With respect to the “debug” issue (your “Uncaught TypeError”), this appears to be related to the fact that slate has its own node_modules inside of it, which has a debug, and somewhere along the lines this is mishandled and confused with the top level debug node module.

mfikes14:07:40

Perhaps this is us. We evidently only index the top level node_modules https://github.com/clojure/clojurescript/commit/921b2630804b387f342e54a711e220a1cf8ff0c6#diff-41c825eb516618c6b81bfcb40f84a54dR329 Perhaps this is a situation we just don’t properly deal with yet?

kommen15:07:15

looks like it

juhoteperi15:07:42

The indexing should only affect which node modules are accessible in Cljs :require

juhoteperi15:07:20

Or hmm. It probably also affects which JS files are given to Closure...

mfikes15:07:24

Ahh. Thanks @juhoteperi. This would imply the problem is on the Closure side.

mfikes15:07:03

OK. Still an open question 🙂

juhoteperi15:07:10

index-node-modules-dir which indexes the top level node_modules is not used

juhoteperi15:07:23

Ahhhhh noooo. the link was pointing to a very old version.

juhoteperi15:07:35

- index-node-modules-dir builds list of top level node_modules. - Intersection of all requires in Cljs code and top level node_module provides is passed into index-node-modules - index-node-modules uses proper Node lib to resolve require calls inside JS files, this step fill find proper files from sub node_module directories

juhoteperi15:07:25

So if there are several instances of a lib inside node_modules tree, files from both should be passed into Closure. It should be possible to debug this by calling (cljs.closure/index-node-modules ["react-slate"]) and looking at the list.

mfikes16:07:45

I’ve started a draft of the news for the next release, for now covering private var use and outward function type hint propagation https://github.com/clojure/clojurescript-site/pull/244

mfikes16:07:08

Thanks @U050TNB9F; added some JavaScript examples to illustrate

ambrosebs17:07:54

@dnolen in CLJS-2801, do we need a record? case in emit-constant? ie. can you embed a record? under a quote? (I actually don't know)

dnolen17:07:28

Hrm that probably works in Clojure?

dnolen17:07:54

Quoted record literals

ambrosebs17:07:05

I actually don't know what a record literal is..

ambrosebs17:07:12

ok a quick google helped. I'll try and write a test case.

ambrosebs17:07:48

I can't find existing tests for record literals at least. I'll add it to cljs.core-test/test-reader-literals.

ambrosebs17:07:42

or around it at least

ambrosebs17:07:29

oh, do record literals even work in CLJS? I see https://dev.clojure.org/jira/browse/CLJS-1328 is open

mfikes17:07:48

To my knowledge, no.

ambrosebs17:07:24

ok. so it's unlikely I've broken any code by not supporting it under quote 😛

ambrosebs17:07:48

erm I think I'm getting mixed up.

ambrosebs17:07:39

what's the purpose of the record? case in analyze-form? is it relevant under self-hosting?

ambrosebs18:07:10

I think I almost understand @dnolen's concern that this "works in Clojure". where might I write a unit test for that? cljs.core-test seems like a bad place because it's tested under self hosting (?)

dnolen18:07:51

well it should work in both cases no?

mfikes18:07:08

Perhaps this is relevant https://github.com/clojure/clojurescript/commit/f5af28f814896d3a538cba717331b651a5ef9239 (“data readers that return records” being the salient bit)

mfikes18:07:52

@ambrosebs If you absolutely must have a test that is skipped in self-host, there are a few test namespaces that are skipped. So, something could be done at that granularity. But, in general, self-hosted isn’t that different from regular JVM ClojureScript; I would expect all of this to be able to be made to work if it doesn’t already

ambrosebs18:07:52

I'm feeling over my head here, but I'll try. Is this a good test to put in cljs.core-test?

(defrecord RecordLiteral [a b])

(deftest test-record-literals
  (testing "Testing record literals"
    (is (= #cljs.core-test.RecordLiteral {:a 1 :b 2}
           (->RecordLiteral {:a 1 :b 2})))))

ambrosebs18:07:38

well, map->RecordLiteral..

ambrosebs18:07:43

or I could add this to data-readers-test.records

(assert (= [(Foo. 1 2)] '[#data_readers_test.records.Foo{:a 1 :b 2}]))

dnolen18:07:48

I’m talkin about that ^

ambrosebs18:07:33

haha ok. thanks for your patience.

mfikes18:07:32

Perhaps that path is only tested under JVM ClojureScripot (because it is driven by cljs.build-api-tests) If you are worried about self-host support, perhaps it simply isn’t tested yet

ambrosebs18:07:11

I'm worried, but I think I'll test JVM CLJS first. getting self-hosting/recursion vertigo xD

mfikes18:07:42

Yeah, self-hosted is a minor side thing; not critical

ambrosebs18:07:34

Switching gears, I'm working on desugaring dotted symbols into explicit fields in the analyzer (`v.f.g` -> (.-g (.-f v))). It's mostly working, but I am completely ignorant of source mapping. Could I be breaking something obvious there?

ambrosebs18:07:15

(I'll prioritise the record literals tho)

dnolen19:07:34

it should probably be ok, but we’ll see

ambrosebs20:07:10

I can't seem to break record literals, but I have a few good unit tests. I realized we can remove :record-value and replace with :const (like we just did with :list), this is what tools.analyzer does too. I'll put all that together in a patch. Curious if anyone else can find a failing test for quoted record literals tho.

ambrosebs20:07:35

@dnolen here's the next patch, removes :record-value and add record literal unit tests https://dev.clojure.org/jira/browse/CLJS-2803

ambrosebs20:07:45

ready to screen