Skip to content

Commit

Permalink
feature #1041 Props tag remove props from the attribute list (matheo,…
Browse files Browse the repository at this point in the history
… WebMamba)

This PR was merged into the 2.x branch.

Discussion
----------

Props tag remove props from the attribute list

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| Tickets       | none
| License       | MIT

An assertion in ComponentFactory is blocking us to use non-scalar values for anonymous components. `@weaverryan` do you know why this check was made for?

Commits
-------

6f693d6 Remove .idea
58fce65 remove .idea
f6ac821 remove attributes from context
416c46a make remove method immutable and test if attributes are still render
66b7f58 remove props from the attribute in the PropsNode
2555d26 Revert "props tag remove props from attributes"
a9d422e props tag remove props from attributes
2d9f300 Fix non scalar value to anonymous component
  • Loading branch information
weaverryan committed Aug 23, 2023
2 parents 5fd8da1 + 6f693d6 commit 2609c67
Show file tree
Hide file tree
Showing 8 changed files with 79 additions and 18 deletions.
13 changes: 13 additions & 0 deletions src/TwigComponent/src/ComponentAttributes.php
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@ public function __toString(): string
function (string $carry, string $key) {
$value = $this->attributes[$key];

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;
Expand Down Expand Up @@ -144,4 +148,13 @@ public function add($stimulusDto): self
// add the remaining attributes for values/classes
return $clone->defaults($controllersAttributes);
}

public function remove($key): self
{
$attributes = $this->attributes;

unset($attributes[$key]);

return new self($attributes);
}
}
4 changes: 1 addition & 3 deletions src/TwigComponent/src/ComponentFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -102,9 +102,7 @@ public function mountFromObject(object $component, array $data, ComponentMetadat
continue;
}

if (!\is_scalar($value) && null !== $value) {
throw new \LogicException(sprintf('A "%s" prop was passed when creating the "%s" 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, $componentMetadata->getName(), $key, get_debug_type($value)));
}
$data[$key] = $value;
}

return new MountedComponent(
Expand Down
30 changes: 23 additions & 7 deletions src/TwigComponent/src/Twig/PropsNode.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,17 +28,21 @@ public function __construct(array $propsNames, array $values, $lineno = 0, strin

public function compile(Compiler $compiler): void
{
$compiler
->addDebugInfo($this)
->write('$propsNames = [];')
;

foreach ($this->getAttribute('names') as $name) {
$compiler
->addDebugInfo($this)
->write('if (!isset($context[\''.$name.'\'])) {')
;
->write('$propsNames[] = \''.$name.'\';')
->write('$context[\'attributes\'] = $context[\'attributes\']->remove(\''.$name.'\');')
->write('if (!isset($context[\''.$name.'\'])) {');

if (!$this->hasNode($name)) {
$compiler
->write('throw new \Twig\Error\RuntimeError("'.$name.' should be defined for component '.$this->getTemplateName().'");')
->write('}')
;
->write('}');

continue;
}
Expand All @@ -47,8 +51,20 @@ public function compile(Compiler $compiler): void
->write('$context[\''.$name.'\'] = ')
->subcompile($this->getNode($name))
->raw(";\n")
->write('}')
;
->write('}');
}

$compiler
->write('$attributesKeys = array_keys($context[\'attributes\']->all());')
->raw("\n")
->write('foreach ($context as $key => $value) {')
->raw("\n")
->write('if (in_array($key, $attributesKeys) && !in_array($key, $propsNames)) {')
->raw("\n")
->raw('unset($context[$key]);')
->raw("\n")
->write('}')
->write('}')
;
}
}
21 changes: 21 additions & 0 deletions src/TwigComponent/tests/Fixtures/User.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<?php

namespace Symfony\UX\TwigComponent\Tests\Fixtures;

class User
{
public function __construct(
private readonly string $name,
private readonly string $email,
) {}

public function getName(): string
{
return $this->name;
}

public function getEmail(): string
{
return $this->email;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<twig:UserCard :user='user' class='foo'/>
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{% props user %}

<div {{ attributes }}>
<p>{{ user.name }}</p>
<p>{{ user.email }}</p>
<p>class variable defined? {{ class is defined ? 'yes': 'no' }}</p>
</div>
13 changes: 13 additions & 0 deletions src/TwigComponent/tests/Integration/ComponentExtensionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
namespace Symfony\UX\TwigComponent\Tests\Integration;

use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase;
use Symfony\UX\TwigComponent\Tests\Fixtures\User;
use Twig\Environment;

/**
Expand Down Expand Up @@ -182,6 +183,18 @@ public function testRenderAnonymousComponentInNestedDirectory(): void
$this->assertStringContainsString('class="primary"', $output);
}

public function testRenderAnonymousComponentWithNonScalarProps(): void
{
$user = new User('Fabien', '[email protected]');

$output = self::getContainer()->get(Environment::class)->render('anonymous_component_none_scalar_prop.html.twig', ['user' => $user]);

$this->assertStringContainsString('class="foo"', $output);
$this->assertStringContainsString('Fabien', $output);
$this->assertStringContainsString('[email protected]', $output);
$this->assertStringContainsString('class variable defined? no', $output);
}

private function renderComponent(string $name, array $data = []): string
{
return self::getContainer()->get(Environment::class)->render('render_component.html.twig', [
Expand Down
8 changes: 0 additions & 8 deletions src/TwigComponent/tests/Integration/ComponentFactoryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -91,14 +91,6 @@ public function testExceptionThrownIfRequiredMountParameterIsMissingFromPassedDa
$this->createComponent('component_c');
}

public function testExceptionThrownIfUnableToWritePassedDataToPropertyAndIsNotScalar(): void
{
$this->expectException(\LogicException::class);
$this->expectExceptionMessage('But, the value is not a scalar (it\'s a stdClass)');

$this->createComponent('component_a', ['propB' => 'B', 'service' => new \stdClass()]);
}

public function testStringableObjectCanBePassedToComponent(): void
{
$attributes = $this->factory()->create('component_a', ['propB' => 'B', 'data-item-id-param' => new class() {
Expand Down

0 comments on commit 2609c67

Please sign in to comment.