crisptrutski / boot-cljs-test

Boot task to run ClojureScript tests.
53 stars 18 forks source link

Update from 0.2.1 to 0.3.0 :out-file is deprecated #54

Closed GiovanniFerrari closed 7 years ago

GiovanniFerrari commented 7 years ago

We have a problem switching from version "0.2.1" to version "0.3.0" of boot-cljs-test,

when we replace the option "--out-file" (allowed in "0.2.1" but deprecated in "0.3.0") with "--ids",

as suggested by the test-cljs task.

Old version of the code

New version of the code

when we run the code, the tests are not executed and we get the following error message:


Testing with Phantom:ClojureScript could not load :main, did you forget to specify :asset-path?

ReferenceError: Can't find variable: goog 
phantomjs://code/phantom4803593222426079124.js:81 in onError

How should we modify the "test-cljs" task options in order to obtain the behaviour we had with the "0.2.1" version of boot-cljs-test?

martinklepsch commented 7 years ago

Hey @GiovanniFerrari โ€” you could try setting :asset-path in your main.cljs.edn file's compiler options to look like this

{:require [,,,]
 :init-fns [,,,]
 :compiler-options {:asset-path "/js/main.out"}}

If that doesn't work on the spot you can look in the Devtools Network tab to see where your browser is trying to load the files from and try different settings for :asset-path. Also look up the docs for :asset-path to understand how it affects where files are loaded from.

Hope that helps ๐Ÿ‘

GiovanniFerrari commented 7 years ago

Hi Martin! Thank you for your answer! :) yes, this has helped. I think that the problem is that if I set ":asset-path 'js/main.out' " the html file is able to get the "js/main.js" file and all the goog variables presents in "js/main.out/goog/base.js". But phantomjs is not able to find the same goog variable and it give me the error above (maybe because it is looking for a "js/main.out" folder in "/js" (I guess)). If I use "main.out" as :asset-path, phantomjs find the goog variable but html can't. I resolved placing the html in the "/js" folder, so the ":asseth-path 'main.out' " version were good for both. I don't know if it is a good solution, or even if it is a solution, but for the moment it is working :thinking:... I wonder why with the old version there was no need for an .end file, while with the new version there is no solution without it :thinking:

crisptrutski commented 7 years ago

@GiovanniFerrari this looks like a pretty face-palmy bug on my side - asset-path is not being respected properly.

The point of breakage in your upgrade seems to be caused by the decision (led by the "main is a single segment namespace" warning?) to nest the javascript in a js folder. Moving the html inside js is basically equivalent to undoing that change - so undoing is probably simpler ๐Ÿ™‚

Another face-palm is that :update-fs? is in opposite-land, it's behaving the opposite way to its description.. and swapping that now will just spread more confusion.. So perhaps fuel for #53 and decomplecting "continue execution" from the main task.

Will publish a version that respects asset-path, and can silence that particular "single segment" too to avoid luring others into relative path rabbit holes ๐Ÿ™‚

GiovanniFerrari commented 7 years ago

Hi Cris!

Thank you for your reply! :) Yes, the source of the nesting is exactly the "main is a single segment namespace".

About the update-fs, ok, in fact in the old version 0.2.1 I settled the parameter to true.

Perfect, we will wait for the new version! ;)

crisptrutski commented 7 years ago

Managed to get a rough version working.. but it's quick hacky and want to refactor before merging. Will push out an alpha for you to try though :)

crisptrutski commented 7 years ago

Published 0.3.1-SNAPSHOT

GiovanniFerrari commented 7 years ago

Yes, it works! Thanks ;) There is still the reverse behaviour of "update-fs" (if I use false I shouldn't have any target directory, right? Now with false I have a target directory). I was wondering if it was possible to avoid to add the .edn file (every time I have to add new namespaces). Was it generated automatically with the old version (0.2.1) of the library? :thinking:

crisptrutski commented 7 years ago

Creating the .edn file is not necessary - it's just a convenient way to configure CLJS compiler options (eg. asset-path here), notably if you're running multiple suites with different options. In your case using :cljs-opts should suffice.

re: update-fs, despite the unfortunate regression I don't want to flip-flop yet another time (for new users or people who adapted to it once) - rather want to extract the pattern to a generic, ร  la carte task-wrapper.