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

Fixes failing attribute defined test #24

Merged
merged 4 commits into from
May 17, 2024
Merged

Conversation

bencroker
Copy link
Contributor

@bencroker bencroker commented May 3, 2024

Running on Craft 5 results in the error, Calling unknown method: craft\fields\Dropdown::valueType().

The code in question is failing because the attribute is defined test on objects that extend the Component class (or any class that implements __call()) always returns true.

{% if attribute(field, 'valueType') is defined %}
{% set type = field.valueType() %}
{% elseif attribute(field, 'phpType') is defined %}
{% set type = field.phpType() %}

This is a known issue.

This PR is a simple workaround that performs the check based on the Craft versions.

 {% for field in fields %}
 {% if craft.app.version < 5 %}
{% set type = field.valueType() %}
{% elseif craft.app.version > 5 and (attribute(field, 'phpType') is defined) %}

There’s probably a more elegant way to check for Craft version, I’ll leave that up to you.
Running on Craft 5, the “attribute is defined” test is failing and resulting in the error, Calling unknown method: craft\fields\Dropdown::valueType().

I also found that type needs a default value since it may not be set when tested against

{% if 'craft\\elements\\db\\MatrixBlockQuery' in type %}

Resulting in this working code.

 {% set type = 'mixed' %}
 {% for field in fields %}
 {% if craft.app.version < 5 %}
{% set type = field.valueType() %}
{% elseif craft.app.version > 5 %}
{% set type = field.phpType() %}
{% endif %}

@bencroker bencroker mentioned this pull request May 17, 2024
@markhuot markhuot merged commit b57f4d8 into markhuot:craft5 May 17, 2024
3 of 5 checks passed
@bencroker bencroker deleted the patch-1 branch June 28, 2024 16:00
@khalwat
Copy link

khalwat commented Jul 1, 2024

So I did a deep dive on this a while ago to figure out what was going on...

    {% if craft.mattwilcox is defined %}
        <p>Hi, Matt!</p>
    {% endif %}

compiles down to this PHP:

        // line 18
        if (craft\helpers\Template::attribute($this->env, $this->source, ($context["craft"] ?? null), "mattwilcox", [], "any", true, true)) {
            // line 19
            echo "        <p>Hi, Matt!</p>

which ends up calling getAttribute() in Twig's CoreExtension.php class, and because the craft variable object implements the PHP magic method __call https://www.php.net/manual/en/language.oop5.overloading.php#object.call it falls through to here: https://github.com/twigphp/Twig/blob/3.x/src/Extension/CoreExtension.php#L1698 and since $isDefinedTest = true it just returns that it's defined.

TL;DR: because the craft object implements the PHP __call magic method, it could dynamically return anything as being defined, so it just says it's defined. So literally anything will be "defined" as a property on an object that implements __call in Twig

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

Successfully merging this pull request may close these issues.

3 participants