lilactown / helix

A simple, easy to use library for React development in ClojureScript.
Eclipse Public License 2.0
624 stars 52 forks source link

Running tests in nodejs with jsdom #136

Closed vloth closed 1 year ago

vloth commented 1 year ago

Is it possible to run helix with jsdom?

It seems like helix requires window to be defined at load time, and shadow-clj doesn't support user defined javascript while bundling. Running tests gives me:

SHADOW import error helix-jsdom/.shadow-cljs/builds/node-tests/dev/out/cljs-runtime/main.app.js

/helix-jsdom/.shadow-cljs/builds/node-tests/dev/out/cljs-runtime/helix/core.cljs:237
  (and (exists? (.-$$Signature$$ js/window))
       ^
ReferenceError: window is not defined
    at Object.helix$core$signature_BANG_ [as signature_BANG_] 

I created a minimal reproducible project here. If that's not supported, is that something you are planning to support in future? (I don't know if that's relevant, but I did manage to make it work with reagent). Is there anything I could do to help?

Thanks for creating and sharing helix!

rome-user commented 1 year ago

I looked into this. Good news for you: this seems to be fixable with a small patch. The exists? macro seems to emit the improper JavaScript for the use case of @lilactown. Here is REPL demonstration:

cljs.user> (str (fn [] (exists? (.-$$Register$$ js/window))))
"function (){\nreturn (!((window.$$Register$$ == null)));\n}"
cljs.user> (str (fn [] (exists? js/window.$$Register$$)))
"function (){\nreturn (typeof window !== 'undefined') && (typeof window.$$Register$$ !== 'undefined');\n}"

I will wait for his input before making a patch

lilactown commented 1 year ago

yeah that sounds like a good change. I've been bit by this bug before, would certainly accept a patch!

rome-user commented 1 year ago

@vloth I can't tell if your issue is fixed. I am using a modified copy of Helix, but your example project uses js/document, which is not available on Node. I tried using goog.dom/getElement with little success.

The good news is that importing Helix no longer causes errors. So I will make a PR.

vloth commented 1 year ago

@rome-user thanks for investigating and submitting the patch!

I had to fix the jsdom setup a little bit and from the tests I did, it seems to be working with the suggested fix! To fix the setup, I extracted the component under test into another file (that's the version in the main branch). I also pushed another version to document-load-free branch which just defers the document use and it's working well too.

Looking forward to see the patch applied, thank you again!