Add wp_get_theme_data_template_parts: create public API to access templateParts from theme.json

[oandregal]

Part of WordPress/gutenberg#45171
Trac ticket: https://core.trac.wordpress.org/ticket/59003

What

This PR adds a new public function wp_get_theme_template_part_metadata to offer access to metadata stored under templateParts in theme.json.

Why

There is a no public function to get access to that data, so consumers need to resort to private APIs. This follows the steps of wp_get_global_settings, wp_theme_has_theme_json, or wp_get_theme_directory_pattern_slugs.

How

It creates a new function that uses the private calls and substitutes all instances of the old one in the codebase.

Performance

This proves to be a small improvement (less than 5%) in the tests performed.

Tested loading a single post (hello world) with a site configured as production (WP_DEBUG, SCRIPT_DEBUG, WP_DEBUG_LOG, WP_DEBUG_DISPLAY to false).

Taking advantage of the new function, it adds a cache following the steps of similar functions such wp_get_global_settings or wp_get_global_stylesheet. This results in a reduction of calls to WP_Theme_JSON_Resolver::get_theme_data from 15 to 8.

Trunk	Branch

In terms of benchmarking with "production" sites, I've done the same test using different tools for comparison.

• benchmark-server-timing

npm run research -- benchmark-server-timing -p --url http://localhost:8889/2023/08/08/hello-world/ -n 100

Theme	Trunk	This PR	Improvement
TwentyTwentyThree	165.69ms	168.51ms	-2.82ms (1.70% faster)
TwentyTwentyOne	84.66ms	85.49ms	-0.82ms (0.98% faster)

• benchmark-web-vitals

npm run research -- benchmark-web-vitals -p --url http://localhost:8889/2023/08/08/hello-world/ -n 100

TTFB

Theme	Trunk	This PR	Improvement
TwentyTwentyThree	228.3ms	225.95ms	-2.35ms (1.02% faster)
TwentyTwentyOne	126.1ms	125.85ms	-0.25ms (0.19%faster)

LCP

Theme	Trunk	This PR	Improvement
TwentyTwentyThree	314.9ms	307.15ms	-7.75ms (2.46% faster)
TwentyTwentyOne	199.55ms	200.45ms	+0.9ms (0.45% slower)

LCP-TTFB

Theme	Trunk	This PR	Improvement
TwentyTwentyThree	92.2ms	92.4ms	+0.2ms (0.21% slower)
TwentyTwentyOne	85.95ms	84.45ms	-1.5ms (1.74% faster)

• curl

seq 100 | xargs -Iz curl -o /dev/null -H 'Cache-Control: no-cache' -s -w "%{time_starttransfer}\n" http://localhost:8889 | awk '{sum+=$1} END {print sum/NR}' | pbcopy

Theme	Trunk	This PR	Improvement
TwentyTwentyThree	154.60ms	152.471ms	-2.3ms (1.37% faster)
TwentyTwentyOne	65.89ms	65.68ms	-0.21ms (0.32% faster)

How to test

Using the TwentyTwentyThree theme:

1. Go to "Appearance > Editor > Patterns" and click on "Template Parts > General".
2. Verify that there are two template parts whose titles are "Comments" and "Post meta".

And then:

1. Go and edit the theme.json of the theme by removing the "post-meta" template part declared in templateParts.
2. Go to "Appearance > Editor > Patterns" and click on "Template Parts > General".
3. Verify that there are two template parts whose titles are "Comments" and "post-meta" (note how the title is taken from the template part file and not from theme.json).

Commit message

Themes: add wp_get_theme_data_template_parts function.

Adds a new public function, `wp_get_theme_data_template_parts` that returns the `templateParts` defined by the active theme from `theme.json`. It also substitutes the usage of private APIs by this new API.

Props felixarntz.
Fixes #59003

[oandregal]

This is what the perf tests report:

Baseline	This PR

In the past, I was told the PR's perf job was not reporting data for the own PR, so I am not sure I should trust these numbers? @felixarntz Should I still run my benchmark locally?

[joemcgill]

@oandregal the benchmarks reported in the screenshots that you've pasted are comparing WP 6.1.1 (the baseline) with your specific commit, so it's not measuring the impact of this specific change. This limitation is something we've got a ticket to address, but have not done so yet, so taking an individual benchmark if you can would be helpful. Thanks!

[felixarntz]

Thanks @oandregal, that looks like a great enhancement! Curious to see the benchmark results.

I left two points of feedback below.

[oandregal]

@felixarntz @joemcgill this is ready: created the trac ticket, provided testing instructions and a performance report.

Performance-wise, this has less impact than I originally thought. It is still valuable, though. Specially, because that the main reason for this PR is to introduce a public method to retrieve data from theme.json (consumers have to use private APIs so far).

[oandregal]

Side-notes about performance report:

