boot-clj / boot-cljs

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

Support Compojure hosted assets with --unified #4

Closed martinklepsch closed 9 years ago

martinklepsch commented 9 years ago

Follow-up on some discussion in #hoplon yesterday night.

Currently when using --unified that will recognize the path to main.js and load additional files based on this information. This is useful behaviour when using a Figwheel-like workflow, as only small portions of the code will be reloaded. When you serve assets as a route in Compojure though ((resources "/assets" {:root "public"})) the modification of the paths stops working as the system can't correctly identify the main.js file anymore.

I think it would be useful to support that usecase by allowing an optional argument to --unified. Perhaps a simple vector creating a mapping between a route and a directory within the target dir could work: ["/assets" "public"]. This way when boot-clis would encounter a <script> starting with /assets i.e. /assets/js/main.js it could determine that this references public/js/main.js in the target directory. With this knowledge all other paths could be determined.

This would also have to work with index.html being in public as well and only referencing js/main.js but I assume that's doable.

Keep in mind that this optional argument would be fully backwards compatible and solve a very real usecase for people serving their assets through Compojure in development.

If that would be a feature you'd be interested in I'll try to come up with a pull request.

In the future maybe it'd make sense to move some of that html-munging stuff into separate tasks. That way you could also create tasks that change the paths from your local assets to CDN hosted ones.

micha commented 9 years ago

Why can't you emit the JS file to the location where you want it to be served? Eg:

boot cljs -u -o assets/js/main.js
martinklepsch commented 9 years ago

Huh. o.O That's actually a good question. Not sure what I was thinking right now. Very sorry for the confusion! :disappointed:

One issue though in that context: lets say I render my index.html on an entirely different path, i.e. /another/page the path that is generated to Closure's base.js is http://0.0.0.0:3000/another/assets/js/out/goog/base.js which won't match the URL structure. The script tag looks like this:

<script type="text/javascript" src="../assets/js/out/goog/base.js">

Maybe this issue can be fixed without introducing anything new though.

micha commented 9 years ago

I guess my real question was why render these pages from URLs that are not expressed in the project structure already? Like can't the HTML file just be in the right place in the project to be served without special case configuration?

martinklepsch commented 9 years ago

In JS heavy apps you might just use one index.html for all incoming links and some JS on the page would handle the routing. Similarly when using pushState relative paths will break. Being able to absolutely specify the path to all assets would be required in this case.

micha commented 9 years ago

There are a lot of permutations that are theoretically possible. People have historically not been super great at figuring out how to organize web apps. Since there hasn't traditionally been any model at all, the developer was totally free to organize their stuff so that it's a tangled mass of unnecessary coupling and arbitrary configuration. This is, really, the norm even today. So while people might do a lot of crazy things, these things might not really make sense, and we don't necessarily want to support them in the cljs task itself, or it'll end up as a project.clj. Since there is no model yet, we're making the model.

Our model is application-oriented. The HTML is the artifact the browser installs and runs. This executable uses libraries (the JS). Like any executable, linking is done by the compiler, which is what the cljs task is doing. Support for dynamic linking is provided by the browser (script loading, JSONP, etc). This is all very standard and well-trodden territory for every other kind of application in the universe, and it's worthwhile to use this model for web apps, too.

Things that don't fit into this model:

In the above cases I think it makes sense to do the unification in your handler or as middleware, rather than hardcoding in lots of arbitrary configuration keys in the cljs task, because then the complexities of the serverside setup don't need to be coupled to the rest of the build.

Does that sound like a relatively sane arrangement?

martinklepsch commented 9 years ago

This sounds sane. If I understood boot correctly the proper changing of src attributes could also be done by another task, as it as access to information about other tasks etc.

I'll see if I can write a task that basically looks for html files and converts relative links to absolute ones.

Deraen commented 9 years ago

If index.html is at "public/index.html" and cljs :output-to is set to public/main.js this happens:

boot.user=> (f/up-parents (io/file "public/index.html") "" (io/file "public") "out" "goog" "base.js")
"../public/out/goog/base.js

Should be fixable if paths are checked for shared prefix or something.

micha commented 9 years ago

@Deraen I can't reproduce this. I think you have a typo in your expression you ran in the REPL:

;; you had an extra "" in there
boot.user=> (f/up-parents (io/file "public/index.html") (io/file "public") "out" "goog" "base.js")
"out/goog/base.js"
martinklepsch commented 9 years ago

There was an additional string between (io/file "public/index.html")and (io/file "public"):

(f/up-parents (io/file "public/index.html") "" (io/file "public") "out" "goog" "base.js")

Maybe that's why you got another path.

Deraen commented 9 years ago

@micha It not a typo: https://github.com/adzerk/boot-cljs/blob/master/src/adzerk/boot_cljs/impl.clj#L30 empty string is there.

micha commented 9 years ago

@Deraen whoa, that's probably the bug

martinklepsch commented 9 years ago

Haha, I just removed that extra string which resulted in

src="../../../../../../../out/goog/base.js"

So I guess it's there for a reason at least.