Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue with datalog under advanced compilation #30

Closed
robert-stuttaford opened this issue Oct 2, 2014 · 13 comments
Closed

Issue with datalog under advanced compilation #30

robert-stuttaford opened this issue Oct 2, 2014 · 13 comments

Comments

@robert-stuttaford
Copy link
Contributor

I built Radiant, a Datascript sandbox, recently. In it, I have found an issue with Datalog under advanced compilation.

This page hosts the :optimizations :none version of Radiant:

http://www.stuttaford.me/radiant/

As you can see, the datalog query runs and produces a result table.

This page hosts the :optimizations :advanced version of Radiant:

http://www.stuttaford.me/radiant-advanced/

You can see that I have printed out the parsed query, the full database, and the raw result from the datalog query above the result table.

The results do not match that of the first link. Looking at the results, for some reason, all the e and v values are missing.

You can visit the "Datoms" section to see that the datoms function is working as expected.

How do I go about debugging this?

I have already ruled out the possibility of externs not being used. They are.

Thank you for your time :-)

@tonsky
Copy link
Owner

tonsky commented Oct 15, 2014

@robert-stuttaford , sorry, I was really busy and haven’t looked at your issue.

First, make sure your project.clj looks like this:

  { :id "prod"
    :source-paths ["src"]
    :compiler {
      :externs  ["datascript/externs.js"]
      :output-to     "....min.js"
      :optimizations :advanced
    }}

Second, what exact version of DS you use?

Third, if that’s possible, maybe you can give me access to source code so I can take a look at it? Maybe partially (I’m interested most about parts where you create data, where you select it and where you display it, and project.clj of course). prokopov@gmail.com

@robert-stuttaford
Copy link
Contributor Author

Thanks @tonsky!

I'm using version 0.4.0. The externs are definitely in place.

All the source code is here:

https://github.com/robert-stuttaford/stuttaford.me/blob/master/src/stuttaford/radiant.cljs
https://github.com/robert-stuttaford/stuttaford.me/blob/master/src/stuttaford/radiant/

The datalog stuff is here:

https://github.com/robert-stuttaford/stuttaford.me/blob/master/src/stuttaford/radiant/datalog.cljs

And the database is created here:

https://github.com/robert-stuttaford/stuttaford.me/blob/master/src/stuttaford/radiant/model.cljs#L20-L25

The source data is in the source of the page at:

http://www.stuttaford.me/radiant-advanced/

It's the really big EDN blob.

Thanks for taking the time to look into it!

@tonsky
Copy link
Owner

tonsky commented Oct 15, 2014

Oh, missed that when looked at your github. Actually I have a hypothesis, try (.log js/console (pr-str query)) right before you execute a query. Judging from symptoms, it looks like that symbols in :find and in :where are, for some reason, different.

@robert-stuttaford
Copy link
Contributor Author

Anticipating that, I had already printed the output of several things directly on the page at:

http://www.stuttaford.me/radiant-advanced/

In order, it's the query as successfully parsed by Datascript, the entire database, and finally the result of that query on that database.

Here's the source:

https://github.com/robert-stuttaford/stuttaford.me/blob/master/src/stuttaford/radiant/datalog.cljs#L57-L59

You can see that the symbols are identical; for example, here's the parsed query:

{:find [?e ?a ?v], :in [$], :where [[?e ?a ?v]]}

@tonsky
Copy link
Owner

tonsky commented Oct 16, 2014

You have pretty complex build schema out there :) So this is what I believe is going on: you build core modules and radiant modules separately and then include them both on a single page. Advanced compilation doesn’t work like that. If you compile something separately you’ll probably get same symbols assigned in first and in a second case to very different things. Google Closure compile does aggressive renaming in order to minimize code size, so it renames everything to names like a, ab, ga, shortest names possible. If you do two independent compilations you’ll get name clashes all around the place. It’s miracle anything works at all. Try build single JS file via one compilation step and put everything in it.

Also you put Google Analytics to the page. It binds to ga symbol (unfortunately, quite short). I had a case once where even that caused name clash with compiled cljs code. So I ended up adding ga to externs file for that project. Consider that too.

@robert-stuttaford
Copy link
Contributor Author

I'm using shadow (https://github.com/thheller/shadow-build) instead of cljsbuild which supports the underlying Google Closure module capability - that is, multiple interoperating js files. core.js and radiant.js are compiled at the same time with the same :advanced name alterations.

If this were not so, a lot more than just the Datalog query would be broken, as all the 3rd-party libraries are in core.js. Om, Om-tools, Om-bootstrap, Datascript etc. And aside from the Datalog thing, they all work fine.

Even so, I'll build it as a single JS and see if that helps.

