Add configurable output minification#56
Conversation
|
Warning Review limit reached
More reviews will be available in 32 minutes and 32 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR introduces configurable HTML output minification for the YiiPress static site generator. It adds a new ChangesHTML Output Minification Feature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR introduces configurable HTML output minification across YiiPress’ build pipeline, controlled via a new minify site configuration option (defaulting to enabled). It adds a dedicated OutputMinifier implementation, wires it into template rendering and page writers, and updates docs/roadmap along with PHPUnit and phpbench coverage.
Changes:
- Add
SiteConfig::$minifyand parseminify: true|falsefrom site YAML (defaulttrue). - Minify generated HTML by default in
PageTemplateRenderer,EntryRenderer, and relevant page writers, while preserving bodies ofpre,textarea,script, andstyle. - Add unit tests for the minifier and a phpbench benchmark; document the new option and update the roadmap.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/Build/OutputMinifier.php |
Introduces the HTML minification utility used throughout the build. |
src/Build/PageTemplateRenderer.php |
Adds a minify toggle and applies output minification by default after rewriteHtml(). |
src/Build/EntryRenderer.php |
Applies minification to entry page output (including cached paths) based on SiteConfig::$minify. |
src/Build/RedirectPageWriter.php |
Minifies redirect HTML output when enabled (or when config is absent). |
src/Build/AuthorPageWriter.php |
Passes the site minify setting through to PageTemplateRenderer. |
src/Build/CollectionListingWriter.php |
Passes the site minify setting through to PageTemplateRenderer. |
src/Build/DateArchiveWriter.php |
Passes the site minify setting through to PageTemplateRenderer. |
src/Build/NotFoundPageWriter.php |
Passes the site minify setting through to PageTemplateRenderer. |
src/Build/TaxonomyPageWriter.php |
Passes the site minify setting through to PageTemplateRenderer. |
src/Content/Model/SiteConfig.php |
Adds public bool $minify = true to the site configuration model. |
src/Content/Parser/SiteConfigParser.php |
Parses minify from YAML with a default of true. |
tests/Unit/Build/OutputMinifierTest.php |
Adds unit tests for the new minifier behavior. |
tests/Unit/Build/PageTemplateRendererTest.php |
Verifies default minification and opt-out behavior in template rendering. |
tests/Unit/Build/EntryRendererTest.php |
Updates site-config construction helper to include the new minify argument. |
tests/Unit/Content/Parser/SiteConfigParserTest.php |
Asserts default minify behavior and verifies minify: false disables it. |
benchmarks/OutputMinifierBench.php |
Adds a phpbench benchmark for minification performance. |
docs/configuration.md |
Documents the minify option and its whitespace-preservation behavior. |
roadmap.md |
Marks configurable output minification as completed. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| $protectedParts = preg_split( | ||
| '~(<(?:pre|textarea|script|style)\b[^>]*>.*?</(?:pre|textarea|script|style)>)~is', | ||
| $html, |
| $tokens = preg_split('~(<[^>]+>)~', $html, -1, PREG_SPLIT_DELIM_CAPTURE); | ||
| if ($tokens === false) { |
| } | ||
| } |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
tests/Unit/Build/OutputMinifierTest.php (1)
32-64: ⚡ Quick winConsider adding test coverage for
textareaelement preservation.The
OutputMinifierprotects four element types (pre,textarea,script,style), but onlypre,script, andstyleare tested. Adding atextareacase would ensure complete coverage of the protected elements.🧪 Suggested additional assertion
public function testPreservesTextareaWhitespace(): void { $html = '<div><textarea> spaced content </textarea></div>'; assertSame( '<div><textarea> spaced content </textarea></div>', OutputMinifier::html($html), ); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/Unit/Build/OutputMinifierTest.php` around lines 32 - 64, Add a new unit test method in the OutputMinifierTest class to cover textarea preservation: create a test (e.g., testPreservesTextareaWhitespace) that feeds a small HTML string containing a <textarea> with leading/trailing spaces and newlines to OutputMinifier::html and assert that the output exactly equals the original (preserving whitespace inside the textarea); place the test alongside testPreservesWhitespaceSensitiveElementBodies so it exercises the same preservation logic for the textarea element.Source: Learnings
src/Build/OutputMinifier.php (1)
66-69: 💤 Low valueRegex tokenizer may mis-split tags containing
>in attributes.The pattern
<[^>]+>doesn't account for>appearing inside quoted attribute values (e.g.,<div onclick="if(x>y){}">). While rare in static site output, this could cause incorrect minification.A more robust pattern would skip quoted sections:
♻️ Suggested improvement
- $tokens = preg_split('~(<[^>]+>)~', $html, -1, PREG_SPLIT_DELIM_CAPTURE); + $tokens = preg_split('~(<[^>]*(?:"[^"]*"|\'[^\']*\')*[^>]*>)~', $html, -1, PREG_SPLIT_DELIM_CAPTURE);Alternatively, document this as a known limitation if attributes with
>are not expected in the generated output.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Build/OutputMinifier.php` around lines 66 - 69, The preg_split call that builds $tokens using the pattern '<[^>]+>' can mis-split when a '>' appears inside quoted attribute values; update the tokenizer in OutputMinifier.php (the preg_split(...) that assigns $tokens) to use a safer regex that ignores '>' inside single- or double-quoted attributes (e.g., a pattern that matches tags while skipping quoted strings) or, if you prefer not to change the regex, add a clear comment/documentation next to the preg_split to mark this limitation; ensure the change targets the preg_split call that creates $tokens so tags like <div onclick="if(x>y){}"> are not broken by the tokenizer.src/Build/AuthorPageWriter.php (1)
43-43: 💤 Low valueConsider extracting repeated PageTemplateRenderer instantiation.
The pattern
new PageTemplateRenderer($this->templateResolver, $siteConfig->theme, $this->assetManifest, $siteConfig->minify)is repeated three times (lines 43, 68, 92). While not critical, extracting this to a private helper method would reduce duplication.♻️ Optional refactor to reduce duplication
+ private function createRenderer(SiteConfig $siteConfig): PageTemplateRenderer + { + return new PageTemplateRenderer( + $this->templateResolver, + $siteConfig->theme, + $this->assetManifest, + $siteConfig->minify + ); + } + public function write( SiteConfig $siteConfig, array $authors, array $entriesByAuthor, array $collections, string $outputDir, ?Navigation $navigation = null, bool $noWrite = false, ): int { if ($authors === []) { return 0; } - $renderer = new PageTemplateRenderer($this->templateResolver, $siteConfig->theme, $this->assetManifest, $siteConfig->minify); + $renderer = $this->createRenderer($siteConfig);Apply similar changes at lines 68 and 92.
Also applies to: 68-68, 92-92
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Build/AuthorPageWriter.php` at line 43, Extract the repeated PageTemplateRenderer construction into a private helper on AuthorPageWriter (e.g., private function createPageTemplateRenderer()) that returns new PageTemplateRenderer($this->templateResolver, $siteConfig->theme, $this->assetManifest, $siteConfig->minify); then replace the three inline instantiations in methods that currently call new PageTemplateRenderer(...) (lines referenced in the review) with a call to $this->createPageTemplateRenderer() to remove duplication and keep behavior identical.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/Build/OutputMinifier.php`:
- Around line 24-29: The regex used in OutputMinifier.php when building
$protectedParts with preg_split can mis-pair opening/closing tags (e.g., a
literal string containing </pre> inside a <script>), so update the pattern to
capture the tag name for the opening element and use a backreference for the
closing tag (i.e., capture the element name and close with </\2>) and keep the
same flags (/is) so only matching pairs are protected; after changing the
pattern, adjust any downstream handling of the preg_split result to account for
the changed capture groups (the variable $protectedParts and any code that
iterates or filters it should be updated accordingly).
---
Nitpick comments:
In `@src/Build/AuthorPageWriter.php`:
- Line 43: Extract the repeated PageTemplateRenderer construction into a private
helper on AuthorPageWriter (e.g., private function createPageTemplateRenderer())
that returns new PageTemplateRenderer($this->templateResolver,
$siteConfig->theme, $this->assetManifest, $siteConfig->minify); then replace the
three inline instantiations in methods that currently call new
PageTemplateRenderer(...) (lines referenced in the review) with a call to
$this->createPageTemplateRenderer() to remove duplication and keep behavior
identical.
In `@src/Build/OutputMinifier.php`:
- Around line 66-69: The preg_split call that builds $tokens using the pattern
'<[^>]+>' can mis-split when a '>' appears inside quoted attribute values;
update the tokenizer in OutputMinifier.php (the preg_split(...) that assigns
$tokens) to use a safer regex that ignores '>' inside single- or double-quoted
attributes (e.g., a pattern that matches tags while skipping quoted strings) or,
if you prefer not to change the regex, add a clear comment/documentation next to
the preg_split to mark this limitation; ensure the change targets the preg_split
call that creates $tokens so tags like <div onclick="if(x>y){}"> are not broken
by the tokenizer.
In `@tests/Unit/Build/OutputMinifierTest.php`:
- Around line 32-64: Add a new unit test method in the OutputMinifierTest class
to cover textarea preservation: create a test (e.g.,
testPreservesTextareaWhitespace) that feeds a small HTML string containing a
<textarea> with leading/trailing spaces and newlines to OutputMinifier::html and
assert that the output exactly equals the original (preserving whitespace inside
the textarea); place the test alongside
testPreservesWhitespaceSensitiveElementBodies so it exercises the same
preservation logic for the textarea element.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: abdf98d8-70fe-416e-aff8-14cee42a9e8a
📒 Files selected for processing (18)
benchmarks/OutputMinifierBench.phpdocs/configuration.mdroadmap.mdsrc/Build/AuthorPageWriter.phpsrc/Build/CollectionListingWriter.phpsrc/Build/DateArchiveWriter.phpsrc/Build/EntryRenderer.phpsrc/Build/NotFoundPageWriter.phpsrc/Build/OutputMinifier.phpsrc/Build/PageTemplateRenderer.phpsrc/Build/RedirectPageWriter.phpsrc/Build/TaxonomyPageWriter.phpsrc/Content/Model/SiteConfig.phpsrc/Content/Parser/SiteConfigParser.phptests/Unit/Build/EntryRendererTest.phptests/Unit/Build/OutputMinifierTest.phptests/Unit/Build/PageTemplateRendererTest.phptests/Unit/Content/Parser/SiteConfigParserTest.php
Summary
minifysite config option, enabled by defaultpre,textarea,script, andstylecontentsTests
Summary by CodeRabbit
Release Notes
New Features
<pre>,<textarea>,<script>, and<style>elements.minifyconfiguration option allows disabling minification when needed.Documentation
minifysetting and behavior details.