The Issue with demo import with WP 6.1

[anarieldesign]

Description

When importing demo content using WP 6.1 styles are not imported correctly although the same works fine in WP 6.0.2.

Imported content displays the theme's default colors and styling instead of the styling set for that particular demo. However, in the "Site Editor" styles are correctly displayed. Sometimes when I exit the Site Editor the styles are then correctly displayed in the front end as well, but sometimes it takes a few tries.

This issue happens with the WordPress default importer as well as our theme's built-in import system.

Step-by-step reproduction instructions

1. install theme (I found this bug while checking our premium Basti theme with WordPress 6.1). If you need a theme file I can share it with you
2. import demo(style) version different from the default demo (style)
3. Imported content displays the theme's default colors and styling instead of the styling set for that particular demo -frontend
4. in the "Site Editor" styles are correctly displayed
5. when I exit the Site Editor the styles are then correctly displayed in the front end as well, but sometimes it takes a few tries or you need to wait for few minutes

Screenshots, screen recording, code snippet

Default WordPress Importer Video:

basti-default.mp4

Basti Demo Importer Video:

basti-importer.mp4

Environment info

• WordPress Version 6.1
• Gutenberg plugin is not installed

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

Is it possible to reproduce this with a default theme as well?

Same is happening with Twenty Twenty Two. I just installed the theme with WordPress 6.0 and choose the pink style variation. Exported the xml file. Installed 6.1 on the new installation, installed Twenty Twenty Two and imported xml file and same thing happens 🤷‍♀️

[annezazu]

cc @scruffian @carolinan @aristath you all in case any of you might be able to investigate this and come up with a possible solution for 6.1. This is definitely a point of concern for block themers for this release and it would be advantageous to have it solved sooner rather than later.

[dream-encode]

Is it possible to reproduce this with a default theme as well?

[anarieldesign]

Is it possible to reproduce this with a default theme as well?

Same is happening with Twenty Twenty Two. I just installed the theme with WordPress 6.0 and choose the pink style variation. Exported the xml file. Installed 6.1 on the new installation, installed Twenty Twenty Two and imported xml file and same thing happens

[scruffian]

cc @WordPress/block-themers

[t-hamano]

I think this issue is the same as #43914 and #43395.

[anarieldesign]

I think this issue is the same as #43914 and #43395.

Hi @t-hamano, I think it's not the same issue as this one is related to importing demo content, xml file. And when u import .xml file, front-end is not showing the right styles (u can wait and wait) till u click on the edit site and later wait for few minutes 🤷‍♀️🤷‍♀️. This is what happend when I did checks. With 6.0 this is not happening. U immidiately see the right styles on the front when u import the demo file. But maybe the same issue is causing both things 🤷‍♀️🤷‍♀️

[anarieldesign]

With 6.0, importing demo content with our Basti theme and default theme works perfectly and u can see styles immidiately applied on the front. In 6.1 it will not apply till u first click Edit Site 🤷‍♀️🤷‍♀️

[seothemes]

Hi @anarieldesign, I think transients could be causing the issue.

Commenting out line 431 of wp-includes/class-wp-theme-json-resolver.php fixes the issue for me.

The name is dynamic, but it should be possible to clear the transient on the after_import hooks in both Merlin and the WP Importer. I'll share what I come up with.

Hopefully there's a way to fix in core so the workaround isn't required.

[seothemes]

That's no longer working, maybe I was on an earlier 6.1 version. Not sure why it worked that time but I'll keep looking into it.

[scruffian]

@ajlende @andrewserong do you think this could have been broken by some of the changes to the caching recently?

[ockham]

cc/ @oandregal @c4rl0sbr4v0 in case this rings a bell since it might be related to WordPress/wordpress-develop#3418.

Note #44946 (comment):

Commenting out line 431 of wp-includes/class-wp-theme-json-resolver.php fixes the issue for me.

but also #44946 (comment):

That's no longer working, maybe I was on an earlier 6.1 version. Not sure why it worked that time but I'll keep looking into it.

[andrewserong]

The note in the PR description that it takes a few minutes for changes to appear points to it being potentially related to transients rather than the change in WordPress/wordpress-develop#3418 (which should be refreshed on each page load?)

Looking through recent commit history in wordpress-develop, I see there's this one WordPress/wordpress-develop#2414 from @spacedmonkey that updated the set_transient logic in get_user_data_from_wp_global_styles (looks like it switched from object cache to transients, and was backported to Gutenberg in #44290). Not sure if that one's at all related, too? Just pinging @oandregal and @draganescu in case this one rings a bell, too 🙂

[anarieldesign]

It's not just that it takes few minutes to appear. When u import the .xml file and u stay on the front-end it will not show the right style till u first click on the edit site. And then u need to wait for few minutes @andrewserong

