Improve CLJS dev setup and testing org

[mrrodriguez]

The CLJS dev setup is less than ideal. I'd like to get it working with a more modern figwheel/sidecar sort of setup.

Also, the cljs testing seems to be disorganized. I think it is not very clear where cljs & cljc files are found at. I'm not even sure it makes sense to have a "common", "clojurescript", and "clojure" directories under "src/test" anymore since read conditionals and the various file extensions are respected now. It makes the sharing of files more confusing and hard to find in my opinion.

I only bring up the testing setup since there seem to be issues getting Figwheel compiling these namespace and to get it all working again, I'd likely want to get this stuff sorted out.

[WilliamParker]

+1 to doing this. I haven't had the time or familiarity/more time to become familiar myself but I agree that the CLJS setup could use improvement and from what I read it sounds like the overall CLJS dev ecosystem has come a long way since Clara's CLJS build was set up.

[sparkofreason]

You might take a look at shadow-cljs as well. We went from boot to figwheel and finally settled on shadow-cljs, have been very happy with it.

[mrrodriguez]

@sparkofreason

I’ve been following shadow cljs a bit. I agree it seems useful in some cases. For Clara though there really isn’t any fancy JS interop dependency management to worry about. So I planned to just do a pretty straightforward lein figwheel setup for now. The biggest problem right now is some of these defsession sort of cljs namespaces aren’t compiling with figwheel.

Also, Clara supports a quite old cljs version. For development time, I think I’m going to use a more recent one for dev tooling sake. We can still run tests against whatever old version we want to support.

I am going to look closer at the figwheel compilation problem stuff and try to figure out what to do about our defsession Usage once I understand it a bit more.

[alex-dixon]

Here's the leiningen/ figwheel config I borrowed from Luminus that I posted a while back: #285 (comment).

I still use this for CLJS REPL with figwheel. There may be better alternatives these days but it works for me.

[mrrodriguez]

Thanks @alex-dixon I'll check it out. I'm pretty familiar with cljs and figwheel stuff nowadays since I've been using it at work for a while now. It's good to see what you have though.

There are some deeper issues in the cljs (test) setup in Clara right now where I think we are abusing the cljs compiler and "getting lucky" to have the build pass.
Some details are in #382 (comment)
but I plan to do more investigation and follow-up on this, likely when I get a PR working as I want for this issue.

[mrrodriguez]

In doing this work, I think we should make the project compatible with JDK11 since it is being used more often now.

The WIP branch is https://github.com/mrrodriguez/clara-rules/tree/iss388-cljs-project

https://www.deps.co/blog/how-to-upgrade-clojure-projects-to-use-java-11/
is a good guide to this end.
Our problems right now are primarily only in the cljs-side, so I think it relates to this issue. I'm looking into this now.

https://stackoverflow.com/a/52502208/924604
mentions how in jdk11 the "--add-modules=java.xml.bind" will no longer work, since the module doesn't exist in the jdk distribution at all, it's a separate lib jar. However, we don't need this in newer cljs versions from this change:
https://dev.clojure.org/jira/browse/CLJS-2377

from cljs version 1.10.238

I am planning to at least make a jdk11 profile that uses newer versions of clj and cljs and doesn't have this issue for those wishing to build and run that way.

This means clj 1.10 as well for cljs 1.10.x

[mrrodriguez]

Adding a note on some current progress:

When using JDK8 and bump to versions:

[org.clojure/clojure "1.10.0"]
[org.clojure/clojurescript "1.10.520"]

then lein test from the current state of master
it fails in clara.test-complex-negation ns due to a malformed throw expression. This one is easy to fix, it is a typo and should have wrapped an ex-info call around the args.

Fixing this, then it passes the tests, but fails the subprocess with:

Running ClojureScript test: phantom-simple
SyntaxError: Invalid regular expression: range out of order in character class

  file:///my/path/to/clara-rules/target/js/simple.js:5640 in RegExp
TypeError: undefined is not an object (evaluating 'clara.test.run')

  undefined:2
  :3

I’ll look at this some, just noting it here for now.

[mrrodriguez]

Update:

I addressed the issue of #388 (comment)

The html files used for the test runners did not have the

  <meta charset="utf-8">

This was causing UTF-8 encoded string parsing problems in newer compiled JS that was coming from Google Closure lib, that is bundled in automatically by CLJS. More recent versions of CLJS take more recent versions of Google Closure. So at some point, this problem was introduced. However, our html files should have this meta tag anyways to be well-behaved.

The last thing I'm working on is getting figwheel running without problems in the ways that is most common to use it with. I'm hitting some old issues I fought with before about how defsession is compiled and cannot resolve symbols during figwheel's cljs compilation. So once I have that sorted out, I will have a PR for this issue.

[mrrodriguez]

I've looked a bit more into this recently and created this gist
https://gist.github.com/mrrodriguez/dc48281196f99e30ee84c56c0ec6f63a#file-foo-cljs

to demonstrate a problem with trying to use clojure.core/eval during CLJS macroexpansion. After discussing this more in the Clojurians Slack channel #clojurescript , I've been convinced that it is fundamentally flawed for clara.macros/defsession to attempt to perform a clojure.core/eval on arbitrary forms the way it currently is doing it. clojure.core/eval operates with no built-in "awareness" of the CLJS compilation cycle. This includes it not necessarily being aware of vars being defed during CLJS compilation, etc. This was confirmed to me by David Nolen [1] in the Slack channel.

There is some history behind why defsession is the way that it is now, but I think some things may have just "happened to work" somehow in the past. They are not reliable however. I immediately run into trouble when working with this in different, modern CLJS dev environments, such as Figwheel. However, the process isn't stable anyways.

I'm going to explore changing this, but it'll require a few things to consider:
(1) defsession is expected to support namespace symbols, as well as collections of symbols referencing production structures. These need to be resolved fully to productions at macroexpansion time, in a way that doesn't rely on clojure.core/eval.
(2) defsession was changed in #94 that was meant to allow arbitrary forms to be evaluated at macroexpansion time to production structures within defsession. This relies on the use of clojure.core/eval and the clj environment happening to be able to eval the forms given.

(1) can probably be addressed by explicitly removing quoted symbols - since we support them for backward compatibility & also resolving symbols via CLJS analyzer functions.

(2) is more difficult. Perhaps it can still fall back to eval if the CLJS-side symbols are already resolved and removed from consideration. However, this will still lead to brittle calls to eval.

From the referenced #94 issue

It is possible to set up a server which can compile and serve up new
rule-containing JavaScript files on demand, generating sets of rules
dynamically from (for example) a database.

It isn't even clear what this means. defsession is a compile-time macro, I don't see how it facilitates on-demand rule generation via a service. If that is being done, I'm guessing it is via abusing eval some more to expand the macro at runtime. I don't think we should tailor to that case. If we want on-demand CLJS session assembly, we can expose some of the underlying parts of defsession as functions.

---

[1] David Nolen conversation snippets regarding CLJS macroexpansion-time eval:

Dnolen:

clojure.core/eval is exactly that
has nothing to do with ClojureScript, doesn't know anything about ClojureScript
if you need it, expect to do a lot of work

Me:

sure, I know it’s a stretch, but was curious if there are anyways to try to “do things” during macroexpansion that may involve eval-like behavior

Dnolen:

there isn't
it's just Clojure eval
there's no way to magically make it know about ClojureScript stuff
it doesn't know about nses, vars, nothing
if anything [you have done before] worked it was probably simple and you got lucky
yeah trying to make it do something useful would make my brain hurt