lambdaisland / kaocha-cljs

ClojureScript support for Kaocha
Eclipse Public License 1.0
40 stars 10 forks source link

Cannot execute tests when code requires JS namespaces (foreign-libs) #16

Closed countgizmo closed 5 years ago

countgizmo commented 5 years ago

I am currently seeing two effects of the foreign-libs issue:

  1. Some JS libraries (fore example React) do not cause any issues in compilation phase, but then cannot be used when required into a CLJS namespace (the JS object is undefined).
  2. Other JS libraries (for example Sentry) fail at compilation phase with "No such namespace..." error.

Why I think it's kaocha-cljs related: both of the issues described above do not happen during CLI compilation using cljs.main or figwheel.main.

Details:

I have created a small github repo that reproduces both issues: https://github.com/countgizmo/hello-webpack

The README has all the commands to build and run the project and run the tests.

Steps to reproduce the issues described in this ticket:

image image

Here's my tests.edn file for quick reference:

#kaocha/v1
{:tests           [{:id                    :unit-cljs
                    :type                  :kaocha.type/cljs
                    :test-paths            ["test/cljs"]
                    :source-paths          ["src/cljs"]
                    :cljs/repl-env         cljs.repl.browser/repl-env
                    :cljs/timeout          60000
                    :cljs/compiler-options {:npm-deps     false
                                            :verbose      true
                                            :foreign-libs [{:file           "dist/index_bundle.js"
                                                            :provides       ["react" "sentry"]
                                                            :global-exports {react React
                                                                             sentry Sentry}}]
                                            }}]
 :reporter        [kaocha.report/debug]
 :bindings        {kaocha.type.cljs/*debug* true}
 :capture-output? false}
countgizmo commented 5 years ago

Looking at the compiled JS for the core.cljs (during kaocha-cljs test run) file there's something that stands out:

...
goog.require('react');
...
hello_webpack.core.global$module$react = goog.global["React"];
...
console.log(hello_webpack.core.node$module$react.Component);
...

The react library is correctly assigned to the global module called react. But the actual usage of react namespace compiles into node$module instead of global$module, which is I'm guessing the reason for the JS namespace not being usable.

If I look at the JS object in the Chrome Dev Console, I can see that the object actually has both react and sentry namespaces in it: image

For comparison, here's the same code compiled using cljs.main:

...
goog.require('react');
...
hello_webpack.core.global$module$react = goog.global["React"];
...
console.log(hello_webpack.core.global$module$react.Component);
...

Unfortunately I don't have enough knowledge on Clojurescript compilation to understand why it decides to build .node$module reference instead of .global$module during code compilation.

countgizmo commented 5 years ago

It seems like, even though I pass :npm-deps false in :cljs/compiler-options the node_modules directory still gets indexed and .node$module reference is generated. I was able to reproduce it in the cljs.main build by setting :npm-deps true - the situation is then exactly the same as kaocha-cljs run with :npm-deps false.

The question now is: why :npm-deps false is not being respected during kaocha-cljs test run...

plexus commented 5 years ago

Would this have something to do with the fact that kaocha-cljs itself requires certain npm packages to be present? (ws and isomorphic-ws)?

countgizmo commented 5 years ago

@plexus Hard to tell: is there an "easy" way to disable npm packages requirement in kaocha-cljs? I can have kaocha-cljs setup as a local dependency and comment out some code as a quick test, but I need help navigating the kaocha's code :)

countgizmo commented 5 years ago

I wanted to check what clojurescript "sees" when it's time to decide wether to index the node modules or not. I've cloned the repo and added some printlns and this is what I've gotten (formatting is mine):

{:output-dir .repl-0.0.146424480
:closure-warnings {:check-types :off, :check-variables :off}
:closure-defines {process.env.NODE_ENV production}
:ups-libs nil
:closure-module-roots []
:optimizations :simple
:ups-foreign-libs []
:aot-cache false
:preloads [process.env]
:ignore-js-module-exts [.css]
:preamble [cljs/imul.js]
:ups-externs nil
:opts-cache cljsc_opts.edn
:cache-analysis-format :transit
:emit-constants nil}

As you can see even though I have specified :deps false in my tests.edn file there's no :deps option in reaching that part of clojurescript.

And since the :deps otpion is nil the next code executes:

(if (not (false? npm-deps))
                      (index-node-modules-dir))
countgizmo commented 5 years ago

Also I've noticed that :foreign-libs option is passed as [] when running kaocha-cljs test event though just like :npm option they are also specified.

I did the same test for cljs.main + cloned clojurescript + printlns and I can see the correct options are being passed through.

countgizmo commented 5 years ago

Sorry @plexus - I've added my last comment at the same time as you've referenced the fix for the whole thing! :) The fix makes sense. Thank you!