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

FYI CljDoc build is broken b/c cannot find snabbdom #23

Open
holyjak opened this issue Nov 5, 2021 · 4 comments
Open

FYI CljDoc build is broken b/c cannot find snabbdom #23

holyjak opened this issue Nov 5, 2021 · 4 comments

Comments

@holyjak
Copy link
Contributor

holyjak commented Nov 5, 2021

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.)

@reinseth
Copy link

I believe that the shadow-cljs support got broken in 392d876 (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?

@cjohansen
Copy link
Owner

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

@reinseth
Copy link

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)

@cjohansen
Copy link
Owner

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.

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

No branches or pull requests

3 participants