Merged
Conversation
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. Thanks for integrating Codecov - We've got you covered ☂️ |
There was a problem hiding this comment.
Pull request overview
This PR fixes the handling of Vite manifest imports by correctly treating them as manifest keys rather than file paths, and adds proper resolution logic to convert these keys to actual file URLs for preloading.
Key Changes:
- Introduced
ManifestCollection::resolveImportUrls()method to resolve import keys to actual file URLs by looking them up in the manifest - Updated
AssetServiceto use the full manifest for resolving import keys when generating preload tags - Modified
ViteHelperto render preload tags as<link rel="modulepreload">elements instead of script tags
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/ValueObject/ManifestCollection.php | Added findByKey() and resolveImportUrls() methods to support import key resolution |
| src/Service/AssetService.php | Updated preload tag generation to accept full manifest and use resolveImportUrls() for proper key resolution |
| src/View/Helper/ViteHelper.php | Modified to render preload tags as link elements with modulepreload attribute |
| tests/TestCase/ValueObject/ManifestCollectionTest.php | Comprehensive test coverage for the new import resolution functionality |
| tests/TestCase/ValueObject/ManifestEntryTest.php | Updated documentation to clarify that getImportUrls() returns processed keys, not resolved paths |
| tests/Fixture/manifest-with-imports.json | Updated to reflect actual Vite manifest structure where imports are keys prefixed with _ |
| README.md | Revised caching documentation with more conservative performance claims |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
josbeir
added a commit
that referenced
this pull request
Mar 8, 2026
* Add support for inline output with block => false When passing `block => false` option to script() or css() methods, the helper now outputs tags directly instead of buffering to view blocks. This is consistent with CakePHP's Html helper behavior and allows for inline asset rendering in templates without using view blocks. Changes: - script(): Echo output when block is false, handle preload tags inline - css(): Echo output when block is false - addDependentCss(): Support inline parameter for dependent CSS Fixes TypeError when passing block => false (previously crashed with "Argument #1 ($name) must be of type string, false given"). * refactor: return ?string from script()/css() instead of echo Change script() and css() methods to return the tag string when block => false, instead of echoing directly. This is more consistent with CakePHP's HtmlHelper conventions and makes the code easier to test. * docs: add inline output section for block => false * Deduplicate @vite/client in development mode and fix inline output edge cases - Add viteClientRendered tracking to ViteHelper to prevent duplicate @vite/client output - Update AssetService to support skipViteClient option - Fix pluginScript/pluginCss to return ?string for inline mode (block => false) - Fix addDependentCss to skip CSS when cssBlock is false and not in inline mode - Add tests for deduplication and plugin inline output - Document deduplication and cssBlock behavior in README --------- Co-authored-by: Jasper Smet <josbeir@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This fixes lookup of imports and properly renders them as module assets for preloading.