webjars / webjars-play

MIT License
80 stars 34 forks source link

refactor WebJarsUtil #80

Closed mberndt123 closed 6 years ago

mberndt123 commented 6 years ago

Hey @jamesward,

as discussed in #79, I have rewritten WebJarsUtil. It is now significantly shorter and more flexible as it doesn't conflate how to locate an asset with what to do with it once it's found. Rather than @Html(webJarUtils.script(webJarUtils.localOrCdnUrl(webJarUtils.fullPath("bar", "qux.js")))) you can now write @webJarUtils.fullPath("bar", "qux.js").script(). Note that not only is this much easier but also that the @Html(…) stuff goes away.

Some things are a little longer now, for instance what used to be @Html(webJarsUtil.script("foo", "bar.js") is now @webJarsUtil.locate("foo", "bar.js").script() (4 characters longer), but I think the added flexibility is worth it. To make things even more terse, we could rename one of WebJarsUtil's methods to apply. I would suggest fullPath as I think it's probably the most robust way to locate a Webjar asset, it doesn't break as easily as locate when you e. g. upgrade a library.

If additional attributes are specified, they are now escaped correctly, making the library safer to use.

If a webjar is not found in dev/test mode, the library will now just blow up rather than generate a comment, making errors harder to miss.

Take a look and let me know what you think.

jamesward commented 6 years ago

Thanks! This looks really great.

One last request, can you update the code in the test-project to take advantage of these changes?

mberndt123 commented 6 years ago

done

mberndt123 commented 6 years ago

Cool, thanks for merging!