[spacedmonkey]

Changing to a transit might have effects this. Transient data, even if you don't have object caching enabled. Global styles are not correctly cache invalidated. Caches are not deleted on global style imports.

We may want to remove the transient caching all together and relay on the caching found in WP_Query.

[draganescu]

Reading this looks like we should invalidate any style caching when importing. Also it does not look like https://github.com/WordPress/wordpress-develop/pull/2414/files is causing this because the values were cached before but in a different way. Speculation from reading code, not tested.

[anarieldesign]

When testing the same thing with 6.0 this is not happening, importing content works all good and it shows immidiately on the front. Maybe this can help to see what has been changed in 6.1 that is causing this issue @draganescu

[spacedmonkey]

When testing the same thing with 6.0 this is not happening, importing content works all good and it shows immidiately on the front. Maybe this can help to see what has been changed in 6.1 that is causing this issue @draganescu

Did you test with object caching enabled?

[spacedmonkey]

The issue here is there is no cache invalidation. You dont see this issue on dev environment as object caching is not enabled. Using a transient means it is cached for sites without object caching. This would have always been an issue.

I am CC @peterwilsoncc has he has some context here.

I think the easier fix would be removing this caching all together and relay on WP_Query caching upstream.

[peterwilsoncc]

I think the easier fix would be removing this caching all together and relay on WP_Query caching upstream.

This is my inclination given WP 6.1 has the lower level caching. It saves double handing and a duplicate cache entry/invalidation.

Just so I am clear, the issue is with the database query rather than the optimisation of theme.json -- is that correct?

[peterwilsoncc]

I attempted to use the lower level caching in WordPress/wordpress-develop#3517 but without success. Tests_Theme_wpThemeJsonResolver::test_get_user_data_from_wp_global_styles_does_not_use_uncached_queries fails consistently.

In the loop to test caching, I am getting zero results for the query (thus returning an empty array). I am unsure if I have discovered an existing bug with the WP_Query arguments or if it's something I have introduced in my PR.

Anyone with the required permissions should feel free to push to my branch if you can see where I've done something silly.

[peterwilsoncc]

I've updated WordPress/wordpress-develop#3517 and it is now ready for review.

The test for test_get_user_data_from_wp_global_styles_does_not_use_uncached_queries() was running as an anonymous user, as they do not have permission to create the post type wp_global_styles() nor terms in the wp_theme taxonomy, the post creation was not happening as expected:

• the post would be created
• no term would be added as current_user_can( $taxonomy_obj->cap->assign_terms ) would prevent the user from doing so.

I'm unclear whether the intent is to create styles for logged out users, I have assumed not as the permissions are not set correctly.

Could I please get a review of the PR and some tests on the branch?

[andrewserong]

I've updated WordPress/wordpress-develop#3517 and it is now ready for review.

This looks good to me for fixing up the caching logic!

Testing out that PR revealed one other piece of the puzzle for me. On a site that already has a global style custom post entry for the currently active theme, when we run the importer, if the post id is the same, then the importer sees that there's already a post, and doesn't create a new post for the imported custom global styles data. For example, here is the output from running the importer on a site that already has an entry in the global styles custom post type for the current theme:

Note that last line: Global Styles "Custom Styles" already exists..

