Allow disabling autosave post type support

[swissspidy]

This came up while reviewing the font library work, see #6034 (comment). Using 'autosave_rest_controller_class' => 'stdClass', to disable the autosave controller is hacky and I would love to find a better way.

This PR:

Adds autosave support by default if the post type has editor support, to maintain backward compatibility.

Support can be removed by explicitly calling remove_post_type_support().

Avoid unnecessarily registering autosave REST controllers.

Trac ticket: https://core.trac.wordpress.org/ticket/41172

---

This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

[github-actions]

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Core Committers: Use this line as a base for the props when committing in SVN:

Props swissspidy, youknowriad, hellofromtonya, peterwilsoncc.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

[youknowriad]

This gets automatically added to the post type REST API right? Can we confirm that with a test or something? I'm thinking that it's a useful information for the editor to know. I know we've been hardcoding some checks recently about whether to enable or disable autosaves.

[swissspidy]

This gets automatically added to the post type REST API right? Can we confirm that with a test or something? I'm thinking that it's a useful information for the editor to know. I know we've been hardcoding some checks recently about whether to enable or disable autosaves.

There is a supports field in the /wp/v2/types endpoint(s) that can be used to check if autosaves are supported.

This could be used in the isEditedPostAutosaveable selector here:

https://github.com/WordPress/gutenberg/blob/8d94c3bd7af977d998466b56bd773f9b19de8d03/packages/editor/src/store/selectors.js#L600-L611

No need for a hardcoded wp_template check anymore

[peterwilsoncc]

I understand why this needs to be opt-out for back-compat purposes.

What do folks think of allowing developers to opt out by setting supports to false?

register_post_type(
	'no-autosave',
	[
		'public' => true,
		'show_in_rest' => true,
		'supports' => [
			'title',
			'editor',
			'autosaves' => false,
		],
	]
);

At present supports['thing'] = false would actually enable support of the thing, so this would be a technical BC break. However, I think the intent of the code is clear so it may be something that can be introduced relatively safely.

[peterwilsoncc]

I'm wondering if we are overthinking this by adding a new supports item.

How about allowing: register_post_type( 'cpt', [ 'autosave_rest_controller_class' => null ] );?

The patch could then become:

public function get_autosave_rest_controller() {
	if ( ! $this->show_in_rest ) {
		return null;
	}

	if ( null === $this->autosave_rest_controller_class ) {
		return null;
	}
	// ...

Regrettably, setting autosave_rest_controller_class => false actually means: enable the controller and use the default. But null is a new value that can indicate the controller should not be enabled.

It would still need a dev note for plugins outside the repository.

I think the current approach would need some additional changes for add|remove_post_types_support to ensure that adding autosaves only works for posts with the editor and removing support for the editor would also need to remove support for autosaves. Similar to this PR for footnotes.

[swissspidy]

null is better than stdClass, so that would be a step in the right direction. However it doesn‘t solve the discoverability issue for the editor, see #6035 (comment)

Any thoughts on how to do that? Maybe with a _links entry in the API.

But then it would be weird to use the existance of a class to check for feature support, when post type supports is designed for this.

[peterwilsoncc]

Any thoughts on how to do that? Maybe with a _links entry in the API.

A non-embedable links entry makes sense. Otherwise could it check the schema, either at wp-json/wp/v2 or higher up the tree if it's already loaded?

[swissspidy]

I don't think Gutenberg looks at the schemas.

@youknowriad Would checking the post type's _links property in isEditedPostAutosaveable in work? i.e. if there's no autosave endpoint mentioned, then it means it's not supported.

[youknowriad]

In Gutenberg, we don't look either at links or schemas so far in core-data selectors and I personally don't like adding "special cases" for some entities to the code base to retrieve information about a given entity. I see "links" as "schemas" as technical informations about the REST API implementation but not something a component user API should provide.

From a data information architecture, I think this is being part of the "supports" key of the endpoint makes more sense. If I'm fetching a post type through an endpoint, I want to know what it supports and what it doesn't. It seems weird to have to check "links" or "schema" to know what a post type support when we actually have a supports property for said entity.

That said, obviously we can adapt.

[swissspidy]

That is certainly true, yeah. I don't think there's anything cleaner than using supports.

[swissspidy]

Just pinging people again to see whether this is something we'd like to commit in 6.5. Otherwise this would be punted.

[hellofromtonya]

Thanks for the ping @swissspidy.

I don't think there's anything cleaner than using supports.
I too am not seeing a more clean solution than what's being proposed.

I'm not a fan of having only this one support be an opt-out, but I do understand why it has to be. That said, I like @peterwilsoncc's thinking:

At present supports['thing'] = false would actually enable support of the thing, so this would be a technical BC break. However, I think the intent of the code is clear so it may be something that can be introduced relatively safely.

but it seems out-of-scope for this specific ticket. Why? It enhances the supports to allow configuring which ones should not be supported, whereas this ticket is about only autosave. IMO this kind of enhancement needs it own ticket and could happen in the next major.

Back to this specific PR.

The Font Library has an immediate need for this change to avoid shipping the hack. That said, the hack is internal and could be replaced in the next cycle once a full solution / enhancement is ready.

Are there any other immediate needs within the editor?

If no, I'm lean towards punting. Then in the next cycle, add the autosave support plus enhance supports to accept false (as Peter suggested).

[peterwilsoncc]

I'm inclined to punt this for now.

While using stdClass is not ideal, it works for now and I think it would be rushing things to try and get this solved in the time we (ie, those of us tracking this ticket) have between now and the beta release.

If using stdClass going to lock WP Core in to supporting something in to the future then let's try to figure it out.

[swissspidy]

Alright, punting for now 👍

[swissspidy]

@peterwilsoncc WDYT about picking this up again for 6.6? I believe it's ready.

The back compat code is small and required, and the new autosave support allows for a better discoverability in Gutenberg, which is a nice win.

[peterwilsoncc]

In the absence of other ideas (I've certainly had none, anyway) I think this can go in.

I think the inline docs could do with some work to clairfy autosaves are based on the editor support.

[swissspidy]

Committed in https://core.trac.wordpress.org/changeset/58201