Thanks for the tip about Google Analytics, I'll include externs for that as well.

@tonsky
Copy link
Owner

tonsky commented Oct 19, 2014

@robert-stuttaford Can you keep me in the loop if you can make it work with cljs-compile? Maybe there’s some incompatibility between DS and shadow-build, if that’s the case, I would look into that deeper

@tonsky
Copy link
Owner

tonsky commented Nov 6, 2014

Here’s what I found:

If you put externs file to src/ folder of your project (I used src/stuttaford/ext.js), everything compiles and works nicely. But if you reference them as "datascript/externs.js" they don’t work. Looks like shadow cannot load them from classpath for some reason.

Probably related comment here https://github.com/thheller/shadow-build/blob/master/src/clj/shadow/cljs/build.clj#L910

I’ll close the issue for now as problem seems to be not in DataScript

@tonsky tonsky closed this as completed Nov 6, 2014
@tonsky
Copy link
Owner

tonsky commented Nov 6, 2014

I managed to get it work:

diff --git a/project.clj b/project.clj
index e9a55bf..fdb6418 100644
--- a/project.clj
+++ b/project.clj
@@ -25,13 +25,13 @@
   :profiles {:dev {:source-paths ["dev"]
                    :dependencies
                    [[org.clojure/tools.namespace "0.2.7"]
-                    [thheller/shadow-build "0.9.3" :exclusions [org.clojure/clojurescript]]
-                    [org.clojure/clojurescript "0.0-2342"]
+                    [thheller/shadow-build "0.9.5" :exclusions [org.clojure/clojurescript]]
+                    [org.clojure/clojurescript "0.0-2371"]
                     [om "0.7.3"]
                     [prismatic/om-tools "0.3.3" :exclusions [org.clojure/clojure]]
                     [racehub/om-bootstrap "0.2.9" :exclusions [org.clojure/clojure]]
                     [sablono "0.2.22" :exclusions [com.facebook/react]]
-                    [datascript "0.4.0"]
+                    [datascript "0.5.1" :exclusions [org.clojure/clojurescript]]
                     [com.cemerick/url "0.1.1"]
                     [cljs-http "0.1.16"]
                     [secretary "1.2.1"]]
@@ -49,6 +49,5 @@
                           sablono.core
                           secretary.core
                           datascript]
-           :externs     ["datascript/externs.js"]
            :modules     [{:id :codex   :main stuttaford.codex}
                          {:id :radiant :main stuttaford.radiant}]})
diff --git a/dev/shadow.clj b/dev/shadow.clj
index 313a812..7938c9e 100644
--- a/dev/shadow.clj
+++ b/dev/shadow.clj
@@ -46,7 +46,8 @@
                            :pretty-print  true}
               :production {:optimizations :advanced
                            :pretty-print  false
-                           :externs       (compose-externs externs)}))))
+                           :externs       (compose-externs externs)
+                           :ups-externs   ["datascript/externs.js"]}))))

 (defn configure-source-paths [state source-paths]
   (reduce (fn [state source-path]

Still puzzled though what’s the difference between :externs and :ups-externs and why react/externs/react.js works in the former but datascript/externs.js only works in the latter.

Here’s part I looked at: https://github.com/clojure/clojurescript/blob/master/src/clj/cljs/closure.clj#L188

@robert-stuttaford
Copy link
Contributor Author

Wow @tonsky, great detective work! Thanks for digging into this so deeply, and for fixing it. It is much appreciated. I never knew about :ups-externs before; you're right, it is very odd. @thheller, any thoughts?

@thheller
Copy link
Contributor

thheller commented Nov 8, 2014

Hey, never heard of :ups-externs either. Putting "datascript/externs.js" into the normal :externs option doesn't work?

Seems like a 3rd party library is supposed to ship a deps.cljs to inform the cljs.compiler about its externs, but looking at the warning it either never reached a stable point or was abandoned because nobody used it.

https://github.com/clojure/clojurescript/blob/9fd6bf5bd55421c3d5becacc5230ed661d6fb3c3/src/clj/cljs/closure.clj#L838

@thheller
Copy link
Contributor

thheller commented Nov 8, 2014

It seems like a useful option to have though. Otherwise everyone that uses a library like datascript must remember to also provide the "datascript/externs.js" in the build.

Maybe it would be prudent to discuss this issue in the CLJS Jira or Google Group, seems like a quality of life change that should have a default solution not one that every build tool/library handles differently.

@tonsky
Copy link
Owner

tonsky commented Nov 10, 2014

@thheller totally agree! thanks for bringing up that issue. Posted on google group: https://groups.google.com/d/topic/clojurescript/LtFMDxc5D00/discussion

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants