Conversation
brendt
force-pushed
the
view-includes
branch
from
ec59d73 to
76cd011
Compare
xHeaven
force-pushed
the
view-includes
branch
from
f1708f5 to
fffb168
Compare
xHeaven
force-pushed
the
view-includes
branch
from
593eb19 to
cd4d903
Compare
innocenzi
changed the title
| } | ||
|
|
||
| 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
| use Tempest\View\Parser\Token; | ||
| use Tempest\View\WithToken; | ||
|
|
||
| final class PhpElement implements Element, WithToken, HasImports |
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
| use Tempest\View\Element; | ||
| use Tempest\View\HasImports; | ||
|
|
||
| final class RootElement implements Element, HasImports |
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
2 participants
The idea of this PR is that we use PHP's
includefor view components instead of parsing everything in one file. This solves issues like colliding imports across view components.The problem however is the performance hit: the test I added yields
0.0068130493164062with our current approach,0.10852599143982with the include approach. On top of that, withincludewe move the performance hit to the render phase (which can't be cached).Maybe there's room for optimizations, but preliminary results aren't good.