Built-in minifying vs. supported minifier package extension

M-PERSIC commented 1 year ago

I had originally submitted a PR request to the original Franklin.jl repo (Replace html-css-js-minify with minify_jll, eliminate Python3 dependency) for eliminating Python3 as an external dependency in lieu of using the recently ported minify_jll package for Taco de Wolff's Minify tool written in Go. I'd be more than happy to include it here so it can be ready for the next big Franklin release!

Advantages

Unlike with html-css-js-minify, the minifier tool is directly packaged as a JLL so we can eliminate Python3 as an external dependency required for this functionality
Additional support for json, svg, and xml minification

Current Ideas

minify_jll as an included dependency

I advocate for this option as the advantages of minimizing code for the web are numerous (reduce page load, increase performance,...) and should be an enabled option by default. The package is loaded transparently to the user since it would be bundled with Xranklin and not exposed, thus could be more easily changed in the future if warranted. My current thinking is to integrate the minification step into the full_pass function which already tracks files.

minify_jll in a package extension

A second option would be to integrate minify_jll as a weak dependency that is manually loaded when the user wishes to enable minification. It could be as simple as redefining the full_pass function like so:

# in src/build/full_pass.jl
function full_pass(watched_files; <options>) ... end

# in ext/MinifyExt.jl
using minify_jll
function full_pass(watched_files; minify::Bool = true, <options>)
full_pass(watched_files; <options>)
if minify
    minify(watched_files) 
end
end
function minify(watched_files) ... end

If we can discuss it further and head towards a consensus, I can see about submitting a draft PR by the end of the week barring exceptional circumstances.

tlienart commented 1 year ago

Thanks a lot! I need to think a bit about your suggestions; it'd be great to have users able to leverage minify_jll in any case!

tlienart commented 1 year ago

sorry I haven't replied here in a while. I'm supportive of this as long as it's opt-in.

I suggest doing it the following way so that this JLL does not unnecessarily become a dependency of Xranklin:

add an arg to serve where a symbol can be passed of a finalizer function that is defined in utils.jl (serve(...; finalizer=:my_finalizer); this will, in the final pass, attempt to run Utils.my_finalizer(gc) (where gc is the global context object).

the process for a user to use it would then be:

add minify_jll to their project
add using minify_jll in their utils.jl
define a function my_finalizer(gc) ... end in utils.jl which calls minify_jll (and whatever else they might want); copying what you suggested here: https://github.com/tlienart/Franklin.jl/pull/1038/files#diff-82304366621d2dd806325b767e09d5eb29180c0c7ff0fbdb607e8fda487d2df9R101-R116

it might not be as ergonomic as you might like but the advantage is that it is not limited to minifying (e.g. the user might decide to add a call to whatever image crushing program they might have on their machine or god knows what else they might want to do prior to deployment).

Eventually, I'd like for such things to be small plugin packages that a user could call from their utils.jl so that they wouldn't have to maintain the inner code (it also means less code surface to maintain on the Franklin side).

what do you think?

tlienart / Xranklin.jl