boot-clj / boot-cljs

Boot task to compile ClojureScript programs.
Eclipse Public License 1.0
176 stars 40 forks source link

Default main.cljs.edn generation should be customizable from other tasks #129

Open arichiardi opened 8 years ago

arichiardi commented 8 years ago

The need came out from a boot-cljs-devtools issue.

The scenario: the user does not specify cljs.edn files explicitly. In general a task before cljs might need to add a :require or :init-fns clause to the default main.cljs.edn generated by cljs.

At the moment there is no way to do that and cljs-devtools blindly ignores the fact that we have no .cljs.edn skipping adding stuff to it. Of course, changing the order works (e.g.: (boot ... (cljs) (cljs-devtools), but in general my idea is to have ways to add stuff to :require (or :init-fns or ...) from tasks before cljs.

The idiomatic way is to add metadata to files in the fileset but I wanted to ask here what we should mark and of course if you folks consider this a good idea or total garbage :smile:

Thanks!

Deraen commented 8 years ago

Idiomatic solution is that the user creates the .cljs.edn file when using boot-cljs-devtools or other tasks that modify the file.

I don't think having the options in another place (metadata) in addition to the file is a good idea.

arichiardi commented 8 years ago

isn't that optional? I have a couple of projects without the file and they work fine...

arichiardi commented 8 years ago

I mean, the user might not know that boot-cljs-devtools needs the file...I agree it can be made clear in the README though...

Deraen commented 8 years ago

I do highly recommend creating the .cljs.edn file and setting :require. Without the file boot-cljs naively requires all cljs files is fileset which might cause unused files being included in compilation (slower compilation, can cause surprising side-effects when from required namespaces).

Boot-reload and boot-cljs-repl seem to work though they are before cljs task on the pipeline? Boot-cljs-devtools should be able to work similarly to them.

arichiardi commented 8 years ago

Ok so I will ask the maintainer (or do it myself) to make clear that you need an explicit file. Thanks for the clarification, maybe we can make it clear in the README or the wiki for boot-cljs as well?

Deraen commented 8 years ago

Ah, now I remember how boot-reload and boot-cljs-repl work.

Boot-reload and boot-cljs-repl similarly search the fileset for existing .cljs.edn and they don't find the file either so they can't add the :require to cljs.edn. The reason why they work but boot-cljs-devtools doesn't, is that they don't use :init-fns.

The generated .cljs.edn file will by default :require all cljs files in fileset and that will include files created by boot-reload and boot-cljs-repl. Because boot-cljs-devtools doesn't create a file in fileset but directly requires devtools namespaces and calls devtools install using init-fns, it doesn't work with the generated cljs.edn.

This is something we could improve in future, but I don't think metadata is a solution.

For now I recommend that boot-cljs-devtools creates a cljs file which requires devtools and calls the install function: https://github.com/adzerk-oss/boot-reload/blob/master/src/adzerk/boot_reload.clj#L34-L44 This file will be picked up by default .cljs.edn.

arichiardi commented 8 years ago

Yeah that's exactly the reason and the reason why I was thinking myself of a more generic method...I am PR-ing the boot-cljs-devtools README..

Deraen commented 8 years ago

Also, the docs could definitely use improvements regarding cljs.edn file. Looks like it is still documented only under "Multiple builds", which is quite confusing.

arichiardi commented 8 years ago

I would make it a first class citizen, promoted to the main README and in Usage as well. It is kind of important :wink:

Deraen commented 7 years ago

I guess this is pretty much the same as https://github.com/boot-clj/boot-cljs/issues/169