New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Use PropertyAccess Component syntax for nested variables access #302
feat: Use PropertyAccess Component syntax for nested variables access #302
Conversation
cf1809f
to
11c5955
Compare
11c5955
to
673c035
Compare
@@ -5,3 +5,4 @@ | |||
/my-app.* | |||
/var/ | |||
/vendor/ | |||
.idea/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you revert that? It must be in your global git ignore
if (str_starts_with($key, '[')) { | ||
return PropertyAccess::createPropertyAccessor()->getValue($context, $key) ?? $default; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed this is a bit weird. Could we find a way to always use property access?
As a user, I would expect:
$context = ['foo' => 'bar', 'nested' => ['a' => 'b']];
$foo = variable('bar');
$a = variable('nested['a']);
But When I write this, I really wonder why it's better than
$a = variable('nested')['a']
(Note: I'm not against this PR. Just asking)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but the component syntax doesn't allow you to do that, as far as I know. An element of an array must be enclosed in square brackets like [nested][a]
The only thing that's better than syntax two (current), is that the default value must be managed again in the case, for example, that the index doesn't exist.
$a = variable('nested')['a'] ?? null
And worse if null
is not the default value
$a = variable('nested', 'another')['a'] ?? 'another'
But as indicated in the comment here #302 (comment), this approach could be handled in pure PHP without any deps
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not a huge fan of this PR as it adds complexity and a dependency for something already feasible in pure PHP (and so, a syntax more usual for PHP developers)
That is true, this is mainly for the DX, WDYT about using this instead of PropertyAccess ? $array = [
'nested' => [
'key' => 'nested_value',
],
];
$value = variable('nested.key', 'default_value'); // 'nested_value'
$value = variable('nested.key.unknown', 'default_value'); // 'default_value' Note Users who use |
IMHO, you should not spend more time on this one. I don't really think the DX is better. I always feels awkward the "property access style" in tools. More over, it breaks static code analyzer |
Okay, thanks for your feedbacks and time 😃 |
Description
This pull request introduces a feature enhancement that allows the use of the Property Access Component syntax for accessing nested variables within the context.
Changes Made
.gitignore
file to exclude the.idea/
directory.friendsofphp/php-cs-fixer
dependency to version 3.51.0 to align the local and CI environments.Example of Usage
Warning
Not sure about the usage of the
str_starts_with('[')
condition for detecting when use PropertyAccess.