Faster and more robust editor asset management

[kean]

What?

The subsequent editor loads are now ~1.3s faster. The first launch is ~1s faster.

There are a few changes compared to the previous implementation:

• Move code that fetches assets (manifest and files) to the new EditorService. The assets manifest is now fetched in parallel with the editor settings.
• Only EditorService now triggers the actual download of the assets. The assets are put into a GutenbergKit/{siteURL}/assets folder.
• Remove EditorAssetsProvider and instead inject the plugin styles and scripts in window.GBKit when launching the editor (the same way it's done for editor settings). It happens only if EditorService verifies that all the required files exist on disk. If not, the plugins are disabled until the next time.
• EditorService reads editor settings and assets manifest in one atomic operation. When new manifest is loaded, it replaces the current one only after fetching all assets for it. Until then, the editor will always use the previous version of the manifest.
• Remove editorSettings from EditorConfiguration. Add them to internal EditorDependencies types fetched by EditorViewController

Minor changes:

• Remove editorAssetsEndpoint parameter from EditorConfiguration
• Make logging a cross-cutting concern, so it could be configured once and would be easier to use across GBK

Planned future changes:

• Cleanup unused assets on launch to make sure the disk usage does not grow
• Improve how and when refresh and assets download is triggered
• Cover with unit tests and generate the same code in Kotlin

Why?

How?

Testing Instructions

Accessibility Testing Instructions

Screenshots or screencast

Before / After

Note: the preparing editor message disappears quicker because "fetch editor settings" is now a step inside EditorViewController.

comparison.mp4

[kean]

This is an existing struct. I made no changes to it – just renamed the file.

[dcalhoun]

Thank you for creating this!

This makes sense to me a high level. It feels like a worthwhile improvement overall, especially given the performance improvements.

I left a few thoughts to consider. Many are for bettering my own understanding.

[dcalhoun]

What is the purpose of this change? Is this merely an identifier of why the configuration is empty?

[kean]

Oh, I added it as part of another changes I later reverted. I'll revert this as well.

[dcalhoun]

Why are we removing editorAssetsEndpoint? I believe it's useful in the future whenever we change the REST API endpoint used, it also allows alternatives for third-parties as needed.

[kean]

I suggest to add it again in the future if it's needed. For now, I kept it simple as EditorService knows what endpoints to use, and the code in the app was duplicating the same endpoint.

[dcalhoun]

Are host apps still able to configure the log levels emitted from the JavaScript logger utility? I believe that to be beneficial.

[kean]

Yes, I still inject the same log level in JS: logLevel: '\(EditorLogger.logLevel.rawValue)',

[dcalhoun]

Any performance implications from adding this? 🤔 I suppose it is no larger than editorSettings, eh?

I wonder if it's worth spreading the properties over the top-level object. I could see one day allowing GBK configuration to control allowedBlockTypes.

[kean]

Any performance implications from adding this? 🤔 I suppose it is no larger than editorSettings, eh?

I wouldn't expect it to be slower than the current implementation, but I already had some second thoughts yesterday. Is it safe to pass these larger strings directly in JS this way? Does it need escaping? Is there a more robust way to do it? Should we pass the whole pre-processed manifest as a separate object? Both settings and manifest seem to have <scripts> and <styles>, is there a better way to inject them? What about allowedBlockTypes? Should the app combine them (for core and plugins)?

[dcalhoun]

I don't believe there is a downside to de-structuring all values up front.

Suggested change

	const gbkit = getGBKit();
	// iOS provides manifest directly in GBKit configuration
	if ( window.webkit && gbkit.manifest ) {
	return gbkit.manifest;
	}
	const { siteApiRoot, editorAssetsEndpoint, authHeader } = getGBKit();
	// Android fallback - fetch from API
	const { siteApiRoot, editorAssetsEndpoint, authHeader } = gbkit;
	const { manifest, siteApiRoot, editorAssetsEndpoint, authHeader } = getGBKit();
	// iOS provides manifest directly in GBKit configuration
	if ( window.webkit && manifest ) {
	return manifest;
	}
	// Android fallback - fetch from API

Alternatively, I don't believe there is a downside to invoking getGBKit() multiple times, as it early returns with the cached value on the window.

Suggested change

	const gbkit = getGBKit();
	// iOS provides manifest directly in GBKit configuration
	if ( window.webkit && gbkit.manifest ) {
	return gbkit.manifest;
	}
	const { siteApiRoot, editorAssetsEndpoint, authHeader } = getGBKit();
	// Android fallback - fetch from API
	const { siteApiRoot, editorAssetsEndpoint, authHeader } = gbkit;
	const { manifest } = getGBKit();
	// iOS provides manifest directly in GBKit configuration
	if ( window.webkit && gbkit.manifest ) {
	return gbkit.manifest;
	}
	// Android fallback - fetch from API
	const { siteApiRoot, editorAssetsEndpoint, authHeader } = getGBKit();

[kean]

I made a few more changes:

• Added unit tests and fixed a couple of issues (it explains most of the line additions)
• Updated EditorService to store processed manifest on disk (it takes around 100ms to generate it)
• Added support for cleaning up orphaned asset files, so we don’t take too much space on disk

With these changes, this whole step now takes 2ms.

I had to switch to a different task today and didn't have time to review the final PR. I'll do it when I have time. It should be good.