Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix html with odd length lists #398

Merged
merged 3 commits into from
Feb 2, 2023
Merged

Fix html with odd length lists #398

merged 3 commits into from
Feb 2, 2023

Conversation

zampino
Copy link
Collaborator

@zampino zampino commented Feb 1, 2023

Fixes #395.

The issue is due to some weird behaviour of cljs.core/reduce against vectors containing lists with an odd number of items.

(defn collect-expandable-paths [state {:nextjournal/keys [value] :keys [path]}]
(let [state' (assoc-in state [:expanded-at path] false)]
(if (vector? value)
(reduce collect-expandable-paths state' value)
state')))

(viewer/collect-expandable-paths 
 {:expanded-at {}}
 {:path [],
  :nextjournal/value [:h1 "Ahoi" (list [:div "one"] [:div "two"] [:div "three"])],
  :nextjournal/viewer {:name :html, :render-fn identity, :hash "5dqwbRQVFnHkdooT8QV8nfPHBGVWtj"}})

Causing:

Uncaught Error: :div is not ISeqable
    at Object.cljs$core$seq [as seq] (core.cljs:1253:20)
    at Object.cljs$core$ITransientCollection$_conj_BANG_$arity$2 (core.cljs:7198:20)
    at cljs$core$_conj_BANG_ (core.cljs:803:17)
    at core.cljs:5686:36
    at core.cljs:5686:35
    at Object.cljs$core$IReduce$_reduce$arity$3 (core.cljs:5690:24)
    at Function.cljs$core$IFn$_invoke$arity$3 (core.cljs:2570:17)
    at Function.cljs$core$IFn$_invoke$arity$2 (core.cljs:5266:36)
    at Function.createAsIfByAssocComplexPath (core.cljs:7134:21)
    at Function.createAsIfByAssoc 

The error is actually visible in browser console and it's not reproducible on the JVM.

don't descend into collections of arbitrary values (not wrapped ones)
(let [state' (assoc-in state [:expanded-at path] false)]
(if (vector? value)
(if (and (vector? value) (not presented?))
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As an alternative, we might just check that all (or just the first of) items in value are wrapped ones.

@zampino zampino marked this pull request as ready for review February 1, 2023 12:47
@zampino
Copy link
Collaborator Author

zampino commented Feb 1, 2023

Narrowing down the above issue, it seems there's a discrepancy between clj/s in destructuring sequences.

The original issue is due to the following (reproduces in a cljs repl):

clj -M:cljs -m cljs.main -r
ClojureScript 1.11.60

cljs.user=> (let [{:keys [key]} (list [:key1 "one"] [:key2 "two"] [:key3 "three"])] key)
Execution error (Error) at (<cljs repl>:1).
:key3 is not ISeqable

cljs.user=> (js/console.log *e)

which produces the same stacktrace as the one above:

Error: :key3 is not ISeqable
    at Object.cljs$core$seq [as seq] (core.cljs:1253:20)
    at Object.cljs$core$ITransientCollection$_conj_BANG_$arity$2 (core.cljs:7198:20)
    at cljs$core$_conj_BANG_ (core.cljs:803:17)
    at core.cljs:5686:36
    at core.cljs:5686:35
    at Object.cljs$core$IReduce$_reduce$arity$3 (core.cljs:5690:24)
    at Function.cljs$core$IFn$_invoke$arity$3 (core.cljs:2570:17)
    at Function.cljs$core$IFn$_invoke$arity$2 (core.cljs:5266:36)
    at Function.createAsIfByAssocComplexPath (core.cljs:7134:21)
    at Function.createAsIfByAssoc (core.cljs:7126:40)

The caller of createAsIfByAssoc (not listed in the stack) should be seq-to-map-for-destructuring.

The same code just doesn't blow up in clojure:

clj -r
WARNING: Implicit use of clojure.main with options is deprecated, use -M
Clojure 1.11.1
user=> (let [{:keys [key]} (list [:key1 "one"] [:key2 "two"] [:key3 "three"])] key)
nil
user=>

@mk mk merged commit 3f742f4 into main Feb 2, 2023
@mk mk deleted the fix-395 branch February 2, 2023 10:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

odd number of nested divs renders a blank page
2 participants