dumdom icon indicating copy to clipboard operation
dumdom copied to clipboard

FYI CljDoc build is broken b/c cannot find snabbdom

Open holyjak opened this issue 2 years ago • 4 comments

CljDoc build is broken b/c it cannot find snabbdom, required in core.cljs - https://app.circleci.com/pipelines/github/cljdoc/builder/24512/workflows/d7e08546-09c4-4358-a63b-18afebf13fcb/jobs/40887

This obviously broke when the patched version of the lib was interned. I am familiar with shadow-cljs and not with how cljs itself works with such dependencies so I do not know how to fix it.

(I found this b/c I wanted to add a link to the cljdoc page to the readme, which is now pointless.)

holyjak avatar Nov 05 '21 09:11 holyjak

I believe that the shadow-cljs support got broken in https://github.com/cjohansen/dumdom/commit/392d8766777161e59f73d87b73abdac2dc301e0b (Temporarily bundle snabbdom with a fix for snabbdom/snabbdom#970).

Would it be possible to publish your patched version of snabbdom to cljsjs/clojars, @cjohansen?

reinseth avatar Nov 28 '21 06:11 reinseth

I wonder if maybe the bundled workaround isn't needed anymore? 🤔 I can try using dumdom with cljsjs/snabbdom at work tomorrow.

cjohansen avatar Nov 29 '21 20:11 cjohansen

Oh, have you solved the innerHTML-issue in cljs-land instead of waiting for Snabbdom to fix it? (I read the issue thread. Fascinating, but a bit discouraging)

reinseth avatar Nov 29 '21 21:11 reinseth

Discouraging to say the least. It made me realize that we will have to write the vdom stuff too at some point...

Anyway, I have a work-around that I think makes the patch superfluous, but I'm not entirely sure. dumdom now prevents snabbdom from ever reusing elements that have had, or is about to have innerHTML written by giving those elements a key that is a hash of the innerHTML 😇 dumdom now also generates comment-nodes in place of nils, which further stops snabbdom from trying to reuse elements that shouldn't be reused.

I'll give our app a spin with vanilla snabbdom to see if we can do without the patch. I don't think the snabbdom people are planning on fixing it.

cjohansen avatar Nov 29 '21 23:11 cjohansen