Not sure why there are so many old commits in here since I based this branch off of 2.x-factories, but that's a problem for future Coby to solve...

Not sure why there are so many old commits in here since I based this branch off of 2.x-factories, but that's a problem for future Coby to solve...

That’s probably because you merged 2.x-docs-api into 2.x-factories. However, the 2.x-docs-api branch is pulled against the 2.x branch in #2073. That’s why all the commits from that PR also show up here.

And I realized it’s hard to look at what you changed if all those commits from #2073 are in here as well. You probably have to revert that somehow.

Up until here, what you write makes sense to me. But I also have to say that the exact inner workings of all the classes that are in involved here were always a bit hard to keep track of for me. So if we can reduce the complexity here, that would be great 👍.

* I _think_ we can do away with the array wrapping the `WP_Query` in an array (i.e. `[ 'query' => $wp_query ]`) when passing to the constructor. The `post_class` option is a precursor to the new Class Maps design and the `merge_default` seems like something that would better be implemented as an option to `Timber::get_posts()`.

Yes! I think I already partially worked on this in #2145. Maybe you could build on that.

* As a nice bonus, we can save on performance and insantiate Posts inside of collection lazily, meaning while we already have the `WP_Post` objects they wrap, the `Timber\Post` instances themselves are not actually created until they're needed. See `PostsIterator` for how this works.

Doesn’t this have big implications on caching if the actual work is only done after fetching cached posts in PHP templates during Timber::render() or earlier?

And then this is probably also related to handling posts in multisite environments (see #1434 and #2207).

And from what I see in issues and StackOverflow questions, a lot of developers are relying on dumping post objects to see what’s in there. This of course wouldn’t be reliable anymore. I’m not saying I wouldn’t add lazy-loading, but we need to check what it means for other parts of Timber and probably add documentation about this in various places.

A related example: we already fetch meta values lazily. A lot of developers were used to work with {{ dump(post.custom) }} to see what’s there. Now, this won’t work anymore. See https://timber.github.io/docs/v2/guides/custom-fields/#accessing-all-meta-values.

If I'm missing something here and we need this class after all, please let me know! But if we can move out the logic we want to keep I'm hoping we can just get rid of it.

I’m all in favor of simplifying this. What I figured when doing more complex stuff with iterators is that as soon as we have factories ready, we can move some logic regarding globals into Post::setup() method.

One specific example is this class: https://github.com/mindkomm/timber-integration-woocommerce/blob/master/lib/ProductsIterator.php. We discussed this in mindkomm/timber-integration-woocommerce#6. With factories, we can get rid of it completley and a WooCommerce product object might solve WooCommerce globals like this:

class Product extends Timber\Post {
    public function setup( $loop_index = 0 ) {
        parent::setup( $loop_index );

        wc_setup_product_data( $this->ID );

        return $this;
    }
}		

That’s probably because you merged 2.x-docs-api into 2.x-factories...You probably have to revert that somehow.

Dang, OK, I'll figure out how to do that.

I think I already partially worked on this in #2145. Maybe you could build on that.

Awesome! I will take a look there, thanks!

Doesn’t this have big implications on caching if the actual work is only done after fetching cached posts in PHP templates during Timber::render() or earlier?

I don't think I understand what you mean. :) Which code are you referring to as "fetching cached posts," and how is that affected by this change?

I agree we'd want to document the effects of the change if it actually affects the behavior of the API. But as far as I can tell this will be completely transparent to the caller. That is, by the time you have a post var within a Twig template, you've already done all the work that's needed. And, to be clear, this only affects how Posts are instantiated in PostQuery for now; in every other context they're instantiated eagerly. So I'm not sure why {{ dump(post.custom) }} wouldn't work.

Again, maybe I'm missing something...wouldn't be the first time. :) I'm also cool with being cautious and reverting the lazy instantiation piece for now. If I'm right and it's truly transparent, that means we can roll it out after 2.0 lands.

I’m all in favor of simplifying this. What I figured when doing more complex stuff with iterators is that as soon as we have factories ready, we can move some logic regarding globals into Post::setup() method.

Makes sense to me! Good idea.

OK, git push --force auto-closed this PR because I reset it to be exactly the same as 2.x-factories. Gonna cherry-pick my commits back into this branch and reopen. Sorry for the confusion.