WordPress / twentytwenty

Twenty Twenty is included in Core as of WordPress 5.3 🎉 To report any issues, please go here: https://core.trac.wordpress.org/newticket
Other
301 stars 140 forks source link

Starter content processed even if not used #973

Closed schlessera closed 5 years ago

schlessera commented 5 years ago

I noticed that the way starter content is currently being handled breaks WP-CLI tests when switching Core versions, even though neither the theme nor its starter content would actually be useful to load. See here for some of the breaking tests: https://travis-ci.org/wp-cli/automated-tests/jobs/605262104#L1103-L1109

The main issue is that the entire starter content is loaded, parsed and processed only to add it to the theme support hook. If it will not be used after that, all the entire processing did was just waste resources: not only is memory allocated to construct the array, we also have filesystem checks and translations that will be processed.

As we don't have autoloading in Core yet, I suggest wrapping the starter content in a Closure that only requires the necessary file once the corresponding hook is triggered.

pattonwebz commented 5 years ago

The overhead on this really is much larger than I realised and we shouldn't be doing all of this on each request when starter content is only needed one time at a very specific point.

Thank you @schlessera for reporting this and very kindly offering to provide a PR to resolve :D

schlessera commented 5 years ago

After having a closer look at how the starter content functionality was implemented, I have to assume there is not easy fix to change this.

The add_theme_support() function can neither accept a callback nor an object that would resolve into an array. Therefore, whatever you want to pass in there needs to already be a fully resolved array, with all processing done.

The only mechanism I can see right now to have the parsing be deferred would be to add iterable objects as children to the first-level keys of the array. But that would never be accepted for Core code I guess.

So, a proper fix would be to adapt the add_theme_support() function so that it can also accept callables, and execute these callables when they are requested for further processing. Alternatively, having the add_theme_support() accept filters to have the needed content built would work as well.