wordpress-mobile / wordpress-mobile/GutenbergKit

Plugins/editorAssetsEndpoint config shape allows silently-ignored states

Open
#494 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
JavaScript
Stars
29
Forks
6
Avg merge
1d 9h
Merged PRs (30d)
41

Description

Summary

EditorConfiguration exposes plugins: Boolean and editorAssetsEndpoint: String? (URL? on iOS) as two independent fields, but only some combinations are meaningful:

plugins editorAssetsEndpoint Behavior
false (any) Endpoint silently ignored — fetchManifest short-circuits
true null Falls back to ${siteApiRoot}wpcom/v2/editor-assets
true set Uses provided endpoint

The (false, set) cell is a footgun. A caller that sets a non-functional endpoint (e.g. on a vanilla self-hosted site that doesn't expose wpcom/v2/editor-assets) is fine today because fetchManifest checks if (!configuration.plugins) return ... empty first, but the configuration object itself doesn't encode that the endpoint is conditional on plugins being enabled. We hit this in WordPress-Android while reviewing a builder change — the builder set the endpoint unconditionally, the URL would 404 on vanilla self-hosted, and the only thing keeping it harmless was the gate inside fetchManifest.

Proposal

Collapse the two fields into a single sealed/enum type so the dependency is encoded in the type:

Kotlin:

sealed class Plugins {
    object Disabled : Plugins()
    data class Enabled(val assetsEndpoint: String? = null) : Plugins()
}

Swift:

public enum Plugins {
    case disabled
    case enabled(assetsEndpoint: URL? = nil)
}

The four-cell table collapses to three meaningful states, and the silently-ignored combination is no longer representable.

Open questions

  • Theme styles. EditorAssetsLibrary's class doc says it backs both plugins and theme styles, but the only gate in fetchManifest is configuration.plugins. If theme styles will eventually consume the same endpoint, an Enabled.assetsEndpoint nested under Plugins is the wrong home — the endpoint should be a sibling field, with each feature gated independently. Worth pinning down before settling on shape.
  • Migration. This is a breaking API change on both platforms. Could be staged: introduce the new type, mark the existing fields @Deprecated, port callers, remove later.

Filing this so we can come back to it — not blocking any work today.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by tracing EditorConfiguration into fetchManifest on both Kotlin and Swift, then read EditorAssetsLibrary's class documentation. Resolve whether theme styles share the endpoint and settle the compatibility approach before changing the public API. Done means the meaningful configuration states, callers, and migration path are agreed and the silently ignored state is no longer representable.

Written by the indexing model from the issue text.

Assessment

Tech stack
kotlin, swift
Domain
api, mobile-dev
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Needs clarification
Newbie friendliness
30/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.