Skip to content

Commit

Permalink
bug #2328 [TwigComponent] Ignore "nested" for Alpine & Vue attributes…
Browse files Browse the repository at this point in the history
… (smnandre)

This PR was squashed before being merged into the 2.x branch.

Discussion
----------

[TwigComponent] Ignore "nested" for Alpine & Vue attributes

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| Issues        | Fix #1839
| License       | MIT

Have lost time on Twig & the website 😓

Alternative implementation of #2325

Update: Improved `__toString` performance following `@Kocal` comments

---
Now all these attributes are directly rendered

| Framework       | Prefix | Code Example                                                                                       | Documentation                                                 |
|-----------------|---------|-------------------------------------------------------------------------------------------------------|----------------------------------------------------------------|
| **Alpine.js**   | `x-`    | `<div x-data="{ open: false }" x-show="open"></div>`                                                 | [Documentation Alpine.js](https://alpinejs.dev/)               |
| **Vue.js**      | `v-`    | `<input v-model="message" v-if="show">`                                                              | [Documentation Vue.js](https://vuejs.org/guide/)               |
| **Stencil**     | `@`     | `<my-component `@onClick`="handleClick"></my-component>`                                               | [Documentation Stencil](https://stenciljs.com/docs/)           |
| **Lit**         | `@`     | `<button `@click`="${this.handleClick}">Click me</button>`                                             | [Documentation Lit](https://lit.dev/docs/)                    |

Commits
-------

7bd2854 Improve __toString performance
cb7809f Add str_contains in nested method
9f7c9c8 Performance
3452436 fabbot
1ed1db7 [TwigComponent] Ignore "nested" for Alpine & Vue attributes
  • Loading branch information
Kocal committed Nov 23, 2024
2 parents e95c0cd + 7bd2854 commit 9219cdf
Show file tree
Hide file tree
Showing 4 changed files with 109 additions and 38 deletions.
81 changes: 43 additions & 38 deletions src/TwigComponent/src/ComponentAttributes.php
Original file line number Diff line number Diff line change
Expand Up @@ -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<string,true> */
private array $rendered = [];
Expand All @@ -35,43 +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 (preg_match(self::NESTED_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
Expand Down Expand Up @@ -215,7 +217,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;
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<div{{ attributes }}></div>
52 changes: 52 additions & 0 deletions src/TwigComponent/tests/Integration/ComponentExtensionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,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('<twig:PrefixedAttributes %s/>', $attributes));

if ($expectContains) {
self::assertStringContainsString($attributes, trim($template->render()));

return;
}

self::assertStringNotContainsString($attributes, trim($template->render()));
}

/**
* @return iterable<array{0: string, 1: bool}>
*/
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()
Expand Down
13 changes: 13 additions & 0 deletions src/TwigComponent/tests/Unit/ComponentAttributesTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,19 @@ public function testNestedAttributes(): void
$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
{
$attributes = new ComponentAttributes([
Expand Down

0 comments on commit 9219cdf

Please sign in to comment.