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

WIP: add html minification #4

Closed
wants to merge 1 commit into from

Conversation

ryancdotorg
Copy link
Contributor

Slightly reworked from some changes I'm using locally. Pelican's execution order for plugins that have hooked a given signal is non-deterministic - previously this was seemingly only in theory, but in current versions it's become a problem in practice.

This patch adds optional HTML minification in addition to precompression. It's turned off by default to avoid surprises to existing users of the plugin.

Parallelism is preserved as much as possible.

TODO: Test coverage - I'm not much good at writing tests. I added a few, but the coverage is still a bit lacking.

@ryancdotorg
Copy link
Contributor Author

Kurt,

Apologies if you're busy, but I wanted to check whether this is something you're potentially interested in merging before spending more time on it. It's definitely a bit of scope creep, but the way Pelican handles plugins makes it hard to avoid.

-Ryan

@kurtmckee
Copy link
Owner

Ryan,

I love this idea. However, I don't want to include this functionality in pelican_precompress.

It may be possible to build off of the existing webassets plugin. In what ways does the Pelican plugin system make it difficult to avoid? Maybe we can work on a minification plugin together if webassets can't be configured well with pelican_precompress!

@ryancdotorg
Copy link
Contributor Author

Pelican cannot run plugins in any particular order on any given signal, so a separate plugin can't work currently. It had previously worked fine despite this (I used pelican_htmlmin), but in recent releases I've been seeing issues with things running out of order.

I don't believe webassets is able to minify pelican's html output.

I think it'd be possible to make a wrapper plugin that does some monkey patching to wrap other plugins and force them to run in a specific order, but that'd result in slightly less parallelism (all html minification would run, then compression would run).

It would also be possible to make a plugin that lets you register multiple processing routines for files and understands dependencies.

Pelican could also add some additional signals, but that's pretty hacky, and feels like the wrong solution.

The best solution would be for Pelican to add first class support for ordering plugins, but it appears that'd require major changes to Pelican's plugin code.

@kurtmckee
Copy link
Owner

kurtmckee commented Apr 15, 2021

Hey Ryan, would you take a look at the latest code in develop? I've re-written the register() function so that it wraps the finalized.send() method instead of registering as a signal receiver. This guarantees that compression happens after everything else, like sitemap generation and minification.

I don't want to add minification but I do want precompression to occur after that, so if this works for you then I'll release a new version and close this merge request.

Note that I recently updated the plugin to support Pelican 4.5's new namespace plugin architecture. To do this, the code had to change locations and is now referenced in the PLUGINS list as "pelican.plugins.precompress". (The big benefit to the new architecture is that if all installed plugins use this architecture then you don't have to maintain a PLUGINS list anymore, so pelican_precompress can require even less setup.)

@ryancdotorg
Copy link
Contributor Author

Oh, that's a very clever technique. I did post a forked repo with the minification code integrated, but separate plugins is definitely better. I'll take a look at this when I have a chance (currently trying to prep for a con talk on Saturday).

@kurtmckee
Copy link
Owner

trying to prep for a con talk on Saturday

No rush on testing. Good luck with the talk!

@kurtmckee
Copy link
Owner

@ryancdotorg, I hope your talk went great!

Please let me know if the recent changes ensure that precompression occurs after minification now. If so, I'll release a new version to PyPI.

@kurtmckee
Copy link
Owner

@ryancdotorg, I hope your talk went well! If you have an opportunity to test the code in develop, please let me know if it resolves the issue you're seeing!

@kurtmckee
Copy link
Owner

Hey Ryan! I've released pelican-precompress 2.1.0 so that it will run at the correct time. I generalized the blinker signal code into another pelican plugin called pelican-granular-signals, so that other plugins can rely on the same code. The finalized signal will be sent first so existing plugins like the minifier will run first, then the compression stage will run.

Please try updating to pelican-precompress 2.1.0 and let me know if you encounter any issues. Thanks!

@kurtmckee kurtmckee closed this Aug 9, 2021
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

Successfully merging this pull request may close these issues.

2 participants