twigphp / twigphp/Twig

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

Open
#4,491 4 comments 2 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
PHP
Stars
8.4k
Forks
1.3k
Avg merge
1d 13h
Merged PRs (30d)
37

Description

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 https://github.com/twigphp/Twig/issues/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:

https://github.com/twigphp/Twig/blob/3b06600ff3abefaf8ff55d5c336cd1c4253f8c7e/src/Extension/CoreExtension.php#L1651-L1652

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.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with src/Extension/CoreExtension.php at the linked getAttribute implementation and review the behavior described in issue #360. Compare the proposed extension/configuration injection point with the alternative __get fallback change; done means the project has an agreed, documented behavior for custom objects without tightly coupling to Laravel.

Written by the indexing model from the issue text.

Assessment

Tech stack
php
Domain
web-dev
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.