refactor(view)!: improved view component rendering#1980
Conversation
ec59d73 to
76cd011
Compare
593eb19 to
cd4d903
Compare
| } | ||
|
|
||
| private function makeElement(Token $token, ?Element $parent): ?Element | ||
| public function make(Token $token, Element $parent): ?Element |
There was a problem hiding this comment.
ElementFactory now always requires a parent/root element
| } | ||
|
|
||
| $children = []; | ||
| $element->setParent($parent); |
There was a problem hiding this comment.
Refactored parent/child setting because not all elements have children, but all have a parent.
| { | ||
| $this->parent = $parent; | ||
|
|
||
| $this->parent->setChildren([...$this->parent->getChildren(), $this]); |
There was a problem hiding this comment.
See before: not all elements have children but all have a parent
There was a problem hiding this comment.
Represents a block of PHP code, currently only used to extract imports
|
|
||
| public function getImports(): array | ||
| { | ||
| preg_match_all('/^\s*use .*;/m', $this->content, $matches); |
There was a problem hiding this comment.
Currently we're still using a regex, we could improve if needed once this refactor works
There was a problem hiding this comment.
This new RootElement is used as… the root.
It ensures a ViewComponentElement has a parent where it can gather imports from
| { | ||
| $imports = []; | ||
|
|
||
| foreach ($this->children as $child) { |
There was a problem hiding this comment.
We only want to resolve imports from PhpElements, not from sibling ViewComponent elements. That's why we check instanceof PhpElement here
| return sprintf('new \%s([%s])', ImmutableArray::class, implode(', ', $entries)); | ||
| } | ||
|
|
||
| public function getImports(): array |
There was a problem hiding this comment.
View components can be nested, and we need to recursively resolve all imports.
Writing this reminds me: we also need to check for sibling PhpElements in the same file still. That's still TODO
|
|
||
| // 4. Map to elements | ||
| $elements = $this->mapToElements($ast); | ||
| $rootElement = $this->mapToElements($ast); |
There was a problem hiding this comment.
Refactoring to accomodate the new RootElement
This reverts commit e5a00a2.
a33b739 to
86afb0a
Compare
cc7041c to
5424184
Compare
This PR fixes some long-standing issues with tempest/view. Technically it does not introduce any breaking changes, but it might lead to failed template rendering in some occasions. First, an overview of what's changing:
includeinstead of compiling everything to one file. This fixes Colliding class names in view imports #1380 when two view components import classes with the same names from different namespacesBreaking changes
With this PR comes a refactor to the
Elementinterface, which now requires agetImportsmethod. However, a default implementation is provided by theIsElementtrait, and thus should have no impact.Apart from that, this PR may still lead to invalid view files with missing imports.
Let's say you have these two components:
When these two view components were referenced in the same file, they would render fine before this PR:
That's because tempest views used to be compiled into one big file, and all import statements would be grouped together. However, now that we've switched to including compiled view components, the above code would break, since there's no
use function Tempest\Router\uri;within the scope of<x-a>The fix is to include
uriin<x-a>as well:Thanks to source-mapping, you will now get a clear exception pointing you to the source of the problem, making fixing these issues simple.