Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Built-in minifying vs. supported minifier package extension #253

Open
M-PERSIC opened this issue Jul 31, 2023 · 2 comments
Open

Built-in minifying vs. supported minifier package extension #253

M-PERSIC opened this issue Jul 31, 2023 · 2 comments

Comments

@M-PERSIC
Copy link

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.

@M-PERSIC M-PERSIC changed the title Built-in minifying vs. supported minifier plugin Built-in minifying vs. supported minifier package extension Jul 31, 2023
@tlienart
Copy link
Owner

tlienart commented Aug 1, 2023

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
Copy link
Owner

tlienart commented Aug 6, 2023

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:

  1. add minify_jll to their project
  2. add using minify_jll in their utils.jl
  3. 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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants