-
-
Notifications
You must be signed in to change notification settings - Fork 13
Draft: feat: add Vite #256
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
base: main
Are you sure you want to change the base?
Conversation
|
Hi @dangerdd1 , thanks for your pull request. While we're working on the migration of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces first-class Vite asset handling to Hypervel, including a Vite service, facade, middleware for Link headers, Blade compile helper, configuration additions, and comprehensive tests/fixtures.
- Adds Hypervel\Foundation\Vite with HMR/build support, CSP nonce, integrity, preloading and prefetch strategies, and helpers/APIs.
- Introduces AddLinkHeadersForPreloadedAssets middleware and a Vite facade/aliases; adds support Str::random factory and compiler @Vite directive.
- Extends tests and fixtures to validate Vite behavior across build/HMR, preloading, integrity, CSP, and prefetching.
Reviewed Changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Support/SupportStrTest.php | Tests Str::random and factory override behavior used by CSP nonce. |
| tests/Integration copy/Testing/TestCaseTest.php | Tests withoutVite() behavior to clear facade/resolved instance. |
| tests/Http/Middleware/VitePreloadingTest.php | Verifies middleware Link header population and limits. |
| tests/Foundation/fixtures/prefetching-manifest.json | Fixture manifest for prefetching tests. |
| tests/Foundation/fixtures/jetstream-manifest.json | Fixture manifest for preload/import tests. |
| tests/Foundation/FoundationViteTest.php | Extensive tests: build/HMR tags, CSP nonce, integrity, attributes, asset URLs, preloads/prefetching, manifest hash, content, and configuration. |
| src/testbench/workbench/config/app.php | Adds frontend_url and asset_url defaults for asset path resolution. |
| src/support/src/Str.php | Adds random string factory and robust random() generator. |
| src/support/src/Js.php | Adjusts convertDataToJavaScriptExpression return type. |
| src/support/src/Facades/Vite.php | Adds Vite facade with documented API surface. |
| src/support/src/Facades/Facade.php | Adds default alias 'Vite' to facade map. |
| src/http/src/Middleware/AddLinkHeadersForPreloadedAssets.php | New middleware to set Link headers from Vite preloaded assets, with optional limit. |
| src/foundation/src/ViteManifestNotFoundException.php | Deprecated exception alias to ViteException. |
| src/foundation/src/ViteException.php | New exception type for Vite errors. |
| src/foundation/src/Vite.php | Core Vite implementation (HMR/build, preloads/prefetching, CSP, integrity, attributes). |
| src/foundation/src/Testing/Concerns/InteractsWithContainer.php | Adds withoutVite()/withVite() helpers for testing. |
| src/foundation/src/Providers/FoundationServiceProvider.php | Minor style adjustment. |
| src/foundation/src/Application.php | Registers Vite binding alias 'vite'. |
| src/foundation/publish/app.php | Adds frontend_url and asset_url to published app config. |
| src/core/src/View/Compilers/Concerns/CompilesHelpers.php | Adds @Vite compiler directive to resolve the Vite service. |
| src/core/src/Context/ApplicationContext.php | Changes getContainer return type to PSR ContainerInterface. |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| public function handle(Request $request, Closure $next, ?int $limit = null): ResponsePlusInterface | ||
| { | ||
| return tap($next($request), function ($response) use ($limit) { | ||
| if ($response instanceof ResponsePlusInterface && Vite::preloadedAssets() !== []) { | ||
| $response->addHeader('Link', (new Collection(Vite::preloadedAssets())) | ||
| ->when($limit, fn ($assets, $limit) => $assets->take($limit)) | ||
| ->map(fn ($attributes, $url) => "<{$url}>; " . implode('; ', $attributes)) | ||
| ->join(', ')); | ||
| } | ||
| }); |
Copilot
AI
Oct 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The handle method declares a ResponsePlusInterface return type but returns whatever $next($request) yields. If a non-ResponsePlusInterface (e.g., Symfony Response) is returned, this violates the signature. Remove the return type or use a union that covers all possible response types (or simply omit the return type and document the union in phpdoc). For example: public function handle(Request $request, Closure $next, ?int $limit = null)
| protected function chunk(array $manifest, string $file): array | ||
| { | ||
| if (! isset($manifest[$file])) { | ||
| throw new ViteException("Unable to locate file in Vite manifest: {$file}."); |
Copilot
AI
Oct 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The trailing period in the exception message doesn't match tests and other messages. Remove the final '.' to match 'Unable to locate file in Vite manifest: {file}'.
| throw new ViteException("Unable to locate file in Vite manifest: {$file}."); | |
| throw new ViteException("Unable to locate file in Vite manifest: {$file}"); |
| $path = public_path($buildDirectory . '/' . $chunk['file']); | ||
|
|
||
| if (! is_file($path) || ! file_exists($path)) { | ||
| throw new ViteException("Unable to locate file from Vite manifest: {$path}."); |
Copilot
AI
Oct 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The trailing period in the exception message doesn't match the test expectation. Remove the final '.' to make it 'Unable to locate file from Vite manifest: {path}'.
| throw new ViteException("Unable to locate file from Vite manifest: {$path}."); | |
| throw new ViteException("Unable to locate file from Vite manifest: {$path}"); |
| use Exception; | ||
| use Hypervel\Support\Collection; | ||
| use Hypervel\Support\Contracts\Htmlable; | ||
| use Hypervel\Support\Facades\URL; | ||
| use Hypervel\Support\HtmlString; | ||
| use Hypervel\Support\Js; | ||
| use Hypervel\Support\Str; | ||
| use Hypervel\Support\Traits\Macroable; |
Copilot
AI
Oct 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The imports Exception and Hypervel\Support\Facades\URL are unused. Remove them to reduce noise and keep imports clean.
| use Hyperf\Stringable\Str as BaseStr; | ||
| use Ramsey\Uuid\Exception\InvalidUuidStringException; | ||
| use Ramsey\Uuid\Rfc4122\FieldsInterface; | ||
| use Ramsey\Uuid\Uuid; |
Copilot
AI
Oct 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The import Ramsey\Uuid\Uuid is not used in this file. Remove it to keep the imports minimal.
| use Ramsey\Uuid\Uuid; |
| class ApplicationContext extends HyperfApplicationContext | ||
| { | ||
| /** | ||
| * @return \Hypervel\Container\Contracts\Container |
Copilot
AI
Oct 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The phpdoc return type does not match the method signature (Container vs ContainerInterface). Update the docblock to reflect the actual return type (Psr\Container\ContainerInterface) or adjust the signature to match the intended project contract.
| * @return \Hypervel\Container\Contracts\Container | |
| * @return \Psr\Container\ContainerInterface |
Not sure how the team intended for this to work but here is my attempt. Let me know if you need refactoring based on your roadmap. Had to make other small fixes to get the tests to pass.