-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
[Feature] Optional chaining operator #3260
Comments
it should even be the default behaviour. |
-1 for being the default behavior. That would make it a lot harder to identify mistakes (typo in a property name) when you don't expect it to be optional. |
In php, there is tools like phpstan returning error for
if getBar can be null. But there is nothing like this for twig. So mistake is easier to make. I find sad the fact that
throw an exception and display an error 500 in production when foo.bar is null. This should be user-first and not dev-first. What about a strict mode, enable in dev ? |
Well, first, it does not throw an exception by default in production, as If what you want is to have everything optional in prod and strict in dev, Twig already has that feature since the 1.0 release (and probably even before in some 0.x releases). To me, the benefit of a |
Oh sorry, didn't know that. That's great. I agree with you then. |
Would be cool to have this feature 👍 |
Null-safe operator was introduced in PHP 8 https://wiki.php.net/rfc/nullsafe_operator |
We should have this. |
Have any maintainers taken a look at this yet? this would be an awesome feature that would aid in writing shorter, more expressive templating. I know maintainers often have a lot on their plate, so I understand if there are other priorities, but I would at least like to know if this is being considered / on a roadmap. I am guessing that the use of this construct would just get translated to the underlying php operator, in which case it would probably be pretty easy to implement, but would only be compatible with php8, so maybe that is holding the feature back? |
As mentioned in #3609 this would also be a nice little improvement for filters like in |
@nicolas-grekas @hason As top contributors, do you know (or know who would know) if an implementation of this would be accepted or is maybe even on the roadmap? PS - sorry for the direct tag, but as no maintainer has replied to my request for comment here or on the slack channel, I didn't know of a better way to get a response. |
You should ask @fabpot. It's a lot more likely to be accepted if there's a PR, but I can understand the reluctance to work on something that might be rejected. |
I would review a PR implementing this new behavior. But if we want to be "simple" to implement, it might mean bumping the min PHP version to 8.0 (7.2 right now for the 3.x branch). |
Not that I am in any way an expert, but I can't see any reason why the implementation has to be constrained by platform feature support of the same. Twig has a history of implementing features before PHP (e.g. named parameters) and that's a good thing. |
Implementation will tell us. |
@Bilge Twig named parameters are a compile-time feature (which is why Twig macros don't support named parameters btw). @fabpot I'm not sure we can use the native optional chaining to implement that in Twig, due to |
Indeed stof. But again, let’s talk about implementation in a PR |
Any news? |
@xepozz Until now nobody proposed a pull request implementing this feature. |
I don't really have time for PR and refinements, but here's some inspiration, it shouldn't be that hard:
Then somehow load it into Twig environment
and finally it coul be used as
|
I would like to also note, that I have a feeling that |
I started playing around with an implementation of this, and as soon as I got a basic something working, I realized that I can't actually think of a situation where it is needed/helpful. Some of these points have been made earlier in the conversation here, but I think it may be helpful to summarize. Say we have: {% set foo = { bar: 'baz' } %} First of all (as @stof pointed out), without strict mode (the default), {# with `strict_variables` set to false #}
{{ foo.bar }} {# output: 'baz' #}
{{ foo.bad }} {# output: '' #}
{{ bad.bar }} {# output: '' #} I think it is pretty common practice for prod environments to be set to non-strict variables (you don't want visitors seeing error messages), and dev environments to be set to strict variables; hence that case does need to be handled too, and I think it is. If the goal is to perform a test, the {# with `strict_variables` set to true or false #}
{% if foo.bar is defined %}defined{% else %}not defined{% endif %} {# output: 'defined' #}
{% if foo.bad is defined %}defined{% else %}not defined{% endif %} {# output: 'not defined' #}
{% if bad.bar is defined %}defined{% else %}not defined{% endif %} {# output: 'not defined' #} If the goal is to assign to a variable, the default filter will do the job (as @realjjaveweb pointed out): {# with `strict_variables` set to true or false #}
{% set var = foo.bar|default(null) %}{{ var }} {# output: 'baz' #}
{% set var = foo.bad|default(null) %}{{ var }} {# output: '' #}
{% set var = bad.bar|default(null) %}{{ var }} {# output: '' #} So the only place where the I'm curious if given this summary, anyone still knows of a use for this in twig. My current take, given Twig's current behaviors and abilities is
|
@acalvino4 if you look at the behavior of the |
I see, so for the case So I acknowledge that there is slightly different behavior. If all you are doing is printing Hence I am curious how many real-world use-cases can't easily be handled with |
@acalvino4 I totally agree. In 99% cases it's about textual output. Outputing null is unlikely and if that's happening very often in the app one should create custom function/filter since that's quite an edge case.
You can test it on https://twigfiddle.com/ |
One more important note note - @stof is a bit misleading here
so this...
=> outputs What wasn't said here is the documentation says that if you want to tackle undefined booleans - you should use
|
@realjjaveweb I never said it was the same that PHP empty. It is the same than Twig* empty. but |
The discussion started to drift away from the initially described problem: The optional chaining operator only really gets useful, when there is a chain of objects. Sure, when the chain only has two elements, there are pretty simple other ways to handle a nullable object (the default-filter or tenary were named). But when there is a real chain, like |
There is a third goal: output of content. Imagine the Output |
@element-code quick correction: |
Just started working with twig files, coming from JavaScript. Would love to have this 🔥 |
@acalvino4 @spackmat My usecase (in strict_variables mode) is {{ site.getLatestResult('host').getHtmlPill() }}
{% set hostResult = site.getLatestResult('host') %}
{{ hostResult ? hostResult.getHtmlPill() }} Not horrible, but not as readable as {{ site.getLatestResult('host')?.getHtmlPill() }} Using With yield Twig\Extension\EscaperExtension::escape(
$this->env,
(
// Once to see if it's empty
TwigBridge\Node\GetAttrNode::attribute(
$this->env,
$this->source,
TwigBridge\Node\GetAttrNode::attribute(
$this->env,
$this->source,
$context["site"],
"getLatestResult",
["host"],
"method",
false,
true,
false,
58
),
"getHtmlPill",
[],
"method",
true,
true,
false,
58
) ? (
Twig\Extension\CoreExtension::defaultFilter(
// Once again if it's not
TwigBridge\Node\GetAttrNode::attribute(
$this->env,
$this->source,
TwigBridge\Node\GetAttrNode::attribute(
$this->env,
$this->source,
$context["site"],
"getLatestResult",
["host"],
"method",
false,
true,
false,
58
),
"getHtmlPill",
[],
"method",
false,
false,
false,
58
),
"XXX"
)
) : ("XXX")
),
"html",
null,
true
); Actually that looks like a bug... Why is it called twice? Once before defaultFilter() ...? With the default value "XXX" twice... Twig 3.9.3 |
Bump for visibility |
In 4 years, this still has not been implemented? I mean it's a very simple and basic feature. |
@tomazartack if you think it is very simple to add it, what about opening a PR contributing an implementation ? |
Add support for the optional chaining operator
?.
that is available in:The text was updated successfully, but these errors were encountered: