tonsky / datascript

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

Self-hosted ClojureScript compatibility #244

Open djwhitt opened 6 years ago

djwhitt commented 6 years ago

It would be really handy to be able to use DataScript in Lumo. Is this something there's interest in supporting? I've done some basic proof of concept work using macrovich in a branch, so I'm pretty confident it's doable with a reasonable amount of effort.

If there is interest, I'm happy to do the work and make a PR. Though, it would probably be a good idea to talk through the general approach first.

arichiardi commented 6 years ago

@djwhitt Not surprising, I am interested ;) Feel free to hit me with questions about macrovich as well (I ported honeysql with it)

tonsky commented 6 years ago

I’d love to read brief description of what such process (converting to self-hosted CLJS) looks like. What does it involve? Is it possible to have single codebase that works both ways (normal CLJS and self-hosted)?

Realistically, though, I doubt I’ll have any time in the near future to review patches and merge into master. It’s been hard lately to find time to support any DS activity at all :(

arichiardi commented 6 years ago

So cljs-js support is supposedly very easy if you don't have many macros in the project. The problem with macros is that in cljs-js while they still need to be in a separate compilation stage (the JVM one - Clojure), they cannot make use of any Clojure. Also, sometimes rules in cljs-js around definition time are weird. For more info, Macrovich has a good intro readme: https://www.npmjs.com/package/macrovich

djwhitt commented 6 years ago

@arichiardi is right. Macros are the big issue. In self-hosted CLJS they're evaluated by the CLJS compiler rather than the Clojure compiler. That means regular ':clj' reader conditionals around macros need to be replaced by something that evaluates them in CLJS, but only if self-hosted CLJS is being used for compilation. macrovich provides macros to do this. They're not much code though, so they could be easily incorporated into the DataScript code base if adding a dependency isn't desirable.

Another related issue is that namespaces containing macros in CLJS are evaluated twice. Macrovich provides a 'usetime' macro to prevent non-macro code from getting evaluated when macro namespaces are evaluated the first time.

Oh, and of course, any JVM specific code in macro bodies themselves needs to be placed behind conditionals and a JS version added.

I think the double evaluation is the most annoying part. If we don't want to wrap 'usetime' calls around tons of code, some macros should probably be moved into separate namespaces.

@tonsky given your time constraints I'll try to come up with the most reasonable patch I can manage and make a PR that includes explanation for my changes. Then you can review it as you find time. I'd like to see this happen, but I'm not on a deadline and I totally understand not having time. One question though, do you want to keep DataScript dependency free or is a macrovich dep ok?

tonsky commented 6 years ago

yes, I prefer it dependency-free

bhurlow commented 6 years ago

I'd also love to use datascript with lumo. @djwhitt's first pass branch worked for me without a hitch

viebel commented 5 years ago

I can confirm that this branch works fine on Klipse. See this demo page What is missing in order to merge the macrovich branch?

@tonsky @djwhitt in order to keep datascript dep free, we could just copy the code of macrovich which is really minimal. It's a single short file.

If you need it I could create a PR for it. Thoughts?

tonsky commented 5 years ago

@djwhitt looks pretty easy to pull in. I would certainly prefer to have macrovich vendored into DS source though. Care to produce a PR? The only thing I would ask is to keep formatting/indentation changes to a minimum

djwhitt commented 5 years ago

@tonsky @viebel I would like to get back to this, but I'm honestly not sure when I'll have time. If anyone else wants to take a crack at a patch with an embedded version of macrovich in the meantime, feel free.

viebel commented 5 years ago

@djwhitt Can you take the time to rebase your branch with latest master and I'll happily vendor macrovich in DS source.

viebel commented 4 years ago

@djwhitt Can you take the time to rebase your branch with latest master and I'll happily vendor macrovich in DS source?