[TwigComponent] Filter service config in AsTwigComponent #1402
Conversation
As this attribute is currently extended by AsLiveComponent, it allow us to also call parent::serviceConfig in LiveComponent
(will require a conflict in composer to force Twig&Live update together)
| 'expose_public_props' => $this->exposePublicProps, | ||
| 'attributes_var' => $this->attributesVar, | ||
| ]; | ||
| ]); |
There was a problem hiding this comment.
Nitpicking, but I think the intent here should be to remove null and '' values, but keep 0 as a valid value, so being explicit and doing fn ($v) => null !== $v && '' !== $v) would be better and less confusing.
There was a problem hiding this comment.
I (it took some time^^) understand why for LiveComponent... but we don't want to keep 0 for any of those 4 vars do we ?
There was a problem hiding this comment.
It took some time me too to figure out why the stuff in my PR does not work. I'm just saying it could have been avoided if valid values like 0 not have been filtered out. Anyway, it doesn't really matter at the moment, with these values.
There was a problem hiding this comment.
To be honest, i'd like to remove all the array_filters. We needed it for a BC thing, maybe we can handle it differently.
And it's because you lost time with this (and me too after haha) that i suggest to put the array_filter with the vars it filters :)
The second thing is... i'm more and more uncomfortable with the way LiveComponent uses TwigComponent... because none of it is covered by any contract or so...
There was a problem hiding this comment.
Honestly i did that to avoid that situation to happen again but if you think (and i see why) that would not, let's close this and focus back on yours ;)
There was a problem hiding this comment.
Should this PR be closed then?
weaverryan
added
Status: Waiting feedback
|
Let's close for now and adress this later |
4 participants
As this attribute is currently extended by AsLiveComponent, it allow us to also call parent::serviceConfig in LiveComponent
(will require a conflict in composer to force Twig&Live update together)