From 1ed1db73acda56b85b45274c965dff43017384cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Andr=C3=A9?= Date: Sun, 3 Nov 2024 03:01:05 +0100 Subject: [PATCH 1/5] [TwigComponent] Ignore "nested" for Alpine & Vue attributes Have lost time on Twig & the website :sweat: --- src/TwigComponent/src/ComponentAttributes.php | 8 ++- .../components/PrefixedAttributes.html.twig | 1 + .../Integration/ComponentExtensionTest.php | 52 +++++++++++++++++++ .../tests/Unit/ComponentAttributesTest.php | 13 +++++ 4 files changed, 73 insertions(+), 1 deletion(-) create mode 100644 src/TwigComponent/tests/Fixtures/templates/components/PrefixedAttributes.html.twig diff --git a/src/TwigComponent/src/ComponentAttributes.php b/src/TwigComponent/src/ComponentAttributes.php index 57c5e4676d6..9f2a3a9dd2f 100644 --- a/src/TwigComponent/src/ComponentAttributes.php +++ b/src/TwigComponent/src/ComponentAttributes.php @@ -22,6 +22,8 @@ final class ComponentAttributes implements \Stringable, \IteratorAggregate, \Countable { private const NESTED_REGEX = '#^([\w-]+):(.+)$#'; + private const ALPINE_REGEX = '#^x-([a-z]+):[^:]+$#'; + private const VUE_REGEX = '#^v-([a-z]+):[^:]+$#'; /** @var array */ private array $rendered = []; @@ -41,7 +43,11 @@ public function __toString(): string fn (string $key) => !isset($this->rendered[$key]) ), function (string $carry, string $key) { - if (preg_match(self::NESTED_REGEX, $key)) { + if ( + preg_match(self::NESTED_REGEX, $key) + && !preg_match(self::ALPINE_REGEX, $key) + && !preg_match(self::VUE_REGEX, $key) + ) { return $carry; } diff --git a/src/TwigComponent/tests/Fixtures/templates/components/PrefixedAttributes.html.twig b/src/TwigComponent/tests/Fixtures/templates/components/PrefixedAttributes.html.twig new file mode 100644 index 00000000000..338d9f2f31c --- /dev/null +++ b/src/TwigComponent/tests/Fixtures/templates/components/PrefixedAttributes.html.twig @@ -0,0 +1 @@ + diff --git a/src/TwigComponent/tests/Integration/ComponentExtensionTest.php b/src/TwigComponent/tests/Integration/ComponentExtensionTest.php index 8b2b0e0987a..5cbe72c61ab 100644 --- a/src/TwigComponent/tests/Integration/ComponentExtensionTest.php +++ b/src/TwigComponent/tests/Integration/ComponentExtensionTest.php @@ -316,6 +316,58 @@ public function testRenderingComponentWithNestedAttributes(): void ); } + /** + * @dataProvider providePrefixedAttributesCases + */ + public function testRenderPrefixedAttributes(string $attributes, bool $expectContains): void + { + /** @var Environment $twig */ + $twig = self::getContainer()->get(Environment::class); + $template = $twig->createTemplate(\sprintf('', $attributes)); + + if ($expectContains) { + self::assertStringContainsString($attributes, trim($template->render())); + + return; + } + + self::assertStringNotContainsString($attributes, trim($template->render())); + } + + /** + * @return iterable + */ + public static function providePrefixedAttributesCases(): iterable + { + // General + yield ['x:men', false]; // Nested + yield ['x:men="u"', false]; // Nested + yield ['x-men', true]; + yield ['x-men="u"', true]; + + // AlpineJS + yield ['x-click="count++"', true]; + yield ['x-on:click="count++"', true]; + yield ['@click="open"', true]; + // Not AlpineJS + yield ['z-click="count++"', true]; + yield ['z-on:click="count++"', false]; // Nested + + // Stencil + yield ['onClick="count++"', true]; + yield ['@onClick="count++"', true]; + + // VueJs + yield ['v-model="message"', true]; + yield ['v-bind:id="dynamicId"', true]; + yield ['v-bind:id', true]; + yield ['@submit.prevent="onSubmit"', true]; + // Not VueJs + yield ['z-model="message"', true]; + yield ['z-bind:id="dynamicId"', false]; // Nested + yield ['z-bind:id', false]; // Nested + } + public function testRenderingHtmlSyntaxComponentWithNestedAttributes(): void { $output = self::getContainer() diff --git a/src/TwigComponent/tests/Unit/ComponentAttributesTest.php b/src/TwigComponent/tests/Unit/ComponentAttributesTest.php index 8ed7aa4f004..9ee141087a7 100644 --- a/src/TwigComponent/tests/Unit/ComponentAttributesTest.php +++ b/src/TwigComponent/tests/Unit/ComponentAttributesTest.php @@ -258,6 +258,19 @@ public function testNestedAttributes(): void $this->assertSame(' class="baz"', (string) $attributes->nested('title')->nested('span')); $this->assertSame('', (string) $attributes->nested('invalid')); } + + public function testPrefixedAttributes(): void + { + $attributes = new ComponentAttributes([ + 'x-click' => 'x+', + 'title:x-click' => 'title:x+', + ]); + + $this->assertSame(' x-click="x+"', (string) $attributes); + $this->assertSame(' x-click="title:x+"', (string) $attributes->nested('title')); + $this->assertSame('', (string) $attributes->nested('title')->nested('span')); + $this->assertSame('', (string) $attributes->nested('invalid')); + } public function testConvertTrueAriaAttributeValue(): void { From 34524363d3b822fea57e5cc9d31ba546e83e736a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Andr=C3=A9?= Date: Sun, 3 Nov 2024 03:04:12 +0100 Subject: [PATCH 2/5] fabbot --- .../tests/Integration/ComponentExtensionTest.php | 4 ++-- src/TwigComponent/tests/Unit/ComponentAttributesTest.php | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/TwigComponent/tests/Integration/ComponentExtensionTest.php b/src/TwigComponent/tests/Integration/ComponentExtensionTest.php index 5cbe72c61ab..66327e13398 100644 --- a/src/TwigComponent/tests/Integration/ComponentExtensionTest.php +++ b/src/TwigComponent/tests/Integration/ComponentExtensionTest.php @@ -352,11 +352,11 @@ public static function providePrefixedAttributesCases(): iterable // Not AlpineJS yield ['z-click="count++"', true]; yield ['z-on:click="count++"', false]; // Nested - + // Stencil yield ['onClick="count++"', true]; yield ['@onClick="count++"', true]; - + // VueJs yield ['v-model="message"', true]; yield ['v-bind:id="dynamicId"', true]; diff --git a/src/TwigComponent/tests/Unit/ComponentAttributesTest.php b/src/TwigComponent/tests/Unit/ComponentAttributesTest.php index 9ee141087a7..d1999400c55 100644 --- a/src/TwigComponent/tests/Unit/ComponentAttributesTest.php +++ b/src/TwigComponent/tests/Unit/ComponentAttributesTest.php @@ -258,7 +258,7 @@ public function testNestedAttributes(): void $this->assertSame(' class="baz"', (string) $attributes->nested('title')->nested('span')); $this->assertSame('', (string) $attributes->nested('invalid')); } - + public function testPrefixedAttributes(): void { $attributes = new ComponentAttributes([ From 9f7c9c8f3a2db0e9d508be3e1ce878156f475eec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Andr=C3=A9?= Date: Sun, 3 Nov 2024 14:52:09 +0100 Subject: [PATCH 3/5] Performance Co-authored-by: Hugo Alliaume --- src/TwigComponent/src/ComponentAttributes.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/TwigComponent/src/ComponentAttributes.php b/src/TwigComponent/src/ComponentAttributes.php index 9f2a3a9dd2f..194acaba941 100644 --- a/src/TwigComponent/src/ComponentAttributes.php +++ b/src/TwigComponent/src/ComponentAttributes.php @@ -44,7 +44,8 @@ public function __toString(): string ), function (string $carry, string $key) { if ( - preg_match(self::NESTED_REGEX, $key) + str_contains($key, ':') + && preg_match(self::NESTED_REGEX, $key) && !preg_match(self::ALPINE_REGEX, $key) && !preg_match(self::VUE_REGEX, $key) ) { From cb7809f87387ca5035405a35ec8cd825e67d107a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Andr=C3=A9?= Date: Sun, 3 Nov 2024 15:05:45 +0100 Subject: [PATCH 4/5] Add str_contains in nested method --- src/TwigComponent/src/ComponentAttributes.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/TwigComponent/src/ComponentAttributes.php b/src/TwigComponent/src/ComponentAttributes.php index 194acaba941..39213d945fe 100644 --- a/src/TwigComponent/src/ComponentAttributes.php +++ b/src/TwigComponent/src/ComponentAttributes.php @@ -222,7 +222,10 @@ public function nested(string $namespace): self $attributes = []; foreach ($this->attributes as $key => $value) { - if (preg_match(self::NESTED_REGEX, $key, $matches) && $namespace === $matches[1]) { + if ( + str_contains($key, ':') + && preg_match(self::NESTED_REGEX, $key, $matches) && $namespace === $matches[1] + ) { $attributes[$matches[2]] = $value; } } From 7bd2854d538cdcd021b1697aec23b0afa405aa92 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Andr=C3=A9?= Date: Sun, 3 Nov 2024 15:34:50 +0100 Subject: [PATCH 5/5] Improve __toString performance Running scenario: Short attributes, empty rendered __toString() time: 0.00077986717224121 seconds __toString2() time: 0.00049495697021484 seconds Improvement: 36.533170284317% Running scenario: Short attributes, partial rendered __toString() time: 0.00054383277893066 seconds __toString2() time: 0.00031709671020508 seconds Improvement: 41.692240245506% Running scenario: Short attributes, full rendered __toString() time: 0.00030899047851562 seconds __toString2() time: 0.00012898445129395 seconds Improvement: 58.256172839506% Running scenario: Long attributes, empty rendered __toString() time: 0.0038020610809326 seconds __toString2() time: 0.0026099681854248 seconds Improvement: 31.353859660124% Running scenario: Long attributes, partial rendered __toString() time: 0.0032980442047119 seconds __toString2() time: 0.0022611618041992 seconds Improvement: 31.439311790646% Running scenario: Long attributes, full rendered __toString() time: 0.00074219703674316 seconds __toString2() time: 0.00014185905456543 seconds Improvement: 80.886604561516% --- src/TwigComponent/src/ComponentAttributes.php | 79 +++++++++---------- 1 file changed, 37 insertions(+), 42 deletions(-) diff --git a/src/TwigComponent/src/ComponentAttributes.php b/src/TwigComponent/src/ComponentAttributes.php index 39213d945fe..bea7876dfd1 100644 --- a/src/TwigComponent/src/ComponentAttributes.php +++ b/src/TwigComponent/src/ComponentAttributes.php @@ -37,48 +37,43 @@ public function __construct(private array $attributes) public function __toString(): string { - return array_reduce( - array_filter( - array_keys($this->attributes), - fn (string $key) => !isset($this->rendered[$key]) - ), - function (string $carry, string $key) { - if ( - str_contains($key, ':') - && preg_match(self::NESTED_REGEX, $key) - && !preg_match(self::ALPINE_REGEX, $key) - && !preg_match(self::VUE_REGEX, $key) - ) { - return $carry; - } - - $value = $this->attributes[$key]; - - if ($value instanceof \Stringable) { - $value = (string) $value; - } - - if (!\is_scalar($value) && null !== $value) { - throw new \LogicException(\sprintf('A "%s" prop was passed when creating the component. No matching "%s" property or mount() argument was found, so we attempted to use this as an HTML attribute. But, the value is not a scalar (it\'s a "%s"). Did you mean to pass this to your component or is there a typo on its name?', $key, $key, get_debug_type($value))); - } - - if (null === $value) { - trigger_deprecation('symfony/ux-twig-component', '2.8.0', 'Passing "null" as an attribute value is deprecated and will throw an exception in 3.0.'); - $value = true; - } - - if (true === $value && str_starts_with($key, 'aria-')) { - $value = 'true'; - } - - return match ($value) { - true => "{$carry} {$key}", - false => $carry, - default => \sprintf('%s %s="%s"', $carry, $key, $value), - }; - }, - '' - ); + $attributes = ''; + + foreach ($this->attributes as $key => $value) { + if (isset($this->rendered[$key])) { + continue; + } + + if ( + str_contains($key, ':') + && preg_match(self::NESTED_REGEX, $key) + && !preg_match(self::ALPINE_REGEX, $key) + && !preg_match(self::VUE_REGEX, $key) + ) { + continue; + } + + if (null === $value) { + trigger_deprecation('symfony/ux-twig-component', '2.8.0', 'Passing "null" as an attribute value is deprecated and will throw an exception in 3.0.'); + $value = true; + } + + if (!\is_scalar($value) && !($value instanceof \Stringable)) { + throw new \LogicException(\sprintf('A "%s" prop was passed when creating the component. No matching "%s" property or mount() argument was found, so we attempted to use this as an HTML attribute. But, the value is not a scalar (it\'s a "%s"). Did you mean to pass this to your component or is there a typo on its name?', $key, $key, get_debug_type($value))); + } + + if (true === $value && str_starts_with($key, 'aria-')) { + $value = 'true'; + } + + $attributes .= match ($value) { + true => ' '.$key, + false => '', + default => \sprintf(' %s="%s"', $key, $value), + }; + } + + return $attributes; } public function __clone(): void