Issue 241: Wrap the fact-type-fn and ancestors-fn to add the fix for …

[WilliamParker]

…issue 149 to the ClojureScript version

[mrrodriguez]

The change looks fine. However, I'm starting to wonder why the cljs has to have so much repetitive code with clj side for this stuff?

It looks to me like there could be quite a bit more sharing in these building session steps, unless I'm missing something?

That could be a separate concern though. I'm just curious the thoughts on that are. Looks like a lot of duplication going on.

[alex-dixon]

@mrrodriguez Ryan also mentioned duplication of CLJ/CLJS (specifically in tests) #288 (comment). There's also a proposal for a CLJS macro that behaves more like mk-session (#292).

William has mentioned Clara's CLJ code uses eval which hasn't worked well in CLJS in the past, but that it may have changed recently. I looked around a bit and think he may be referring to cljs.js/eval released in CLJS 1.7.10.

[mrrodriguez]

@alex-dixon thanks for the reminder. I read that from Ryan before and didn't realize it was the same code we were talking about.

I think the eval suggestion may be fine to explore for testing, however, I don't think that'd be something that we'd actually want to have happening in real production use.

However, there are a few places here that look like it there could just be the correct functions pulled out to share code between the clj vs cljs impl. The stuff that isn't involved with code gen or macros.

e.g. the alpha fn wrapper logic doesn't strike me as something that would need much separation frm the cljs vs clj side.

Anyways, it isn't a big issue for the sake of this PR. I appreciate your input though.

[WilliamParker]

I've looked over this again and I don't see any problems; Mike looked at it as well from #295 (comment). We should give some further thought to merging our Clojure and ClojureScript code bases to have as much common code as possible but this can be addressed on other issues.

Merging.