Weekly update – 7 May 2021

[ryan]

This week I've been working on a combination of core improvements, optimizations, and fixes, plus a dozen pull requests have been added. Thanks for all of the great pull requests to the core that many of you have submitted. PR authors will appear in our GitHub contributors list once the changes are merged to the master branch (that's apparently how GitHub works). I do think soon we'll focus on getting a new master version out, as 3.0.165 is starting to feel old relative to the current dev branch. You can see all that's been changed and added this week in the dev branch commit log. Next week we'll be doing more of the same, though planning to get into some of the PRs that I didn't this week because they required more review and testing (those that involved more than a few lines of code changes). We're not going to bump the dev branch version till likely next Friday, since this week's work will continue into next week. Thanks for reading and have a great weekend! 

[Zeka]

@ryan Thanks for update!

For some time I see a lot of such changes in the commits:

//before
$config = $this->wire('config');
$log = $this->wire('log');

//after
$config = $this->wire()->config;
$log = $this->wire()->log;

Is there is some technical background for such changes or it's just a matter of preference? (I remember that you have explained it in some other thread or blog post, but I could not find it).

[bernhard]

Not sure about performance, but I'm always using $this->wire->... because this supports intellisense while the other variants do not (in my setup):

$this->wire->... correctly suggests the class properties and methods:

$this->wire('files')->... does not:

I guess $this->wire needs an extra call of the magic getter, but I have no idea if that really matters?! Whereas having intellisense matters a lot for me in my daily work...

[ryan]

Quote

Is there is some technical background for such changes or it's just a matter of preference? (I remember that you have explained it in some other thread or blog post, but I could not find it).

Like what Bernhard mentioned — my IDE knows all about what I'm accessing when I type in $this->wire()->apivar or wire()->apivar, so it can suggest methods, arguments, and tell me when I've typed something wrong. Whereas less of this happens with $this->wire('apiVarName') or wire('apiVarName') — the IDE isn't nearly as helpful. So it's more of catering to the way the IDE works than anything else, and in exchange it makes coding easier, faster and less error prone. It's the same reason you'll sometimes see this in modules: 

/** @var Pages $pages */
$pages = $this->wire('pages');

That comment in the first line tells the IDE that $pages is referring to the Pages class. But this produces the same result with no /** comment */ necessary:

$pages = $this->wire()->pages;

In most Wire derived classes, $this->wire->pages (like Bernhard was using above) or $this->pages will also work and the IDE will still know what API var you are accessing. The reason I prefer to call it as a function $this->wire()->pages rather than a property $this->wire->pages (or $this->pages) is because it will work consistently everywhere in PW, even if a class has "fuel" turned off (see useFuel). Having fuel off is necessary in classes that can potentially have properties or field names that could overlap with API var names; an example is the Page class or your own custom Page classes in /site/classes/.

The other reason is that $this->wire()->apivar (wire method) is the most efficient access to an API var because it has the fewest hops. Accessing $this->wire->apivar or $this->apivar first goes to $this->__get('apivar') and then $this->wire()->apivar. So $this->wire()->apivar is more direct (one less hop). In reality it probably doesn't matter much though.

@Zeka

[bernhard]

Thx Ryan, I knew about the extra hop, but didn't know about the fuel thing. Nevertheless $this->wire()->... is no option for me at the moment, because I simply don't get any suggestions while $this->wire->... does bring me everything I need.

Does anybody reading this know how to make VSCode recognize $this->wire() calls? Maybe I'm just missing a setting in Intelephense or maybe we are just missing some typehints in the core where PHPStorm is clever enough to work while VSCode is not?

Thx

[teppo]

On 5/13/2021 at 4:01 PM, bernhard said:

Does anybody reading this know how to make VSCode recognize $this->wire() calls? Maybe I'm just missing a setting in Intelephense or maybe we are just missing some typehints in the core where PHPStorm is clever enough to work while VSCode is not?

It's not really about being clever: the core ships with a metadata file (wire/core/.phpstorm.meta.php) that makes PHPStorm aware of the inner workings of the wire() method. PHPDoc comments only state that the wire() method may return a number of different objects, so there's little that the editor can figure out from that alone.

VSCode doesn't (AFAIK) have anything similar to this, but depending on the plugins you use this may be possible — with some caveats. 

I use PHP Intelephense, and while it does actually read PHPStorm metadata, it doesn't seem to support reading it from subdirectories: https://github.com/bmewburn/vscode-intelephense/issues/1384. At the moment I don't have any idea how to get this to work, except of course for duplicating the metadata file to your project's root directory.

(Or finding a plugin that supports this feature ?)

[bernhard]

Thx @teppo that helped a lot already

3 hours ago, teppo said:

At the moment I don't have any idea how to get this to work, except of course for duplicating the metadata file to your project's root directory.

Did you try that? It does not have any effect at all for me... The only way I got that working is changing the return hints of the wire() method in wire.php

// from this
@return ProcessWire|Wire|Session|Page|Pages|Modules|User|Users|Roles|Permissions|Templates|Fields|Fieldtypes|Sanitizer|Config|Notices|WireDatabasePDO|WireHooks|WireDateTime|WireFileTools|WireMailTools|WireInput|string|mixed
// to that
@return ProcessWire |mixed

But that is really ugly and incorrect of course.

@ryan do you have any ideas what ProcessWire could do about that "problem"? Or would it be better to ask the Intelephense developer for help to support multiple return types in the @return hint? Maybe it should just return the first listed class by default or maybe it should list all listed classes and all derived methods? I've created an issue to ask at Intelephense as well: https://github.com/bmewburn/vscode-intelephense/issues/1800

The wire property already returns the ProcessWire class correctly. But if wire() is used as the new standard I'd prefer to make that work on my setup and start using it in my own modules as well.

[bernhard]

I got a promising response by the Intelephense developer:

Quote

If you rewrite the meta file in the newer phpstorm format it will be understood.

https://www.jetbrains.com/help/phpstorm/ide-advanced-metadata.html#override

Does anybody know how to do that?

[matjazp]

On 5/13/2021 at 2:48 PM, ryan said:

The other reason is that $this->wire()->apivar (wire method) is the most efficient access to an API var because it has the fewest hops. Accessing $this->wire->apivar or $this->apivar first goes to $this->__get('apivar') and then $this->wire()->apivar. So $this->wire()->apivar is more direct (one less hop). In reality it probably doesn't matter much though.

What about $this->wire('apivar') in regards to hops?

[ryan]

@bernhard

Quote

But if wire() is used as the new standard I'd prefer to make that work on my setup and start using it in my own modules as well.

There isn't really a standard, just a lot of options to find which suits you best. For most modules, just simply doing $this->pages, etc., is likely the simplest route to take. The core has some cases where fuel is turned off, which is rare with modules. But for the core I use $this->wire()->pages; or $this->wire('pages'); because it's one thing that's going to work consistently in all core classes since there will always be a wire() method on all Wire derived classes. If you are using $this->wire->pages that is fine, but just note that $this->wire hits __get(), then returns ProcessWire instance, then does it again to get to the pages part. So if it also works with your IDE, then just $this->pages would likely be preferable to $this->wire->pages. In the end, it's kind of micro optimization so I would use whatever you prefer. 

@matjazp

Quote

What about $this->wire('apivar') in regards to hops?

I think this is going to be the same as $this->wire()->apivar in that regard. The only reason I don't use the string argument so much anymore is because phpstorm seems to understand exactly what's being accessed with wire()->apivar but not wire('apivar'). Plus, it'll know if you've mistyped an API var name so long as it's not in a string.