@mfikes tried to follow your instructions from https://dev.clojure.org/jira/browse/CLJS-2757?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=49484#comment-49484

but I keep getting

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 ()

when trying to use self built closure snapshot

@kommen Are you running

mvn -DskipTests -pl externs/pom.xml,pom-main.xml,pom-main-unshaded.xml

yes

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] ------------------------------------------------------------------------

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

clj -Sdescribe yields :version "1.9.0.381" for me

OK… yeah, perhaps it is some other side artifact

(`closure-compiler-parent`)

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

doesn’t look like it

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

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.

@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?

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

@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)

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

@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.

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?

looks like it

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

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

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

OK. Still an open question 🙂

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

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

- 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

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.

Cool!

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

@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)

Hrm that probably works in Clojure?

Quoted record literals

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

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

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

or around it at least

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

To my knowledge, no.

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

erm I think I'm getting mixed up.

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

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 (?)

well it should work in both cases no?

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

@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

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})))))

well, map->RecordLiteral..

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

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

I’m talkin about that ^

haha ok. thanks for your patience.

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

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

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

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?

(I'll prioritise the record literals tho)

it should probably be ok, but we’ll see

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.

these are my tests FWIW https://github.com/frenchy64/clojurescript/commit/717ea647cd0508b312ff179d4e929b49fcdb3574

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

ready to screen