Discussion: Move FSE auto-draft creation into a read-time filter

[ockham]

Since the inception of Full Site Editing, auto-draft creation has been a pain point. The original requirements seem simple enough:

1. We distribute themes as a set of template files (the main difference to pre-FSE WordPress being that they are now block templates).
2. We also need to be able to allow editing of those templates in the Site Editor.

In order to leverage the block editor's existing functionality and patterns, it was decided to store templates in wp_template CPT objects (and template parts in wp_template_part), which can then be edited in Gutenberg. But this means we still need a bridge from template files to those CPTs, when first editing a template.

This is where auto-draft creation comes in: Whenever attempting to load a template, we first check whether there's already a CPT object for it; and if there isn't, we check if the current theme includes a template file for it. If the latter, we then create an on-the-fly wp_template object for it (with its post status set to auto-draft) and persist it to the DB. This doesn't just apply to loading in the editor, but also on the frontend, or more generally to any read access to a template -- e.g. when bundling all templates for export in a zip. Only once it's actually been edited and saved, the wp_template object will get the publish post status.

auto-draft creation has proven notoriously tricky for as long as people have worked on them: It had to be extended multiple times, e.g. to work for querying templates via the REST API; to accomodate for cases when the CPTs were deleted; etc. This also necessitated storing a number of additional meta data, and updating them through a number of actions.

I think that all of these things are inherent in what is basically a sync workflow: Whenever a wp_template object is read or modified, we need to do a bunch of bookkeeping to make sure it doesn't "disagree" with the underlying template file. This process is necessarily manual, and error-prone, as it's easy to miss instances where running these methods is required.

I had a quick Slack chat with our lovely FSE folks earlier today, and they have a few good ideas how to circumvent some of the issues inherent in the auto-draft post status (e.g. by using a different post status).

I wanted to discuss an idea that might be complementary to these efforts.

---

Cycling back to the first paragraphs of this issue, we really only need the auto-draft when:

1. we're trying to read a wp_template object
2. that doesn't actually exist in the DB yet
3. but for which there is a template file in the current theme.

The first item -- trying to read a wp_template object -- is relevant: I think it basically boils down to this call in gutenberg_resolve_template:

gutenberg/lib/template-loader.php

Lines 127 to 145 in 3d5c413

	// Find all potential templates 'wp_template' post matching the hierarchy.
	$template_query = new WP_Query(
	array(
	'post_type' => 'wp_template',
	'post_status' => array( 'publish', 'auto-draft' ),
	'post_name__in' => $slugs,
	'orderby' => 'post_name__in',
	'posts_per_page' => -1,
	'no_found_rows' => true,
	'tax_query' => array(
	array(
	'taxonomy' => 'wp_theme',
	'field' => 'slug',
	'terms' => wp_get_theme()->get_stylesheet(),
	),
	),
	)
	);
	$templates = $template_query->get_posts();