• It is still costly to prepare one. Would you think https://core.trac.wordpress.org/ticket/58358 could be prioritized? It's a bottleneck to analyze PRs manually. As a result, we do less pre-merge performance analysis. For example, in this PR, where it was not strictly necessary, I've spent a few hours.

• I'm surprised about the differences depending on the method. Not the numbers themselves, but the ratio for block vs classic themes. Codevitals reports server time as almost equal for classic and blocks while the other methods report classic themes being 2x faster than blocks (curl is more pessimistic and lies in 2.3x with benchmark-* tools being more optimistic around 1.9x).

[felixarntz]

Thanks @oandregal, looks good to go now code-wise, except one small docs inaccuracy.

[felixarntz]

@oandregal

I'm surprised about the differences depending on the method. Not the numbers themselves, but the ratio for block vs classic themes. Codevitals reports server time as almost equal for classic and blocks while the other methods report classic themes being 2x faster than blocks (curl is more pessimistic and lies in 2.3x with benchmark-* tools being more optimistic around 1.9x).

I have no idea, but that's definitely something we should research more. I wonder whether there's a notable difference in how the two environments set up the test sites? Otherwise, there's a good chance it is because the automated workflow used for Codevitals uses a headless browser, while benchmark-server-timing does not (similar to curl). I wonder if the headless browser setup adds some overhead that skews the data somehow - or vice versa?

[oandregal]

🤔 I just realized the test data in use by CI is different from the “dummy” content that comes by default. I'd need to use that dataset to know whether it affected these tests.

[oandregal]

I'm surprised about the differences depending on the method.

I just tried compare-wp-performance by @swissspidy and this is the result of running ./run.sh latest trunk.

wp-total (p50) for classic themes is at 20.88ms while it sits at 31.64ms for block themes, a 1.5x (unlike the ~1x of codevitals and ~2x of curl and benchmark-* tools). The little tools disagree with each other.

[oandregal]

@felixarntz btw, all your feedback has been incorporated. Would you 🟢 this when you have the time, so I can continue follow-up work?

[felixarntz]

Thanks @oandregal!

[felixarntz]

@oandregal Related to this change, @swissspidy shared some relevant feedback on how creating more and more individual wrapper functions for theme.json data is not exactly a sustainable approach, see Slack message. Wouldn't it be more reasonable to consider implementing caching directly into WP_Theme_JSON_Resolver, or implement a wrapper class that comes with caching?

[oandregal]

@oandregal Related to this change, @swissspidy shared some relevant feedback on how creating more and more individual wrapper functions for theme.json data is not exactly a sustainable approach, see Slack message. Wouldn't it be more reasonable to consider implementing caching directly into WP_Theme_JSON_Resolver, or implement a wrapper class that comes with caching?

Happy to re-evaluate caching, though, we need the individual functions: they are the public API. Given the discussion around caching doesn't modify the API, I'm going to commit this to continue follow-up work. Please, let the conversation continue.

So, caching: the resolver already caches data. The issue is that it has to invalidate some of it once per request, resulting in some expensive processes having to run twice. I'm looking into changing how it works to make it run once. Take get_core_data as an example: the next image show all its callees to the right. They are called twice when they should have been called once:

By introducing the public APIs (and caching "at the edge" when it makes sense, such as for settings, styles, templateParts, etc.) I'm hoping to be able to decouple some of the internal processes that theme.json classes do, removing the unnecessary ones depending on the data being accessed, making sure caches are use effectively, etc.

Hope this helps?

[swissspidy]

A class can be a public API too, we don‘t have to add functions for everything. My concern was mostly that I‘m worried we‘ll end up creating X new functions each with its own caching wrapper around that class, leading to poor API design and lots of repetition
Hope that helps!

[oandregal]

A class can be a public API too, we don‘t have to add functions for everything.

I know. Though, note that WP_Theme_JSON and WP_Theme_JSON_Resolver classes are indeed private, which means we cannot ask the consumers to use them for accessing data (normal use case).

My concern was mostly that I‘m worried we‘ll end up creating X new functions each with its own caching wrapper around that class, leading to poor API design and lots of repetition

I understand, those are valid concerns I also have. For reference, this is the plan. Note that the plan is not to have a micro-method for everything. The plan is to have five methods. One per top-level keys that can exists in a theme.json:

{
  "settings": {},
  "styles": {},
  "templateParts": {},
  "customTemplates": {},
  "patterns": {}
}

And the public API:

• wp_get_global_settings(). Shipped in 5.9.0.
• wp_get_global_styles(). Shipped in 5.9.0.
• wp_get_theme_directory_pattern_slugs. Shipped in 6.3.
• wp_get_theme_data_template_parts(). Added by this PR.
• wp_get_theme_data_custom_templates(). TBD.

The rationale for this is that each piece has its own requirements (translated or not translated, how sanitization works, etc.). By having these methods, we can improve how things work by refactoring the private classes/processes as needed without breaking backwards compatibility.

Does this overview alleviate some of your concerns?

[oandregal]

Commited at https://core.trac.wordpress.org/changeset/56385