tonsky / datascript

Immutable database and Datalog query engine for Clojure, ClojureScript and JS
Eclipse Public License 1.0
5.5k stars 309 forks source link

Query parsing breaks on cljs 1.7.48 advanced compilation #108

Closed milt closed 9 years ago

milt commented 9 years ago

Hey! Thanks again for the amazing work on this. I'm seeing an odd issue when trying to compile a project using datascript under cljs 1.7.48 with :optimizations :advanced, where query parsing will fail:

calling:

(let [db (-> (d/empty-db)
             (d/db-with [ { :db/id 1, :name  "Ivan", :age   15 }
                          { :db/id 2, :name  "Petr", :age   37 }
                          { :db/id 3, :name  "Ivan", :age   37 }
                          { :db/id 4, :age 15 }]))]
  (print (d/q '[:find ?e
                :where
                [?e :name]] db)))

Throws:

Uncaught #error {:message "Cannot parse binding, expected (bind-scalar | bind-tuple | bind-coll | bind-rel)", :data {:error :parser/binding, :form .}}

from:

(defn parse-binding [form]
  (or (parse-bind-coll form)
      (parse-bind-rel form)
      (parse-bind-tuple form)
      (parse-bind-ignore form)
      (parse-bind-scalar form)
      (raise "Cannot parse binding, expected (bind-scalar | bind-tuple | bind-coll | bind-rel)"
             {:error :parser/binding, :form form})))

Works fine in development. I looked over #30, but it looks like the externs are loading fine, if I explicitly include them I get an error that they're already there. Since DS works fine on 1.7.28 I'm thinking it might have something to do with this commit but I'm really not sure.

I threw together a minimal project to demonstrate the problem, just run scripts/release to build it.

milt commented 9 years ago

1.7.48 also bumps the closure deps, so that might also be something to look at.

tonsky commented 9 years ago

@milt You were right about commit, that helped a lot. Thanks! DataScript had a bad luck using both '$ and '. symbols in the API extensively, exactly the two who was conflicting.

Waiting for this to be applied: http://dev.clojure.org/jira/browse/CLJS-1432

milt commented 9 years ago

Awesome, thanks!

tonsky commented 9 years ago

Issue has been fixed in 1.7.122. Check it out

ThomasDeutsch commented 9 years ago

i still get an error, using:

[org.clojure/clojure "1.7.0"]
[org.clojure/clojurescript "1.7.122"]
[datascript "0.11.6"]

Error-message:

Cannot compare {0 [540 960]} to {0 [540 1140]}

cljs$core$compare @ core.cljs:1668datascript$core$cmp_val_quick @ core.cljc?rel=1441350387628:293
tonsky commented 9 years ago

@ThomasDeutsch that’s another bug. Values should be comparable. Maps are not comparable. Either wrap them or extend to implement IComparable. I’m going to add “don’t index” attribute, but only in a next version