Fixed bound attributes not rendering for non-data attributes #50479
Conversation
dash8x
commented
Mar 12, 2024
dash8x
commented
- Fixed bound attributes not rendering for non-data attributes, see pull request [10.x] Anonymous component bound attribute values are evaluated twice #50403
- Fixed bound attributes not rendering for non-data attributes, see pull request laravel#50403
|
I think you may need a PR to target 10.x too |
|
Snapshot tests will need updating with the new compiled syntax |
Co-authored-by: Dan Harrin <[email protected]>
|
I just bumped into this bug as well. This test in the Aire test suite started failing at |
|
I was able to confirm that the code of this PR also fixes my issues too. Thanks, guys! Now we need to wait for @taylorotwell . |
|
@inxilpro @devajmeireles depending on the amount of free time you have, it might be worth opening a separate PR to add tests for Blade component usage. Currently, only compilation is tested, so its difficult to find edge cases like this, and if your packages depend on this functionality then it might be worthwhile in the long run :) |
| @@ -478,7 +478,7 @@ public function testClasslessComponents() | |||
| <?php if (isset(\$attributes) && \$attributes instanceof Illuminate\View\ComponentAttributeBag && \$constructor = (new ReflectionClass(Illuminate\View\AnonymousComponent::class))->getConstructor()): ?> | |||
| <?php \$attributes = \$attributes->except(collect(\$constructor->getParameters())->map->getName()->all()); ?> | |||
| <?php endif; ?> | |||
| <?php \$component->withAttributes(['name' => \Illuminate\View\Compilers\BladeCompiler::sanitizeComponentAttribute(\$componentData['data']['name'] ?? null),'age' => 31,'wire:model' => 'foo']); ?>\n". | |||
| <?php \$component->withAttributes(['name' => \Illuminate\View\Compilers\BladeCompiler::sanitizeComponentAttribute(array_key_exists('src', $componentData['data'] ?? []) ? (\$componentData['data']['src'] ?? null) : 'Taylor'),'age' => 31,'wire:model' => 'foo']); ?>\n". | |||
There was a problem hiding this comment.
Aren't these src references supposed to be name?
There was a problem hiding this comment.
There are a few tests containing this
|
Thanks. We're reverting the original PR instead: #50482 |
|
@driesvints that's a shame, can it be tried again in 11.x with these proposed changes, considering it is a major version? |
|
@danharrin I don't know. You can try a new PR with new tests that prove the breaking changes don't re-occur. Then maybe Taylor can reconsider it. Given this is all very brittle I wouldn't mess with it myself. |
|
I also hope this can be revisited with the proposed fixes. It’s a nice performance improvement. |
