Make Editor and Frontend reflect Block Styles in theme.json

[ockham]

Based on @c4rl0sbr4v0's #3401, and @oandregal's suggestion.

Description

Currently, individual block styling defined in a theme's theme.json isn't being applied on the frontend (see https://core.trac.wordpress.org/ticket/56736 or the testing instructions below).

While working on WordPress/gutenberg#44434 (which is somewhat related), we've identified WP_Theme_JSON_Resolver's caching of theme data as the problem. That caching happens early and sanitizes block-specific styling found in theme.json against the list of registered blocks. However, this can happen before all blocks are registered. As a consequence, some styling is scrapped.

This can be fixed by removing the caching from WP_Theme_JSON_Resolver::get_theme_data(), and instead freshly computing theme data each time that method is called, which will take into account all blocks registered at that point.

In order to prevent performance from being impacted all too much, we add caching to the JSON file reading and processing in WP_Theme_JSON_Resolver::read_json_file() instead (per this suggestion).

Testing instructions

To test, activate the TT2 theme. In a new post, insert some button blocks and view on the frontend. They should be rectangular, with a dark green background (rather than round and black).

Before (broken)	After (fixed)

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

---

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.

[ockham]

cc/ @andrewserong @oandregal @ndiego @c4rl0sbr4v0 for review

[ockham]

As a follow-up, we could consider caching WP_Theme_JSON_Resolver::translate() for some more performance gains. (We might wanna use a checksum generated from its $theme_json arg as cache key.)

Alternatively, we could explore introducing a new, cached, method that calls both read_json_file and translate after each other, since those seem to almost always occur in sequence.

[cbravobernal]

LGTM and works as expected 🚀

[andrewserong]

This is testing well for me, too, and it looks like it nicely implements the suggestion from @oandregal. Good idea using an array and storing the parsed file by $file_path key 👍

✅ TwentyTwentyTwo button styling is now output correctly
✅ Saving changes to global styles still works as expected in the site editor, and overrides values set set at the theme's theme.json
✅ Visually confirmed the code now reduces the scope of caching for get_theme_data to just read_json_file, via moving the caching to that function

Alternatively, we could explore introducing a new, cached, method that calls both read_json_file and translate after each other, since those seem to almost always occur in sequence.

I like that idea. But in principle I think this fix is a really good one to go with — it reduces the caching to what we know we can safely cache, and opens the path to incrementally adding caching in future follow-ups, where it'll be possible to tease apart the reasoning as to which pieces require caching, and what the potential implications might be.

LGTM! ✨

[oandregal]

We're missing one path here: if the decoded file is not an array.

[oandregal]

We also need to remove the caching in get_core_data and get_user_data. I'm looking at pushing changes to this PR. I have them locally, have to coordinate with Bernie for permissions.

Edit: the changes are pushed and this is ready.

[aristath]

LGTM 👍

[dream-encode]

Merged into core in https://core.trac.wordpress.org/changeset/54399.

[ockham]

Unfortunately, it seems like this introduced a major performance issue after all: WordPress/gutenberg#44772 (comment) 😕

[oandregal]

As I mentioned WordPress/gutenberg#44772 (comment) it'd be good to have some before/after numbers (ms) as to know whether this is something we need to act for 6.1 or can be tackled after. Ari mentioned he's into that (my setup is malfunctioning).