Conversation
There was a problem hiding this comment.
Pull request overview
This PR migrates Laravel View functionality to Hypervel, including comprehensive test coverage for Blade compilation, view components, and the view engine system. The migration includes test fixtures, unit tests for various Blade directives and features, and core source files for the view service provider, view name normalization, and engine implementations.
Key Changes:
- Added comprehensive test suite covering Blade compilation features (150+ test files)
- Implemented ViewServiceProvider for dependency injection and service registration
- Added ViewName class for view name normalization
- Created ViewFinderInterface defining the view finder contract
- Implemented ViewException for view-specific error handling
- Added ShareErrorsFromSession middleware for error handling
- Implemented FileEngine for raw file rendering
Reviewed changes
Copilot reviewed 150 out of 151 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/View/fixtures/* | Test fixture files for view rendering tests |
| tests/View/ViewTest.php | Unit tests for View class functionality |
| tests/View/ViewPhpEngineTest.php | Tests for PHP engine |
| tests/View/ViewFileViewFinderTest.php | Tests for file view finder |
| tests/View/ViewFactoryTest.php | Comprehensive tests for view factory |
| tests/View/ViewEngineResolverTest.php | Tests for engine resolver |
| tests/View/ViewComponentTest.php | Tests for view components |
| tests/View/ViewComponentAttributeBagTest.php | Tests for component attributes |
| tests/View/ViewCompilerEngineTest.php | Tests for compiler engine |
| tests/View/ViewBladeCompilerTest.php | Tests for Blade compiler |
| tests/View/ComponentTest.php | Component integration tests |
| tests/View/Blade/* | Extensive Blade directive compilation tests |
| src/view/src/ViewServiceProvider.php | Service provider for view system |
| src/view/src/ViewName.php | View name normalization utility |
| src/view/src/ViewFinderInterface.php | Interface for view finding |
| src/view/src/ViewException.php | Custom exception for views |
| src/view/src/Middleware/ShareErrorsFromSession.php | Middleware for error sharing |
| src/view/src/Engines/FileEngine.php | File-based rendering engine |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/foundation/src/Http/Kernel.php
Outdated
|
|
||
| protected function enableHttpMethodParameterOverride(): bool | ||
| { | ||
| return $this->container->get(ConfigInterface::class)->get('view.enable_override_http_method', false); |
There was a problem hiding this comment.
we should cache this value to prevent getting from config instance at each time.
| /** | ||
| * The Content Security Policy nonce context key. | ||
| */ | ||
| protected const NONCE_CONTEXT_KEY = 'hypervel.vite.nonce'; |
There was a problem hiding this comment.
check if only these three properties should be isolated with context
There was a problem hiding this comment.
Yap, the other properties are configuration-related, and all requests should share the same configuration, so there's no need to isolate with content.
| { | ||
| $id = $id ? $this->stripParentheses($id) : "'" . (string) Str::uuid() . "'"; | ||
|
|
||
| return '<?php if (! $__env->hasRenderedOnce(' . $id . ')): $__env->markAsRenderedOnce(' . $id . '); ?>'; |
There was a problem hiding this comment.
just leave a note for myself to verify everything called from the view factory should be context-isolated.
There was a problem hiding this comment.
On line 84 of src/view/src/Factory.php, the __env variable is save a as Factory instance.
$this->share('__env', $this);
Then on line 165 of src/view/src/View.php, all shared data is retrieved.
protected function getContents(): string
{
return $this->engine->get($this->path, $this->gatherData());
}
/**
* Get the data bound to the view instance.
*/
public function gatherData(): array
{
$data = array_merge($this->factory->getShared(), $this->data);
...
return $data;
}
|
Hi guys. It’d be great to include this new PR as well: |
| }; | ||
|
|
||
| $view = Container::getInstance() | ||
| ->make(ViewFactory::class) |
| return $view->toHtml(); | ||
| } | ||
| return Container::getInstance() | ||
| ->make(ViewFactory::class) |
| */ | ||
| public function guessClassName(string $component): string | ||
| { | ||
| $namespace = Container::getInstance() |
| // the instance out of the IoC container and call the method on it with the | ||
| // given arguments that are passed to the Closure as the composer's data. | ||
| return function () use ($class, $method) { | ||
| return $this->container->make($class)->{$method}(...func_get_args()); |
| */ | ||
| public function callCreator(ViewContract $view): void | ||
| { | ||
| if ($this->getContainer()->get(ConfigInterface::class)->get('view.event.enable', false) |
| */ | ||
| public function renderTranslation(): string | ||
| { | ||
| return $this->container->make('translator')->get( |
- Cache HTTP method override flag in Kernel to avoid repeated config lookups - Cache ComponentTagCompiler instance in BladeCompiler for reuse - Cache application namespace in ComponentTagCompiler - Cache view event enabled flag in ManagesEvents trait - Replace container->make() with container->get() for singleton resolution
|
@binaryfire thx for your advice. I will find time to handle it this weekend. |
3 participants
Breaking Change