Simplify template part block attributes

[carlomanf]

Is your feature request related to a problem? Please describe.
Currently, the template part block supports four attributes: postId, slug, tagName and theme. In light of #26650 being merged, there is now no longer a need for the postId and theme attributes.

The purpose of the theme attribute was to preserve the rendering of user-edited templates that survived a theme switch. Now that all templates are retired from use upon theme switch, there is no longer a need for this attribute.

As for the postId attribute, I think its purpose was to locate template parts that the user created independently of the theme. This idea of theme-less templates/parts is no longer being supported for the time being.

Describe the solution you'd like
Simplify the supported attributes of the template part block to just slug and tagName, as these are now the only ones needed.

The block would then render the template part from the current theme with the specified slug, if it exists. It shouldn't need to look through an inactive theme.

Also related: #25845, #22717

[carlomanf]

Upon further thought, I don't think it would be such a good idea after all to remove the theme attribute. Sorry about that, I'm still getting my head around the recent change that was made in this area.

I still think the postId attribute is superfluous. Given there was recently a patch #26268 to strip this attribute on export, you have to ask why it's there to start with.

[Copons]

Totally missed this issue!
Pinging @Addison-Stavlo @noahtallen @jeyip who should be familiar with the Template Part block.

[Addison-Stavlo]

I don't think postId is superfluous. It references a specific entity. The slug/theme combo is not unique, we can end up in a state where we have 2 template parts with the same slug/theme combo. While the postId isn't needed for a theme template to reference in its markup, once things start being customized this ensures the proper entity is referenced.

[carlomanf]

we can end up in a state where we have 2 template parts with the same slug/theme combo.

Are you sure? Can you give an example?

[Copons]

we can end up in a state where we have 2 template parts with the same slug/theme combo.

Are you sure? Can you give an example?

The sync that automatically converts theme-provided files into templates and template parts auto-drafts, only check if there are already either publish or auto-draft elements.

Reasoning is that only templates and parts with status publish and auto-draft are actually "usable" by the site.

With this in mind, we can end up with slug/theme duplicate draft templates/parts.

See for example what I have on my dev site right now:

(As an aside: I thought trash status was affected as well, but apparently trashed posts' slugs become post-slug__trashed)

[Addison-Stavlo]

we can end up in a state where we have 2 template parts with the same slug/theme combo.

Are you sure? Can you give an example?

One example could be just creating 2 new template parts in the editor. Either through adding the block and creating a new one through the placeholder, or selecting blocks and using the ellipsis menu to create a template part out of them. These template parts will be made with the same slug and be named 'Untitled Template Part', they will also have the same theme term. So currently postId is the only block attribute that can guarantee each template part block is actually referencing the correct entity.

Also with the most recent changes, new template parts that are created will have the active theme's name set as their theme term. So we could find similar examples where a user might name a template part "header", where the theme had created a "header" template part. The template part block is set up to try to find an entity by slug/theme combo alone if there is no postId, this is necessary since themes don't really have any way of knowing what postId a template part will be created as when installed on a users site.

---

I do have some other concerns with the current set up though. The combination of these things does create the potential for some sticky situations if I am understanding the recent changes correctly. Continuing with that last example we now have a theme supplied "header" and a custom (from scratch) created "header" both with the same slug/theme combo. Now the unedited theme template referencing the header only by slug/theme will resolve to reference the custom template part since it prioritizes publish over auto-draft (This was initially the case so that if the user customized the theme supplied template part elsewhere, this customization would show up on all theme supplied templates referencing it). However, referencing the user created/custom template part may not be desired behavior.

When custom template parts were created with a separate value for the theme term, this was not an issue. The only conflict would be between an unedited theme supplied template part and a customized version of that same theme supplied template part, in which case resolving to the customized one made sense. But now we have an issue where a created from-scratch custom template parts could override unedited theme supplied ones when referenced from those base template part blocks that don't have a postId attribute set. This is a reason why I still feel that template parts created from scratch by the user should have a distinct theme term, such as '' or _custom. But I suppose incorporating wp_file_based in that check would solve this. 🤷‍♀️

[carlomanf]

One example could be just creating 2 new template parts in the editor. Either through adding the block and creating a new one through the placeholder, or selecting blocks and using the ellipsis menu to create a template part out of them. These template parts will be made with the same slug and be named 'Untitled Template Part', they will also have the same theme term.

Wouldn't it be better to name it Untitled 2, or require the user to give it a unique name before it's saved, etc?

Also with the most recent changes, new template parts that are created will have the active theme's name set as their theme term. So we could find similar examples where a user might name a template part "header", where the theme had created a "header" template part. The template part block is set up to try to find an entity by slug/theme combo alone if there is no postId, this is necessary since themes don't really have any way of knowing what postId a template part will be created as when installed on a users site.

Again, why not name it header-2, or show an error message that the name is already taken?

[carlomanf]

This may be a bigger issue than I even realised, because there is a core function wp_delete_auto_drafts that flushes auto-drafts that are more than 7 days old. Consequently, the auto-draft template part will be regenerated with a new ID and thereby break any blocks that were referencing its old ID.

I literally just discovered this now with a template part showing up as "Template Part Not Found" because it was referencing an auto-draft by ID that got automatically deleted. Then I removed the ID attribute, left the slug and theme attributes intact and it started working again.

Come to think of it, there are also plugins like Advanced Database Cleaner and others that can flush auto-drafts on-demand, so it's really not a good idea to assume that a template part will always have the same ID.

[Copons]

This may be a bigger issue than I even realised, because there is a core function wp_delete_auto_drafts that flushes auto-drafts that are more than 7 days old. Consequently, the auto-draft template part will be regenerated with a new ID and thereby break any blocks that were referencing its old ID.

I literally just discovered this now with a template part showing up as "Template Part Not Found" because it was referencing an auto-draft by ID that got automatically deleted. Then I removed the ID attribute, left the slug and theme attributes intact and it started working again.

Come to think of it, there are also plugins like Advanced Database Cleaner and others that can flush auto-drafts on-demand, so it's really not a good idea to assume that a template part will always have the same ID.

Ah!
I was kinda right then when I tried pushing for a different post status for auto-generated templates! 😅

So, on one hand we definitely need a way to prevent multiple templates with same slug and theme term — and this regardless of anything else, as it makes it unpredictable which one will be eventually resolved.
On the other, I'm not extremely comfortable with having templates that get erased and recreated for no good reasons, just for the sake of keeping the auto-draft status.

I'm also indeed convinced that while the ID as unique identifier is useful in most cases, templates (and template parts) resolution is not one of them: we only need to rely on slug and theme term.

[Addison-Stavlo]

Wouldn't it be better to name it Untitled 2, or require the user to give it a unique name before it's saved, etc?
Again, why not name it header-2, or show an error message that the name is already taken?

That's how it was originally implemented. It proved to be a bit clunky UX wise, and wasn't necessary technically. After working through some design iterations it was updated to not require unique slugs and no longer append *-# onto everything. Perhaps @MichaelArestad may remember more reasons for why we went this route.

[carlomanf]

@Copons I'm just curious about this comment you made at #27277:

• This PR shouldn't change the behaviour of Site Editor and front-end with one exception: the Template Part block's preview selector doesn't return "theme-agnostic" template parts anymore.
  I'm considering this not an issue, considering we are removing that feature for the foreseeable future in Site Editor: Only request templates and template parts for the current theme #27152 anyway.

Does that mean I was on to something by proposing to remove the theme attribute as well? The idea was to restrict the template part block to only ever render template parts from the active theme.

The reason I reneged is I was thinking of cases where a template part is included in a post or page. These blocks would persist after a theme switch (unlike those inside templates) and would risk unexpected output if they don't specify a theme.

On the other hand, template parts don't really belong in posts and pages, and you'd probably expect them to be hidden from the block inserter, so if a user managed to sneak one in, then perhaps it should be at their own risk?

[carlomanf]

Reverting this issue back to it's original title. I understand the postId attribute is now gone with #27910, and there is discussion on removing the theme attribute as well.

[carlomanf]

thinking of cases where a template part is included in a post or page. These blocks would persist after a theme switch (unlike those inside templates) and would risk unexpected output if they don't specify a theme.

After working on #31971, I'm further convinced by the above argument, so I will go ahead and close this issue.