day8 / re-frame-10x

A debugging dashboard for re-frame. X-ray vision as tooling.
MIT License
631 stars 68 forks source link

[Bug]: Sorting an app-db view with non-comparable keys crashes re-frame-10x #406

Closed p-himik closed 8 months ago

p-himik commented 8 months ago

What happened?

Steps to reproduce:

  1. (swap! re-frame.db/app-db assoc :x 1 "x" 2)
  2. Open re-frame-10x -> "app-db" -> "+ path inspector"
  3. Do something that emits an event so that the app-db panel is populated
  4. Expand the root path inspector and check "sort"

The root cause is that day8.re-frame-10x.tools.datafy/deep-sorted-map uses sorted-map. A potential solution is to use sorted-map-by and pass to it a comparator that compares not only based on values but also based on types. It won't handle values that are of the same type but can't be compared - perhaps that case should be caught and reported in a more friendly way suggesting the user to change the data types or avoid sorting them.

Factory Reset

10x Version

1.8.1

Reagent Version

1.2.0

React Version

17.0.2

re-frame Version

1.3.0

What browsers are you seeing the problem on?

No response

Relevant console output

react-dom.development.js:20086 The above error occurred in the <day8.re_frame_10x.components.cljs_devtools.simple_render_with_path_annotations> component
react-dom.development.js:11341 Uncaught Error: Cannot compare x to :workspace
    at cljs$core$compare (core.cljs:2433:13)
    at eval (core.cljs:8752:14)
    at Object.cljs$core$tree_map_add [as tree_map_add] (core.cljs:8752:13)
    at Object.eval [as cljs$core$IAssociative$_assoc$arity$3] (core.cljs:8959:18)
    at Object.eval [as cljs$core$ICollection$_conj$arity$2] (core.cljs:8901:15)
    at cljs$core$_conj (core.cljs:598:16)
    at eval (core.cljs:2532:22)
    at Function.eval [as cljs$core$IFn$_invoke$arity$3] (core.cljs:2532:21)
    at Function.eval [as cljs$core$IFn$_invoke$arity$3] (core.cljs:2582:9)
    at Function.eval [as cljs$core$IFn$_invoke$arity$2] (core.cljs:5267:11)

When expanding the above:

eval    @   cljs_devtools.cljs:404
eval    @   cljs_devtools.cljs:408
day8$re_frame_10x$components$cljs_devtools$simple_render_with_path_annotations  @   cljs_devtools.cljs:394
[React stack trace garbage removed]
kimo-k commented 8 months ago

Hey, thanks for the report.

Repro is good. I'm going to forgo the custom comparator, since we can't handle every case.

Now when a subtree is unsortable, it will gracefully fail, and the rest of the tree will still sort.

In the rare case a user needs to inspect an unsortable map as a sorted map, they can figure out how to sort it themselves IMO.

p-himik commented 8 months ago

I'm going to forgo the custom comparator, since we can't handle every case.

@kimo-k If you follow this guide, you'll handle the vast majority of all the cases: https://clojure.org/guides/comparators#_comparators_that_work_between_different_types

kimo-k commented 8 months ago

Looks like there's a library mentioned in that article, so I think we can support this.

Not sure if we still need error handling, then. I took it out for now.