fix(admin): resolve plugin admin pages opened at the plugin root#1305
Open
eyupcanakman wants to merge 1 commit into
Open
fix(admin): resolve plugin admin pages opened at the plugin root#1305eyupcanakman wants to merge 1 commit into
eyupcanakman wants to merge 1 commit into
Conversation
The Plugin Manager Settings gear opens a plugin at its root (/plugins/<id>/, an empty route splat), but a plugin that registers its admin page at /settings has no page there, so usePluginPage returned null and the admin showed a Plugin Error 404. Resolve the plugin root to the first registered page, treat /settings and /settings/ as the same path, and share the resolver between page rendering and the sidebar nav. Fixes emdash-cms#281
🦋 Changeset detectedLatest commit: 3ae9d46 The changes in this PR will be included in the next version bump. This PR includes changesets to release 14 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
github-actions
Bot
added
area/admin
area/docs
size/M
review/needs-review
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Fixes plugin admin page resolution so plugins opened at their root (e.g. via Plugin Manager gear) don’t error, and page matching works regardless of trailing slashes.
Changes:
- Add
resolvePluginPagePathresolver with trailing-slash equivalence and root fallback to first registered page. - Update sidebar page visibility checks to use the same resolver.
- Add Vitest coverage and update docs/changeset to reflect new behavior.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| skills/creating-plugins/references/admin-ui.md | Updates reference comment to note trailing-slash tolerant page keys. |
| packages/admin/tests/lib/plugin-context.test.ts | Adds unit tests for the new page-path resolution behavior. |
| packages/admin/src/lib/plugin-context.tsx | Implements shared resolvePluginPagePath and updates usePluginPage to use it. |
| packages/admin/src/components/Sidebar.tsx | Uses resolver when determining whether a configured admin page is available. |
| docs/src/content/docs/plugins/creating-native-plugins/react-admin.mdx | Updates docs to explain trailing slash/root fallback behavior. |
| .changeset/fix-plugin-page-root-resolution.md | Adds a patch changeset describing the fix and behavior change. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+56
to
+62
| /** | ||
| * Resolve a plugin page component by path, treating a trailing slash as equivalent and falling back to the first registered page at the root | ||
| */ | ||
| export function resolvePluginPagePath( | ||
| pages: Record<string, React.ComponentType> | undefined, | ||
| path: string, | ||
| ): React.ComponentType | null { |
|
|
||
| <Aside type="caution"> | ||
| Page paths in `pages` must match the `path` values in `admin.pages`. Widget keys must match the `id` values in `admin.widgets`. | ||
| Page paths in `pages` should match the `path` values in `admin.pages`. A trailing slash is treated as equivalent, so `/settings` and `/settings/` resolve to the same page. Opening a plugin at its root resolves to the first registered page. Widget keys must match the `id` values in `admin.widgets`. |
Comment on lines
+47
to
+49
| it("returns null when the plugin has no pages", () => { | ||
| expect(resolvePluginPagePath(undefined, "/settings")).toBeNull(); | ||
| }); |
| if (match) return match; | ||
| // The Plugin Manager gear opens a plugin at "/plugins/<id>/", so the root | ||
| // falls back to the first registered page when no page is keyed at "/". | ||
| if (path === "/") return Object.values(pages)[0] ?? null; |
@emdash-cms/admin
@emdash-cms/auth
@emdash-cms/auth-atproto
@emdash-cms/blocks
@emdash-cms/cloudflare
@emdash-cms/contentful-to-portable-text
emdash
create-emdash
@emdash-cms/gutenberg-to-portable-text
@emdash-cms/plugin-cli
@emdash-cms/plugin-types
@emdash-cms/registry-client
@emdash-cms/registry-lexicons
@emdash-cms/sandbox-workerd
@emdash-cms/x402
@emdash-cms/plugin-ai-moderation
@emdash-cms/plugin-atproto
@emdash-cms/plugin-audit-log
@emdash-cms/plugin-color
@emdash-cms/plugin-embeds
@emdash-cms/plugin-field-kit
@emdash-cms/plugin-forms
@emdash-cms/plugin-webhook-notifier
commit: |
![emdashbot[bot]](https://avatars.githubusercontent.com/in/3255304?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
This is a clean, well-scoped bug fix that solves a real usability problem: plugin admin pages opened from the Plugin Manager at the root (/plugins/<id>/) would 404 when the plugin registered its page at a sub-path like /settings.
Approach judgment: The right fix. Introducing a shared resolvePluginPagePath resolver that both page rendering (usePluginPage) and sidebar nav-visibility consume keeps the two in sync, which is better than fixing only the router and leaving the sidebar with the old exact-match logic. The trailing-slash equivalence is a sensible, low-surprise addition. Falling back from / to the first registered page is pragmatic and documented.
What I checked:
- The resolver logic (exact match, trailing-slash toggle, root fallback, null handling) — all correct.
- Edge cases: both variants registered (exact wins), empty pages object, undefined pages, non-root unregistered paths, root with a registered
/page. - Call sites:
usePluginPagein the router andresolvePluginPagePathin the sidebar are the only consumers; no stale direct lookups remain. - Test coverage: exact, trailing-slash both directions, both-registered preference, root exact, root fallback, first-of-several, unregistered non-root, and no-pages cases are all covered.
- AGENTS.md conventions: changeset present, no new UI strings (no Lingui impact), no SQL, no Tailwind changes.
- Docs and skill reference updated accurately.
Headline conclusion: The implementation is clean and I found no issues.
github-actions
Bot
added
review/approved
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Closes #281.
Opening a plugin from the Plugin Manager showed a Plugin Error. The Settings gear links to the plugin root (
/plugins/<id>/, an empty route splat), but a plugin that registers its admin page at/settingshas no page at the root, sousePluginPagereturned null and the admin fell through to the Block Kit route, which 404s for a React plugin. The lookup also compared paths by exact key, so/settingsand/settings/were treated as different.Fix:
resolvePluginPagePathnow resolves the plugin root to the first registered page, so opening a plugin lands on its admin page instead of a 404. It also matches a trailing-slash variant, so/settingsand/settings/resolve to the same registration.usePluginPage(page rendering) and the sidebar nav-visibility check share this resolver.Testing: verified in a running admin (
demos/plugins-demo, which wires theapi-testplugin registered only at/test). Before, the Plugin Manager gear opened/plugins/api-testand rendered "Plugin Error: Plugin route not found (404)"; after, it renders the plugin's page.resolvePluginPagePathhas unit coverage for the exact, trailing-slash (both directions), both-registered, root-fallback, and unregistered cases.@emdash-cms/admintypecheck,oxlint, andoxfmtpass.Type of change
Checklist
pnpm typecheckpassespnpm lintpassespnpm testpasses (or targeted tests for my change)pnpm formathas been runmessages.pochanges are included.AI-generated code disclosure