Timber: Timber::get_terms does not support all native arguments

[chetzof]

\Timber::get_terms('scom_property_contract', ['fields' => 'id=>name']);

Notice: Trying to get property of non-object in /srv/www/proj/current/web/app/plugins/timber-library/functions/timber-term-getter.php on line 69

Notice: Trying to get property of non-object in /srv/www/proj/current/web/app/plugins/timber-library/functions/timber-term-getter.php on line 69

Notice: Undefined property: WP_Query::$queried_object in /srv/www/proj/current/web/wp/wp-includes/query.php on line 3960

Notice: Trying to get property of non-object in /srv/www/proj/current/web/app/plugins/timber-library/functions/timber-term.php on line 47

Notice: Trying to get property of non-object in /srv/www/proj/current/web/app/plugins/timber-library/functions/timber-term-getter.php on line 69

Notice: Trying to get property of non-object in /srv/www/proj/current/web/app/plugins/timber-library/functions/timber-term-getter.php on line 69

Notice: Undefined property: WP_Query::$queried_object in /srv/www/proj/current/web/wp/wp-includes/query.php on line 3960

Notice: Trying to get property of non-object in /srv/www/proj/current/web/app/plugins/timber-library/functions/timber-term.php on line 47

I have no problem using the native function, but i think this fact should be mentioned in the docs. And maybe the method should throw an exception when using unsupported args.

[jarednova]

@chetzof the issue is that the result from native get_terms is translated into an array of TimberTerm objects. Since you're requesting the result be the id=>name associative array, the error is thrown.

It's an easy fix, the only question is what's the best user expectation....?

1. Use the fields arg to determine the return (as native WP does)
2. Ignore the fields designation and _always_ return an array of TimberTerms (as you are using Timber::get_terms, right?)
3. Something else

I think the most important thing (as you note) is to document the choice. I'm struggling with which of the above is the best option. Any thoughts? (or anyone else want to weigh-in?)

[chetzof]

I think solution 2 is the least expected behavior.

Solution 1 sounds better, but why using then the Timber::get_terms if the native function is available for returning plain array. Moreover, the project scope is not to proxy native methods but offer extra functionality, therefore I think it's important to keep such behavior consistent. I myself did use it that way just out of curiosity ;)

I would rather define a list of supported arguments and throw an exception or trigger an error if the passed argument is not valid. That's way the developer is instantly notified that he's doing something wrong.

[palmiak]

IMO the most important thing now is to add the args to docs.

[jarednova]

I'm closing all 2+ year old tickets. If someone wants to pick this up, let's make it a fresh issue