datascript icon indicating copy to clipboard operation
datascript copied to clipboard

Self-hosted ClojureScript compatibility

Open djwhitt opened this issue 8 years ago • 11 comments

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.

djwhitt avatar Dec 18 '17 22:12 djwhitt

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

arichiardi avatar Dec 18 '17 22:12 arichiardi

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 :(

tonsky avatar Dec 19 '17 11:12 tonsky

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

arichiardi avatar Dec 19 '17 17:12 arichiardi

@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?

djwhitt avatar Dec 20 '17 17:12 djwhitt

yes, I prefer it dependency-free

tonsky avatar Dec 20 '17 19:12 tonsky

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

bhurlow avatar Apr 15 '18 12:04 bhurlow

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?

viebel avatar Jun 29 '19 18:06 viebel

@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

tonsky avatar Jul 01 '19 15:07 tonsky

@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.

djwhitt avatar Jul 14 '19 02:07 djwhitt

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

viebel avatar Jul 23 '19 05:07 viebel

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

viebel avatar Sep 04 '20 09:09 viebel