Fork me on GitHub
#clojurescript
<
2021-12-28
>
Franklin14:12:35

Error messages in ClojureScript, when compiled with the prod build don't look so good

Franklin14:12:41

is there  a way to make them clear when you want to log to something like `sentry`?

p-himik14:12:59

Sorry, I meant sourcemaps. 😅

Franklin14:12:56

yes, source maps solve this problem. although, it's possible that errors might be thrown even before the browser get gets the chance/time to load source maps, right?

p-himik14:12:07

I think that only a completely invalid JS would result in this. And then nothing could really help here - you'd have to debug the compiler.

Franklin14:12:04

aaah... cool, thanks again 🙂

👍 1
Eric Scott23:12:32

I'm wondering if I have the wrong impression about whether/how tagged literals can be included in clojurescript source. Here's a unit test (note the reader conditionals):

(testing "lstr tag"
    (let [x #?(:clj #lstr "dog@en" :cljs (read-string "#lstr \"dog@en\""))
          ]
      (is (= ont_app.vocabulary.lstr.LangStr (type x) ))))
Straight clojure works fine, but Without the special treatment under cljs, I get this error:
Compiling ["resources/test/compiled.js"] failed.
clojure.lang.ExceptionInfo: failed compiling file:/home/eric/Projects/Ont-app/vocabulary/test/ont_app/vocabulary/core_test.cljc {:file #object[java.io.File 0x7a68818c "/home/eric/Projects/Ont-app/vocabulary/test/ont_app/vocabulary/core_test.cljc"], :clojure.error/phase :compilation}
...

Caused by: java.lang.IllegalStateException: Attempting to call unbound fn: #'ont-app.vocabulary.lstr/read-LangStr
Is this just not the way cljs works, or is there something I can do to make the read-LangStr function available to cljs at compile-time?

seancorfield23:12:53

@eric.d.scott You have #lstr defined where...?

Eric Scott23:12:54

(ns ont-app.vocabulary.core-test (:require ... [ont-app.vocabulary.lstr :as lstr] ... ))

Eric Scott23:12:22

There's a data_readers.cljc:

Eric Scott23:12:28

{
 lstr ont-app.vocabulary.lstr/read-LangStr
}

Eric Scott00:12:30

I'm trying to support clojurescript in this and its sibling libraries where I can, but I have to admit, I don't actually play in this space much at all.

seancorfield00:12:35

Mostly I wanted to check you had .cljc on data_readers. I don't play much in cljs so I also often struggle to add cljs support to libraries. What you have looks "right" to me so I'm not sure what to suggest.

seancorfield00:12:17

I believe you can use .-lang for Clojure as well and get rid of the reader conditional in this though:

(defn lang 
  "returns the language tag associated with `langStr`"
  [^LangStr langStr]
  (#?(:clj .lang :cljs .-lang) langStr))

Eric Scott00:12:43

Well I appreciate your taking the time - here and in all the work you do!

Eric Scott00:12:21

I did not know that. Thanks!

john00:12:56

So, you're saying when you use read-string in the reader conditional, it does work in cljs, but when you try it like the clj version, it does not work?

Eric Scott00:12:52

That's right. Says read-LangStr is an unbound function.

john00:12:08

Where is ont-app.vocabulary.lstr being required in, within the rest of your app (other than your tests)? You might need to just require that in, in your core namespace, and then it'll be available

john00:12:30

I mean it could be a dead code elimination thing, if nothing is requiring in that ns

Eric Scott00:12:35

I'm requiring it in ont-app.vocabulary.core, but I don't actually have any reason to use it in that module.

john00:12:25

I don't see it required in

Eric Scott00:12:18

Oh. I guess I haven't committed this branch yet, and that's when I added the requirement.

Eric Scott00:12:18

I also tried requiring cljs.reader, and declaring register-tag-parser!, with no effect.

seancorfield00:12:34

Heh, yeah, I looked at the repo and didn't see the test you were talking about 🙂

Eric Scott00:12:28

Yeah this is issue # 12.

seancorfield00:12:07

Do you want to commit the WIP as a new branch so we can take it for a spin?

seancorfield00:12:04

I think the problem here isn't so much ClojureScript and the data_readers file, I think it might actually be a bug in cljsbuild?

seancorfield00:12:33

I think it is trying to compile the data_readers file...

Eric Scott00:12:38

Wow. I've never used anything but cljsbuild. What are the alternatives to it?

seancorfield00:12:57

Good question 🙂

john01:12:43

With calva all you need is a deps.edn with your dependencies. It can launch a "vanilla repl" in the browser or node and inject the necessary deps to make the repling work. You can also launch a repl yourself with clj -M -m cljs.main --repl-env node

seancorfield01:12:22

Doesn't help with running tests from the command-line @U050PJ2EU

seancorfield01:12:38

Here's how I deal with ClojureScript in a non-Leiningen project: https://github.com/seancorfield/honeysql/blob/develop/deps.edn @eric.d.scott

john01:12:09

If you want fancier stuff, you can bring in figwheel or shadow, but it's good to also test without those, to ensure it works in all environments

Eric Scott01:12:44

Unfortunately, I have to step away from my desk now, but when I come back I'll familiarize myself with this and try it out.

john01:12:47

Yeah @eric.d.scott would probably have to bring in the deps.edn equivalent of doo, for testing

Eric Scott01:12:03

Thanks so much for your help!

seancorfield01:12:43

I put a modified version of HoneySQL's deps.edn in there and managed to get the exact same error running it via the cljs test runner -- so it's not a cljsbuild issue. But I don't know what it is yet 😐

seancorfield01:12:07

Ah, I'm now getting a different error:

(! 612)-> clojure -M:test:cljs
#error {
 :cause failed compiling constant: dog; ont_app.vocabulary.lstr.LangStr is not a valid ClojureScript constant.
 :data {:constant #ont-app/lstr "dog@en", :type ont_app.vocabulary.lstr.LangStr, :clojure.error/phase :compilation}
 :via
 [{:type clojure.lang.ExceptionInfo
   :message failed compiling file:/Users/sean/clojure/fresh/vocabulary/test/ont_app/vocabulary/core_test.cljc
   :data {:file #object[java.io.File 0x62672ad5 /Users/sean/clojure/fresh/vocabulary/test/ont_app/vocabulary/core_test.cljc], :clojure.error/phase :compilation}
   :at [clojure.core$ex_info invokeStatic core.clj 4739]}
  {:type clojure.lang.ExceptionInfo
   :message failed compiling constant: dog; ont_app.vocabulary.lstr.LangStr is not a valid ClojureScript constant.
   :data {:constant #ont-app/lstr "dog@en", :type ont_app.vocabulary.lstr.LangStr, :clojure.error/phase :compilation}
   :at [clojure.core$ex_info invokeStatic core.clj 4739]}]
 :trace
 [[clojure.core$ex_info invokeStatic core.clj 4739]
  [clojure.core$ex_info invoke core.clj 4739]
  [cljs.compiler$fn__3260 invokeStatic compiler.cljc 304]
  [cljs.compiler$fn__3260 invoke compiler.cljc 301]
  [clojure.lang.MultiFn invoke MultiFn.java 229]
  [cljs.compiler$emit_constant_no_meta invokeStatic compiler.cljc 283]

seancorfield02:12:03

That problem above is because the defmethod for emit-constant* isn't registering correctly. If you change LangStr to a defrecord, it all works -- because ClojureScript's compiler knows about records. But because it is a deftype, when it is used in a context that expects to emit a constant, it fails. I still get that weird error from cljsbuild with Leiningen (but the tests do run and pass). The only change I needed to make was to replace the deftype with this:

(defrecord LangStr [s lang]
  Object
  (toString [_] s))

seancorfield02:12:23

I added this deps.edn file so I could test without Leiningen in the picture as well:

{:mvn/repos {"sonatype" {:url ""}}
 :paths ["src" "resources"]
 :deps {org.clojure/clojure {:mvn/version "1.9.0"}}
 :aliases
 {;; running tests/checks of various kinds:
  :test ; clojure -X:test to run clj tests
  {:extra-paths ["test"]
   :extra-deps  {io.github.cognitect-labs/test-runner
                 {:git/tag "v0.5.0" :git/sha "48c3c67"}}
   :exec-fn     cognitect.test-runner.api/test}

  ;; various "runners" for tests/CI:
  :cljs ; clojure -M:test:cljs to run cljs tests
  {:extra-deps {olical/cljs-test-runner {:mvn/version "3.8.0"}}
   :main-opts ["-m" "cljs-test-runner.main"]}}}
Here's the (edited) output from running the tests via Leiningen with just that one defrecord change:
(! 648)-> lein clean && lein test && lein cljsbuild test

lein test ont-app.vocabulary.core-test

Ran 6 tests containing 34 assertions.
0 failures, 0 errors.
Compiling ClojureScript...
Reloading Clojure file "ont-app.vocabulary.core-test" failed.
clojure.lang.Compiler$CompilerException: Syntax error reading source at (ont_app/vocabulary/core_test.cljc:182:35).
...
Caused by: java.lang.IllegalStateException: Attempting to call unbound fn: #'ont-app.vocabulary.lstr/read-LangStr
...
Compiling ["resources/test/compiled.js"] from ["src" "test"]...
Dec 28, 2021 6:16:08 PM com.google.javascript.jscomp.LoggerErrorManager println
WARNING: /Users/sean/clojure/fresh/vocabulary/resources/test/js/compiled/out/cljs/compiler.js:861:78: WARNING - [JSC_REGEXP_REFERENCE] References to the global RegExp object prevents optimization of regular expressions.
  861| cljs.compiler.emit_constant_STAR_.cljs$core$IMultiFn$_add_method$arity$3(null,RegExp,(function (x){
                                                                                     ^^^^^^

Dec 28, 2021 6:16:08 PM com.google.javascript.jscomp.LoggerErrorManager printSummary
WARNING: 0 error(s), 1 warning(s)
Successfully compiled ["resources/test/compiled.js"] in 33.825 seconds.
Running ClojureScript test: test

WARNING: You have $CLASSPATH set, probably by accident.
It is strongly recommended to unset this before proceeding.
Dec 28, 2021 6:16:44 PM com.google.javascript.jscomp.LoggerErrorManager println
WARNING: /Users/sean/clojure/fresh/vocabulary/resources/test/js/compiled/out/cljs/compiler.js:861:78: WARNING - [JSC_REGEXP_REFERENCE] References to the global RegExp object prevents optimization of regular expressions.
  861| cljs.compiler.emit_constant_STAR_.cljs$core$IMultiFn$_add_method$arity$3(null,RegExp,(function (x){
                                                                                     ^^^^^^

Dec 28, 2021 6:16:44 PM com.google.javascript.jscomp.LoggerErrorManager printSummary
WARNING: 0 error(s), 1 warning(s)

;; ======================================================================
;; Testing with Node:


Testing ont-app.vocabulary.core-test

Ran 7 tests containing 65527 assertions.
0 failures, 0 errors.

2
Eric Scott14:12:28

Holy moly, You really dug into this @U04V70XH6! Thank you so much!

seancorfield17:12:20

@eric.d.scott Was there a particular reason you were using deftype instead of defrecord?

john17:12:08

Wondering if he could have changed the syntax on that defmethod, maybe quoting the symbol or something

seancorfield17:12:25

@U050PJ2EU I tried several variants, unsuccessfully, before giving up and just switching to defrecord. If you figure out a way to make the emit-constant* multimethod work, please post your solution back in this thread. I suspect it may be an evaluation ordering issue, that the defmethod needs to execute before the compiler tries to read the #lstr form -- but I don't know enough about how the cljs compiler works to be able to make that work?

Eric Scott17:12:38

@U04V70XH6 I don't see any references to this in my notes when I was writing this. Probably just an arbitrary decision.

Eric Scott17:12:10

However, I've tried changing deftype to defrecord, and am still having the same problem.

seancorfield17:12:44

If you're using lein, you'll need lein clean I think, for that to get picked up.

seancorfield17:12:09

If you've updated the repo branch on GH, I can take another look at it.

seancorfield17:12:02

As noted above, cljsbuild still throws that error but the tests work. Using the CLI and deps.edn instead, I do not see that error (and the tests work).

Eric Scott17:12:33

I just pushed the change to the same branch. I've been doing lein clean. I guess it's finally time for me to familiarize myself with deps.edn and its related tools so I can apply your script with some understanding of what is going on. Probably time for me to roll up my sleeves and deepen my understanding of the cljs compiler as well.

seancorfield17:12:21

Yeah, that update completely fails with lein cljsbuild test for me, whereas the version I had before worked. But it works with the CLI / deps.edn.

seancorfield18:12:58

I sent you a PR with a basic deps.edn for that branch.

seancorfield18:12:58

rm -rf cljs-test-runner-out/ && clojure -M:test:cljs && clojure -X:test
That will run the tests as ClojureScript and then as Clojure. And also remove the JS output tree just to ensure that it's a fully-clean compile/test.

Eric Scott19:12:38

Fun fact: the following procedure will trick my cljsbuild into working: - edit the code not to barf with reader conditionals where the cljs version calls read-string - lein cljsbuild once - this will show a stack trace if the previous build failed , but then succeed - edit the direct tagged literal back in - lein cljsbuild test - this succeeds

Eric Scott19:12:57

Thanks for the PR @U04V70XH6, I'll try it out