Skip to content
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

[Developer Feature Request] Add injection point to CoreExtension::getAttribute for getting attributes from custom objects #4491

Open
mnpenner opened this issue Dec 1, 2024 · 2 comments

Comments

@mnpenner
Copy link

mnpenner commented Dec 1, 2024

After sitting on Twig 2, PHP 7.1, and Laravel 5.5 for ~10 years, I decided to upgrade some of my packages. One of the many issues I hit was #360

After much trial and error, I figured out that adding this:

if($object instanceof \Illuminate\Database\Eloquent\Model) {
    return $object->getAttribute($item);
}

Right in here:

// object property
if (/* Template::METHOD_CALL */ 'method' !== $type) {

Makes most of my problems go away.


Firstly, yes, I'm using Twig with Laravel (via https://github.com/rcrowe/TwigBridge). As you might know, Laravel is riddled with magic. Models have both __get and __call methods. Some methods look like this:

    public function customer() {
        return $this->belongsTo(Customer::class,'customer_id');
    }

Which represents a database relationship. So when I try {{ booking.customer }} in Twig (which used to work in old versions), Twig will first try looking for a $customer property on my Booking, won't find that, then implicitly tries __get, which might work but if it comes back null (per the referenced issue), it'll then try calling it instead which returns a BelongsTo object. Now any {% if booking.customer %} checks are broken because BelongsTo is truthy. What I really wanted is for Twig to evaluate that relationship, the same way Laravel does it when I call it like $booking->customer (no parens) in PHP.

Now there's no reason for Twig to provide tight coupling/integration with Laravel, so I propose some way for developers to override how getAttribute works. Maybe via a Twig extension, or some configuration option, something that would let me inject that little snippet of code to handle Laravel models properly.

As an alternative, I'm realizing now that adding this to the end of that same block:

if (method_exists($object, '__get')) {
    if ($isDefinedTest) {
        return false;
    }
    return null;
}

Also seems to work. It prevents the method call fallback and allows getters to properly return null. This is probably a backwards-incompatible change, so an option to enable "prevent method call fallback when __get is defined" might work and be simpler.

@ericmorand
Copy link
Contributor

As you might know, Laravel is riddled with magic.

Maybe the issue is that Laravel is plagued with magic.

@shyim
Copy link

shyim commented Dec 11, 2024

We do this uggly hack that works for us pretty okayyish.
https://github.com/shopware/shopware/blob/trunk/src/Core/Framework/Adapter/Twig/TwigEnvironment.php#L38-L45

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants