Fork me on GitHub

I'm looking into this and it looks like the error is caused because cljs.closure/compile-file receives a File as output-file, instead of a relative path output-file is #object[ 0xfabe3f8 /tmp/out3012973275628785939777962205402581/cljs/user/user358016D.js] instead of a string like clojure/browser/repl/preload.js. Where I'm stuck is, should compile-file be able to handle an output-file that is a File (and then use that instead of creating a new File) or has something gone wrong further back and it should have been passed the string cljs/user/user358016D.js instead?


@regen Yeah, that's a good question. I wouldn't go so far as to conclude that it need to be a relative string, as a relative File would also work. To be honest, I think you could realistically debate whether the fix should take the form of making cljs.repl/load-file pass a relative path, or making compile-file be more forgiving, accepting absolute paths (perhaps checking that they are in the output dir as a sanity check).


Not exactly the same problem, but I had solved a similar issue downstream in this ticket


Thanks for your thoughts, @mfikes. I went a bit back and forth, wrote a fix that passed it as a relative path string, but then I found out that cljs.repl.rhino, nashorn, node and graaljs all do the same thing: using cljs.closure/src-file->target-file for output-file to compile-file. (I guess that means that these other repl's would fail the same way if used with a temp out dir?) With this and the issue you posted in mind, it seems to me that making compile-file more forgiving is the solution.

👍 4

should it be considered a bug or a feature that the CLJS ns parser seems to accept this?


all usages I ever saw followed :include-macros with true? is my understanding just wrong?


Yeah, true should go there


I’m AFK, but it appears that the ns spec would catch this, as it has

(s/def ::include-macros #{true})


seems like it doesn't since people use that library and it has been that way since Jan 2017


In hindsight, that spec could be simplified to use true?


Yeah, I’d previously discovered you could put any truthy value there, but perhaps it doesn’t even care if a value is there


I use a custom ns parser based on spec in shadow-cljs which doesn't accept this


not sure if I should adjust the parser since that would make it an "official" feature


but it looks like a bug to me


(doc ns) specifies :include-macros takes true


My take is that the posh code is invalid. The ns form parser could always catch more corner cases, but perhaps enabling cljs.core.specs.alpha is a better long-term solution (especially given we will also soon have a compiler option to disable macro spec checking — which may also apply to the hack we are doing for the ns special form)