Implement cljs version of ordered-map

[plexus]

In a case of parallel evolution we wrote this as a utility in a project we are working on with the Nextjournal folks, unaware of this project. We only wrote the cljs version, and it seems this project only has a clj version, so that works out well.

This only implements ordered-map at the moment, so no transient map or sets. Still I assume this may already be useful for some, and can help as a guide to implement the rest. We used Datascript's entity implementation as a guide for which protocols to implement.

Partially addresses #43 .

[borkdude]

My 2 cts: I think having an incomplete implementation is OK, the map is the most used one anyway. I think it's good to merge this, perhaps add some unit tests?

[danielcompton]

I think having an incomplete implementation is OK, the map is the most used one anyway

Yep, I don't see this as a blocker.

Could you see if the tests can run against both implementations as CLJC? If not, someone else can add them later and we could mark the map implementation as alpha for now?

[plexus]

I'll have a look at the tests, those comments at the end were meant to become tests anyway. For the existing tests I'll have to split them up a bit in the ones that are cljc and the ones that are still only clj.

[plexus]

I made the map tests cljc to the extent possible, made the data reader cljc, and added kaocha setup for running the clj and cljs tests. I didn't add kaocha to the CI yet.

[borkdude]

@plexus Just out of curiosity, is kaocha able to run tests in advanced compiled mode too nowadays? (That would help catching nasty advanced compiled bugs during testing).

[plexus]

Only with kaocha-cljs2, which is a lot more involved to set up, so I tend to stick to kaocha-cljs for simple libraries like this one.

[borkdude]

It's slightly concerning to me that we're not testing with advanced. Perhaps we should also set up shadow or cljs-test-runner for this, in addition to kaocha, to run advanced in CI? Or is this unimportant for this library since it doesn't do a lot of interop in CLJS (I haven't checked).

[plexus]

Also something I should point out, the #ordered/map reader literal works in both clj and cljs, but I had to resort to a bit of a hack. It seems the official line is that cross platform reader conditionals are not supported.

https://clojurians.slack.com/archives/C03S1KBA2/p1629984675020400

[plexus]

It's slightly concerning to me that we're not testing with advanced.

This is a valid concern, but this is fairly straightforward ClojureScript code, and we've been using this implementation in an advanced build for some time already, so I think the payoff for setting up an advanced CI build for this is limited, given that this also isn't a trivial thing to set up.

[borkdude]

Fair enough!

[plexus]

I also just became aware of linked, which is very similar in scope to ordered, and included cljs support.

[borkdude]

I'm aware of the other library, but I think it's nice to have CLJS support for this library nonetheless.

I would say let's merge, but this is @danielcompton's call.

[filipesilva]

Hey all, any chance of a release soon? Would love to use the cljs ordered map in a project I have right now.

[borkdude]

Pinging @slipset: not sure if this project has been set up for deployment like the others?
@filipesilva Is it possible to use this project as a git dep?

[filipesilva]

@borkdude I think so, would have to refactor this lein project to use deps edn or some lein plugin to do that. Hadn't really thought about that option.

[borkdude]

@filipesilva Released as 1.15.10

[filipesilva]

Thanks for letting me know!