I'm not too sure what the correct behaviour here is for the importer logic, as it's outside of my experience so far (I haven't touched the importer logic, and am also not all that familiar with the global styles custom post type logic, so please take my observations here with a grain of salt 🙂).

However, it sounds like something will likely need to change in how importers handle importing the wp_global_styles custom post type. For these, I imagine the desired logic would be that the existing custom post type entry for the current theme would be overwritten by the imported entry, whether or not the id matches?

This might need some input from @jorgefilipecosta, @oandregal or someone who has deeper knowledge of the expected behaviour of the global styles custom post type, and how importing over the existing settings should work.

[peterwilsoncc]

I've created WP#56901 for tracking the commit to WordPress-Develop.

@anarieldesign are you able to link your WordPress.org account to GitHub so you can be included in the props for the ticket? You can do so on your profile https://profiles.wordpress.org/me

I'll keep this ticket open for tracking the changes for the Gutenberg repository.

[anarieldesign]

I've created WP#56901 for tracking the commit to WordPress-Develop.

@anarieldesign are you able to link your WordPress.org account to GitHub so you can be included in the props for the ticket? You can do so on your profile https://profiles.wordpress.org/me

I'll keep this ticket open for tracking the changes for the Gutenberg repository.

Thank u, I connected it now 🙏 @peterwilsoncc

[oandregal]

Looked at WordPress/wordpress-develop#3517 (comment) from a performance point of view and I have seen no performance penalties from removing the caching from the method. I haven't seen any data provided by the PRs that introduced the caching either (#36584 and WordPress/wordpress-develop#2414 ). A personal takeaway is that any PR that claims a performance improvement should provide data for a test case that is actually improved.

[oandregal]

However, it sounds like something will likely need to change in how importers handle importing the wp_global_styles custom post type. For these, I imagine the desired logic would be that the existing custom post type entry for the current theme would be overwritten by the imported entry, whether or not the id matches?

My understanding is that the importer works the way it does to prevent an unexpected data loss.

As it works now, styles work the same as any other post type. If the user wants the styles from the XML and they have some data, they can delete the existing and import. Changing the behavior to always overwrite means that users can lose changes to styles if they import (they need to be aware that styles also come in that XML and whether the source they import from has user changes to styles).

[andrewserong]

Changing the behavior to always overwrite means that users can lose changes to styles if they import (they need to be aware that styles also come in that XML and whether the source they import from has user changes to styles).

Great point, it's easy to overlook things like this when focusing on a single use case (the user who does want to overwrite). I agree, that for the user who wishes to overwrite their own settings, it's up to them to clear out their user styles first. Sounds like there's nothing that needs to change from the importers side, then. Thanks for confirming!

[spacedmonkey]

Looked at WordPress/wordpress-develop#3517 (comment) from a performance point of view and I have seen no performance penalties from removing the caching from the method. I haven't seen any data provided by the PRs that introduced the caching either (#36584 and WordPress/wordpress-develop#2414 ). A personal takeaway is that any PR that claims a performance improvement should provide data for a test case that is actually improved.

To be clear, fallbacking to WordPress core's WP_Query does have a down side. Currently global styles are cached for a day or a hour if found / not found. Meaning, a lookup to global styles will only happen once a day or hour for 99% of sites. But using WP_Query caching, means that when ANY post of any type, including revisions, menu items, embed caches and autosaves, the cache would be invalidated. On high traffic sites, that create a of posts, this mean the cache would be invalidated a lot, resulting lots of queries.

I think removing this cache for 6.1 is a quick work around. But a longer term solution is to cache global styles forever and then correctly cache invalid when a global style post is created/update/delete in the database. Using a last changed date for just global styles as the salt for the cache key might work. But this would need unit tests etc and does seems likely to complete in time we have left in WP 6.1 release cycle.

[spacedmonkey]

It is also worth noting that wp_get_global_stylesheet and wp_get_global_styles_svg_filters also appear to cache global style data in a transient. These too, do not seems to have correct cache invalidation.

[anarieldesign]

Do u have any workaround in case this will not be fixed with 6.1? What we can do as a theme authors? We already sell our block theme with demo imports and it worked perfectly with 6.0 and now is a big mess. We can't tell users, oh please first click on edit site, exit and then wait for 10min or something 🤷‍♀️🤷‍♀️.

Thank u for all your hard work 🙏

[seothemes]

@anarieldesign it should be possible to add a hook after import to clear the transients and global styles.

I haven't tested lately but if this approach works for you then I'd be happy to assist further:

add_action( 'merlin_after_all_import', 'delete_existing_global_styles', 11 );
/**
 * Deletes global style transients after importing demo content.
 *
 * @since 1.0.0
 *
 * @return void
 */
function delete_existing_global_styles() : void {
	$theme = get_stylesheet();

	delete_transient( 'gutenberg_global_styles_' . $theme );

	// Force delete existing custom styles.
	$custom_styles = get_posts( [
		'post_type'  => [ 'revision', 'wp_global_styles' ],
		'post_title' => 'Custom Styles',
	] );

	foreach ( $custom_styles as $custom_style ) {
		wp_delete_post( $custom_style->ID, true );
	}
}

[anarieldesign]

Thank u for sharing this @seothemes 🙏. I'll test this out during the weekend and will let u know 🙏

[dream-encode]

@seothemes In order to make sure you receive props for your work on this PR in the 6.1 release of WordPress, are you able to link your GitHub account to your WordPress.org account?

[hellofromtonya]

Update:

Fixes were merged into WP Core's trunk (see https://core.trac.wordpress.org/changeset/54706) and backported to the 6.1-branch (see https://core.trac.wordpress.org/changeset/54707). They will be released in WP 6.1-RC4 today.

[anarieldesign]

Thank you,I'll check it tomorrow and let u know if I notice anything strange 👌🙏

[seothemes]

@dream-encode thanks for the tip, I have linked my .org account blockify - although I didn't really help much with this one.

[dream-encode]

@seothemes Thanks! You'll now be credited in the final 6.1 release!

[anarieldesign]

Thank you everyone for the help and for the fixes 👍

[cbravobernal]

I guess this has been solved, so we can close the issue. Feel free to reopen if needed!