Absolute paths in :output-dir break Node.js shim for :none

Question

When compiling a trivial [example|https://github.com/bensu/node-abs-ticket] with the following script:


(require 'cljs.build.api)

(cljs.build.api/build "src"
  {:main 'hello.core
   :output-to "main.js"
   :output-dir "/home/carlos/Playground/node-abs/out"
   :target :nodejs})


It generates code that tries to resolve the following path:


/home/carlos/Playground/node-abs/home/carlos/Playground/node-abs/out/goog/bootstrap/nodejs.js


We should check if the provided path for {{:output-dir}} is absolute before resolving it in the Node.js {{:none}} shim. The shim has a related ticket in CLJS-1444.

Even if it's uncommon for users to have absolute paths, tooling might need to.

Answer 1

Comment made by: bensu

The attach patch {{cljs_1466.patch}} solves the issue by using {{path.resolve}} which takes into account relative vs absolute paths when joining paths. Successfully tested in the example repo with both relative and absolute {{:output-dir}}

Answer 2

Comment made by: martinklepsch

Looking at the patch it seems that it might break current behaviour in some cases? Have you thought about that?

CLJS-1444 (link: 1) would probably also break the shim in some way so would be good to get these in together and be very clear about what will break etc. As long as we come up with a robust and predictable impl this is something worth breaking imo.

(link: 1) http://dev.clojure.org/jira/browse/CLJS-1444 for

Answer 3

Comment made by: dnolen

Yes would like to get feedback from people already heavily invested in ClojureScript + Node.js before moving forward on this.

Answer 4

Comment made by: bensu

(link: ~martinklepsch): I did think about breakage but I couldn't find any cases. Do you have an example one? In the (link: https://github.com/bensu/node-abs-ticket text: example repo) I've put together some tests (by running {{./script/test.sh}}) but it boils down to {{path.join(path.resolve("."),paths)}} being equivalent to {{path.resolve(paths)}} for all relative paths, since the "Resolve to absolute" method is the same for both ({{process.cwd()}} inside of {{path.resolve}}). When considering absolute paths, only the new version does the right thing.

On the other hand, those tests also reveal that the proposed patch doesn't cover CLJS-1446 as I originally thought since

node main.js

succeeds while:

cd .. node node-abs/main.js

fails.

Answer 5

Comment made by: mfikes

Patch no longer applies.

Answer 6

Reference: https://clojure.atlassian.net/browse/CLJS-1466 (reported by bensu)