Avoid throwing when reloading foreign Javascript in boot

[arichiardi]

The patch avoids a "is not a relative path" exception, in javascript-reloading/cljs-target-file-from-foreign.

Fixes #536 and https://github.com/boot-clj/boot-figreload/issues/4

[bhauman]

First of all I am not sure what problem you are trying to solve here, this function is always passed an absolute path. And the intention of the function is to return the path of the compiled output file not simply return the file that was passed to it. It is intended to take a full path of a source file and return the output path. I'm thinking you are running into a different problem.

[arichiardi]

Mmm thanks for your very good input Bruce, I tried again.

Basically relativize-local does not really return a relative path when the path is not relative to the local project. In Boot's case, this is always true.

I am thinking now about how to handle this, I will get back with another patch or just close this one. Thanks and sorry for the noise.

[bhauman]

So relativize-local is probably failing for boot because it is making it relative to the current directory?

[arichiardi]

Yep that is what is happening, what do you think about supporting this case? I think I can do very little on my side because of the .exists check there. Probably relativize-local needs to return nil when it detects a non-local path and then in the filter we can branch and put directly the path passed in input.

[bhauman]

You may want to write a couple of tests to see if you can write a function that given the path of the changed js file you can get the path of the compiled output file. And we will start from there.

[arichiardi]

Checking this again, basically in boot it is easy because the full path to the changed js file is always passed in.

[arichiardi]

@bhauman I finally had time to implement this (with tests, as you suggested). It works on my computer, as they always say :wink:. Please let me know if anything needs change or better testing.

[arichiardi]

@danielcompton I removed my custom path from the tests, it was misleading. I have not yet tried on Windows but I will. Note that figwheel-sidecar.build-middleware.javascript-reloading/file-locations does not have any file system side effect (.exists) so in theory it is pure string transformation using java's Path and File.

@bhauman this is still an open issue that I would like to address, could you please have a look and give me some feedback?

[arichiardi]

Revised the patch and improved tests so that are built using java.nio.file.Paths (should run on windows as well now).

[kommen]

I'm just running our project with this patch and live reloading in the browser with boot-figreload now works as expected, fixing https://github.com/boot-clj/boot-figreload/issues/4 and https://github.com/boot-clj/boot-figreload/issues/5

[arichiardi]

It looks like there are still some cases when this fix fails, especially when applying this to node (/cc @kommen).

My fix was more done in order to avoid for relativize-local to create wrong paths. This always happens when for instance the input is:

/home/arichiardi/.boot/cache/tmp/home/arichiardi/git/botkit-starter-slack/bs3/-hmx26q/app.out/cljs_deps.js

I haven't investigated deeper in how figwheel passes things from the caller. It might be worth maybe adding a little explanation here if you can @bhauman. Someone else can also pick it up (if I have not time) :smile:

[arichiardi]

Closing this one, I worked around it. Hopefully solidly enough :smile: