From 8c277dadd6f05f36f2dff719695e911e5f4f647a Mon Sep 17 00:00:00 2001 From: fadrian06 Date: Thu, 25 Jun 2026 16:48:54 -0400 Subject: [PATCH] Fix component style/script dedupe leakage Use per-render instance state for component style/script dedupe in View::fetch so assets are deduped within a render without leaking across tests or subsequent renders. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- flight/template/View.php | 173 +++++++++++++++------------- tests/ViewTest.php | 236 +++++++++++++++++++-------------------- 2 files changed, 211 insertions(+), 198 deletions(-) diff --git a/flight/template/View.php b/flight/template/View.php index 1db48ef..8983db5 100644 --- a/flight/template/View.php +++ b/flight/template/View.php @@ -29,6 +29,11 @@ class View private string $componentPrefix; private string $componentsPath; + private int $fetchDepth = 0; + /** @var array */ + private array $styles = []; + /** @var array */ + private array $scripts = []; /** * @param string $path Path to templates directory @@ -119,110 +124,118 @@ class View */ public function fetch(string $file, ?array $data = null): string { - $template = $this->getTemplate($file); - - if (!$this->exists($file)) { - throw new Exception("Template file not found: $template."); + if ($this->fetchDepth === 0) { + $this->styles = []; + $this->scripts = []; } - extract($this->vars); + $this->fetchDepth++; - if (is_array($data)) { - extract($data); + try { + $template = $this->getTemplate($file); - if ($this->preserveVars) { - $this->vars += $data; + if (!$this->exists($file)) { + throw new Exception("Template file not found: $template."); } - } - - ob_start(); - $component = include $template; - - switch (true) { - case is_callable($component): - $view = $component(); - ob_end_clean(); - - break; - case $component instanceof Component: - $view = $component->html(); - $css = $component->css(); - $js = $component->js(); - static $styles = []; - static $scripts = []; + extract($this->vars); - if ($css && !array_key_exists($template, $styles)) { - $view .= << - $css - - html; + if (is_array($data)) { + extract($data); - $styles[$template] = true; + if ($this->preserveVars) { + $this->vars += $data; } + } - if ($js && !array_key_exists($template, $scripts)) { - $view .= << - $js - - html; - - $scripts[$template] = true; - } + ob_start(); + $component = include $template; + + switch (true) { + case is_callable($component): + $view = $component(); + ob_end_clean(); + + break; + case $component instanceof Component: + $view = $component->html(); + $css = $component->css(); + $js = $component->js(); + + if ($css && !array_key_exists($template, $this->styles)) { + $view .= << + $css + + html; + + $this->styles[$template] = true; + } + + if ($js && !array_key_exists($template, $this->scripts)) { + $view .= << + $js + + html; + + $this->scripts[$template] = true; + } + + ob_end_clean(); + + break; + default: + $view = ob_get_clean(); + } - ob_end_clean(); + preg_match_all( + "/<$this->componentPrefix-(?[a-z-]+)\s*(?([a-z]+=\"[^\"]+\"\s*)*)?\s*\/>/", + $view, + $tagsMatches, + ); - break; - default: - $view = ob_get_clean(); - } + $tagsMatches = array_filter($tagsMatches); - preg_match_all( - "/<$this->componentPrefix-(?[a-z-]+)\s*(?([a-z]+=\"[^\"]+\"\s*)*)?\s*\/>/", - $view, - $tagsMatches, - ); + foreach ($tagsMatches[0] ?? [] as $tagIndex => $match) { + $tag = $match; + $component = $tagsMatches['component'][$tagIndex]; + $props = $tagsMatches['props'][$tagIndex] ?? ''; - $tagsMatches = array_filter($tagsMatches); + preg_match_all( + '/(?[a-z]+)="(?[^"]+)"/', + $props, + $propsMatches, + ); - foreach ($tagsMatches[0] ?? [] as $tagIndex => $match) { - $tag = $match; - $component = $tagsMatches['component'][$tagIndex]; - $props = $tagsMatches['props'][$tagIndex] ?? ''; + $propsMatches = array_filter($propsMatches); - preg_match_all( - '/(?[a-z]+)="(?[^"]+)"/', - $props, - $propsMatches, - ); + if ($propsMatches) { + $props = []; - $propsMatches = array_filter($propsMatches); + foreach (array_keys($propsMatches[0]) as $propIndex) { + $name = $propsMatches['name'][$propIndex]; + $value = $propsMatches['value'][$propIndex]; + $props[$name] = $value; + } + } else { + $props = []; + } - if ($propsMatches) { - $props = []; + $component = $this->fetch("$this->componentsPath/$component", $props); + $tagPosition = strpos($view, $tag); - foreach (array_keys($propsMatches[0]) as $propIndex) { - $name = $propsMatches['name'][$propIndex]; - $value = $propsMatches['value'][$propIndex]; - $props[$name] = $value; + if ($tagPosition === false) { + continue; } - } else { - $props = []; - } - $component = $this->fetch("$this->componentsPath/$component", $props); - $tagPosition = strpos($view, $tag); - - if ($tagPosition === false) { - continue; + $view = substr_replace($view, $component, $tagPosition, strlen($tag)); } - $view = substr_replace($view, $component, $tagPosition, strlen($tag)); + return $view; + } finally { + $this->fetchDepth--; } - - return $view; } /** diff --git a/tests/ViewTest.php b/tests/ViewTest.php index 520140a..ce37cd6 100644 --- a/tests/ViewTest.php +++ b/tests/ViewTest.php @@ -266,124 +266,124 @@ class ViewTest extends TestCase public static function pagesDataProvider(): array { return [ - // [ - // 'page-with-component-with-old-syntax', - // <<<'html' - // my-component - // html, - // ], - // [ - // 'page-with-component-with-new-syntax', - // <<<'html' - // my-component - // html, - // ], - // [ - // 'page-with-component-with-subcomponent', - // <<<'html' - //
- // my-component-with-subcomponent - // subcomponent - //
- // html, - // ], - // [ - // 'page-with-multiple-components', - // <<<'html' - //
    - //
  • my-component
    • - //
  • my-component
    • - //
- // html, - // ], - // [ - // 'page-with-functional-component', - // <<<'html' - // my-functional-component - // html, - // ], - // [ - // 'page-with-class-component', - // <<<'html' - // my-class-component - // html, - // ], - // [ - // 'page-with-class-component-with-styles', - // <<<'html' - // - // my-class-component-with-styles - // - - // - // html, - // ], - // [ - // 'page-with-class-component-with-scripts', - // <<<'html' - // my-class-component-with-scripts - - // - // html, - // ], - // [ - // 'page-with-class-component-that-extends-another-class-component', - // <<<'html' - // another-class-component extended by my-class-component-that-extends-another-class-component - // html, - // ], - // [ - // 'page-with-component-with-one-prop', - // <<<'html' - // - // - //

Hello, James

- // - // - // html, - // ['name' => 'James'], - // ], - // [ - // 'page-with-component-with-one-prop', - // <<<'html' - // - // - //

Hello, Victoria

- // - // - // html, - // ['name' => 'Victoria'], - // ], - // [ - // 'page-with-component-with-two-props', - // <<<'html' - // - // - //

Hello, Astronaut Victoria

- // - // - // html, - // ['name' => 'Victoria', 'occupation' => 'Astronaut'], - // ], - // [ - // 'page-with-three-different-components', - // <<<'html' - // my-component - // my-functional-component - // my-class-component - // html, - // ], - // [ - // 'page-with-component-with-prop-which-value-contains-spaces-and-numbers', - // <<<'html' - //

Hello, Constantine 1st the Great

- // html, - // ['name' => 'Constantine 1st the Great'], - // ], + [ + 'page-with-component-with-old-syntax', + <<<'html' + my-component + html, + ], + [ + 'page-with-component-with-new-syntax', + <<<'html' + my-component + html, + ], + [ + 'page-with-component-with-subcomponent', + <<<'html' +
+ my-component-with-subcomponent + subcomponent +
+ html, + ], + [ + 'page-with-multiple-components', + <<<'html' +
    +
  • my-component
    • +
  • my-component
    • +
+ html, + ], + [ + 'page-with-functional-component', + <<<'html' + my-functional-component + html, + ], + [ + 'page-with-class-component', + <<<'html' + my-class-component + html, + ], + [ + 'page-with-class-component-with-styles', + <<<'html' + + my-class-component-with-styles + + + + html, + ], + [ + 'page-with-class-component-with-scripts', + <<<'html' + my-class-component-with-scripts + + + html, + ], + [ + 'page-with-class-component-that-extends-another-class-component', + <<<'html' + another-class-component extended by my-class-component-that-extends-another-class-component + html, + ], + [ + 'page-with-component-with-one-prop', + <<<'html' + + +

Hello, James

+ + + html, + ['name' => 'James'], + ], + [ + 'page-with-component-with-one-prop', + <<<'html' + + +

Hello, Victoria

+ + + html, + ['name' => 'Victoria'], + ], + [ + 'page-with-component-with-two-props', + <<<'html' + + +

Hello, Astronaut Victoria

+ + + html, + ['name' => 'Victoria', 'occupation' => 'Astronaut'], + ], + [ + 'page-with-three-different-components', + <<<'html' + my-component + my-functional-component + my-class-component + html, + ], + [ + 'page-with-component-with-prop-which-value-contains-spaces-and-numbers', + <<<'html' +

Hello, Constantine 1st the Great

+ html, + ['name' => 'Constantine 1st the Great'], + ], [ 'page-two-same-components-with-one-style-and-script-tag', <<<'html'