Conversation
…event handling to support TYPO3 14
…nnecessary whitespaces after opening and before closing tags
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughThis PR upgrades the extension to support TYPO3 14 and PHP 8.5, modernizing event listeners with constructor injection and new component APIs, enhancing HTML minification with dedicated helper transformations, and updating dependency constraints and test tooling configuration. ChangesFramework Upgrade to TYPO3 14 & PHP 8.5
Event Listener & Asset Rendering Modernization
Content Minification Enhancement
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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. Review rate limit: 0/1 reviews remaining, refill in 2 minutes and 41 seconds.Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Classes/EventListener/LastDeploymentEventListener.php (1)
23-26:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPre-existing logic bug prevents this feature from working.
Line 24 has an inverted condition that causes the function to return early whenever the path is not empty:
if ($path !== '' || !file_exists($path)) { return; }This should check if the path IS empty or doesn't exist:
🐛 Proposed fix
- if ($path !== '' || !file_exists($path)) { + if ($path === '' || !file_exists($path)) { return; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Classes/EventListener/LastDeploymentEventListener.php` around lines 23 - 26, In LastDeploymentEventListener, the early-return condition is inverted: change the check on $path in the method where $path is set from GeneralUtility::getFileAbsFileName so that it returns only when the path IS empty or the file does not exist (i.e. use if ($path === '' || !file_exists($path)) return; or equivalent empty() logic) instead of the current ($path !== '' || !file_exists($path)); update the condition in that function to the corrected form to allow execution when a valid path exists.
🧹 Nitpick comments (1)
Classes/EventListener/ContentMinifierEventListener.php (1)
100-105: 💤 Low valueConsider adding
JSON_THROW_ON_ERRORtojson_decodefor consistent error handling.Currently
json_decodedoesn't useJSON_THROW_ON_ERROR, so invalid JSON returnsnullsilently, which is then caught by the'null' === $minifiedJsoncheck at line 107. For consistency with the try/catch pattern and clearer intent:♻️ Proposed refactor
try { - $minifiedJson = json_encode(json_decode($json, true), JSON_THROW_ON_ERROR | JSON_UNESCAPED_SLASHES); + $decoded = json_decode($json, true, 512, JSON_THROW_ON_ERROR); + $minifiedJson = json_encode($decoded, JSON_THROW_ON_ERROR | JSON_UNESCAPED_SLASHES); } catch (\JsonException) { return ''; } - - if ('null' === $minifiedJson) { - return ''; - }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Classes/EventListener/ContentMinifierEventListener.php` around lines 100 - 105, In the try/catch around building $minifiedJson in ContentMinifierEventListener (the json_encode/json_decode block), update the json_decode call to pass JSON_THROW_ON_ERROR via the options parameter (e.g., json_decode($json, true, 512, JSON_THROW_ON_ERROR)) so malformed JSON throws a JsonException that the existing catch (\JsonException) will handle; this makes error handling consistent with the json_encode usage and avoids relying on null-return checks for invalid JSON.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@composer.json`:
- Around line 9-21: The TYPO3 package constraints ("typo3/cms-core",
"typo3/cms-backend", "typo3/cms-frontend", "typo3/cms-adminpanel",
"typo3/cms-linkvalidator") are set to "^14.3" which is not available on
Packagist; update those composer version constraints to either a resolvable
release like "^14.2" or to the development branch "14.3.x-dev" (or mix as
appropriate) so composer can install dependencies successfully.
In `@README.md`:
- Line 7: The badge markdown "[]()" has an empty link
causing MD042; fix it by either supplying a meaningful URL (e.g., the TYPO3
project page or release notes) inside the parentheses or remove the link wrapper
so the image stands alone ("[]" or simply ""); update the
README.md line containing that exact badge markup accordingly.
---
Outside diff comments:
In `@Classes/EventListener/LastDeploymentEventListener.php`:
- Around line 23-26: In LastDeploymentEventListener, the early-return condition
is inverted: change the check on $path in the method where $path is set from
GeneralUtility::getFileAbsFileName so that it returns only when the path IS
empty or the file does not exist (i.e. use if ($path === '' ||
!file_exists($path)) return; or equivalent empty() logic) instead of the current
($path !== '' || !file_exists($path)); update the condition in that function to
the corrected form to allow execution when a valid path exists.
---
Nitpick comments:
In `@Classes/EventListener/ContentMinifierEventListener.php`:
- Around line 100-105: In the try/catch around building $minifiedJson in
ContentMinifierEventListener (the json_encode/json_decode block), update the
json_decode call to pass JSON_THROW_ON_ERROR via the options parameter (e.g.,
json_decode($json, true, 512, JSON_THROW_ON_ERROR)) so malformed JSON throws a
JsonException that the existing catch (\JsonException) will handle; this makes
error handling consistent with the json_encode usage and avoids relying on
null-return checks for invalid JSON.
🪄 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: b1348593-50ad-4887-9a7f-91c81c7a8436
⛔ Files ignored due to path filters (1)
composer.lockis excluded by!**/*.lock
📒 Files selected for processing (8)
Classes/EventListener/ContentMinifierEventListener.phpClasses/EventListener/LastDeploymentEventListener.phpClasses/EventListener/SaveCloseButtonEventListener.phpClasses/Page/AssetRenderer.phpREADME.mdTests/CGL/phpstan.neonTests/CGL/rector.phpcomposer.json
💤 Files with no reviewable changes (1)
- Tests/CGL/phpstan.neon
Summary by CodeRabbit
New Features
Documentation
Chores