(There's at least one copy of sorts of this logic, in gutenberg_edit_site_export.)

My suggestion is basically the following: Knowing that we need to provide the auto-draft at read time, how about creating it inside of a filter that is applied at read time? The best candidate that I could find is the_posts (but there are a few 'nearby' ones). That filter allows to modify the results list, and gets the original query as an additional argument. We could do something like

function gutenberg_create_template_auto_drafts( 'the_posts', WP_Post[] $results, WP_Query $q ) {
	if ( ! 'wp_template' === $q->post_type || ! empty( $results ) ) {
		return $results;
	}

	// ...Load template from corresponding file in theme...

	_gutenberg_create_auto_draft_for_template( ... );

	return $newly_created_autodraft;
}
add_filter( 'the_posts', 'gutenberg_create_template_auto_drafts', 10, 2 );

One major advantage of this approach would IMO be that we'd separate concerns much better: We'd only ever create the auto-draft for the one template that is being queried (after template resolution is done, and cleanly separated from it), rather than creating them in bulk upon first loading, such as _gutenberg_synchronize_theme_templates does now. In fact, I hope that this would allow us to get rid of about pretty much all other synchronization logic.

One downside is that it would quite clearly add side effects to a filter -- that go as far as to write to the database in such a way that changes the results that are returned for the original read operation. (Tho we're kinda doing that already -- there might not be a way around that, given the prerequisites for FSE).

I was originally hoping we could even get away with 'simulating' a wp_template object when there really isn't one in the DB, and have the the_posts filter return it, but I now think that that wouldn't work: We'd probably need to include an ID that would be used by the editor when modifying the object, and we can't just pull one out of thin air.

Curious to hear your thoughts: @vindl @Copons @david-szabo97 @Addison-Stavlo @noahtallen @jeyip @mattwiebe and of course @youknowriad 😄

[noahtallen]

Currently, I believe it is executed only on wp_load. But it is executed every time it loads, which could be overly expensive. But that's much better than it was previously! So I guess the goal here would be to reduce that even further.

That's an interesting idea, and it seems like it could theoretically work.

I wonder if we could go even further by asking the question, "when can a template file change?"

I believe only a few times:

1. When the theme updates. In this case, we need to make sure non-modified templates are updated.
2. When a new theme is activated. In this case, we need to sync all of the theme templates.
3. Whenever the file is changed during development.

I wonder if we could simplify everything by only running the database sync op on theme activate or update. We wouldn't check the files outside of that. Then we could add a specific dev mode which does the sync op on wp_load to make sure development changes exist.

I think the main limitation of this approach is "what if the data goes missing or gets corrupted at some point". The benefit of doing the sync all the time is that we are constantly making sure that it's there when it needs to be. I could see someone deleting the "auto-drafts" (or whatever they will become), and then not having a working theme any more.

[david-szabo97]

I think we had a quite in-depth discussion here: #26383

Originally we wanted this:

When the theme updates. In this case, we need to make sure non-modified templates are updated.
When a new theme is activated. In this case, we need to sync all of the theme templates.
Whenever the file is changed during development.

But we ditched it. We had synchronization code laying around everywhere, rather than one place to handle it. We are already checking for file modification times, which should be enough to prevent perf problems. We aren't reading the files if they haven't changed since the last check time. filemtime is blazing fast compared to reading a file. (since it's just a file-meta)

I was originally hoping we could even get away with 'simulating' a wp_template object when there really isn't one in the DB, and have the the_posts filter return it, but I now think that that wouldn't work: We'd probably need to include an ID that would be used by the editor when modifying the object, and we can't just pull one out of thin air.

This would overcomplicate everything 😬

One major advantage of this approach would IMO be that we'd separate concerns much better: We'd only ever create the auto-draft for the one template that is being queried (after template resolution is done, and cleanly separated from it), rather than creating them in bulk upon first loading, such as _gutenberg_synchronize_theme_templates does now. In fact, I hope that this would allow us to get rid of about pretty much all other synchronization logic.

If we don't have CPTs for all the templates then we can't show them in the navigation sidebar or in the wp-admin. Well, we can if we filter all those stuff. Which sounds like a lot more work and code to maintain.

I'd lean towards this PR: #27277 This cleans up all the auto-draft mess.

[youknowriad]

In fact, I hope that this would allow us to get rid of about pretty much all other synchronization logic.

Thanks for the issue and this is exactly why we did the recent refactoring in the first place. So it seems like you're suggesting that it's not working as intended? you're basically suggesting to move from wp_load to get_the_posts. While it might reduce a little bit the number of times this get called, I do wonder whether it will have any meaningful impact compared to what we do now. Basically in the frontend, both those filters should always be executed and in the backend, a small delay is not the end of the world.

[ockham]

Thanks @noahtallen, @david-szabo97, and @youknowriad!

To be clear, my main concern is less about performance, but about 'correctness' and stability. I was recently dealing with an issue that was also related to synchronization, and some takeaways from there got me thinking about how could apply them to FSE.

'Correctness' not for correctness' sake: Synchronization can be messy and fragile, in that one must be careful to add the required sync step(s) whenever an operation is performed that requires it. This isn't obvious to contributors that haven't worked on the project before and who'd like to extend it -- bugs can crop up easily there.

I believe that our code is most stable when it best expresses semantically what we're trying to achieve -- this is why I've emphasized the requirements (create a template CPT: only at read time, when there's no CPT yet for the template in the DB, etc).

@youknowriad I didn't just want to move the sync logic from wp_load to [get_]the_posts -- rather, I was hoping to get rid of it pretty much entirely, and to replace it only with the on-the-fly generation of the wp_template object in the the_posts filter.

@noahtallen It's true however that I missed cases such as theme updates or switches that would still require syncing. (Thanks for pointing those out to me!)

I still think it might be worth exploring an alternative a bit more, if y'all are willing to indulge me 😬

Let me restate the problem: Let's think of the case of a freshly activated FSE theme whose templates haven't seen any modifications by the user in the Site Editor yet. If that website is loaded on the frontend, we wouldn't even need CPTs at all for things to work -- it'd be enough to load the relevant template file instead. Let's assume for a moment we start by implementing a first iteration of FSE just like that -- completely ignoring the Site Editor and wp_template CPTs.

Let's now consider the Site Editor as the next iteration, added on top of that first stage. We decide to use a wp_template (and _part) CPT based approach just like we do now. On the frontend, this means we now need to add logic to first attempt to find a such a CPT for the current template, and if none exists, fall back to the template file. (This still means the frontend will continue to work, even if no CPT has been created!)

Only once the editor comes in do we need the CPT to be actually created -- because that's how the editor works: It requires a post-like object. This is where things get tricky.

Our current solution has lumped together template resolution and CPT creation (the latter through auto-drafts, at the moment). This turned out difficult to get right, and has necessitated the sync logic.

But that solution is firmly based on the requirement that the editor needs a post-like object for editing. This has allowed us to make minimal adjustments to the editor, but has required a lot of logic at a lower level.

But since it's proven quite difficult to do all that to make the editor work, maybe it's reasonable to instead change the editor to accomodate for a system that otherwise works fine? How about we extend the concept of a 'blank' initial, unsaved, state (that's e.g. currently used for a new post in the post editor) into a customized one, that can be provided to the editor, basically as a prop? I.e. a post-like object -- but without all the DB ties, i.e. no ID etc. Really just a continuation of the blank slate into more customized territory. A bit like a template (for lack of a better word) in a word processor 😉

[noahtallen]

That's really interesting, Bernie, thanks for sharing! I definitely understand the first few phases, and I buy into that idea a bit. I do wonder how performance compares though. For example, is rendering a template by reading from the database significantly faster than rendering a template by reading from the filesystem? If it is much faster, maybe we would prefer to keep the CPT approach for rendering?

Changing the editor, though, would definitely be the challenging part of this. I'm not sure what I think, but some thoughts:

• Are we gaining a lot of built-in stuff by relying on posts? E.g. queries, entity resolvers, block content parsing, etc. Would we have to re-build that for an alternative approach?
• Would we be making everything less consistent by having two different storage methods for templates? Like, we would no longer be able to ask WordPress to give us all entities with postType: 'wp_template'. We would have to do a query for template CPTs, and then a filesystem query. I wonder if the developer experience of just treating the database wp_template CPT as the source of truth would be better.

[youknowriad]

@ockham Just came here after creating #27321 Is this what you have in mind? It seems we share the same idea. I don't really see why we'd need any filter in that case though :)

[ockham]

@ockham Just came here after creating #27321 Is this what you have in mind? It seems we share the same idea.

Yep, sounds like we did have the same idea -- yay for alignment!

I don't really see why we'd need any filter in that case though :)

We don't -- the issue title doesn't really apply to this idea anymore.

I'll close this issue in favor of #27321.