From 898209725a662ff4f855bd6dff8ce9bb70981bf8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Padez?= Date: Tue, 1 Sep 2026 07:17:33 +0100 Subject: [PATCH] a plugin owns the routing under its mount MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reverts the manifest route table. Declaring a plugin's URLs to the platform put the platform in the middle of a decision that is not its own, and it was more code than what it replaced. One splat where there were two routes. A splat matches zero segments, so the bare route still works, and the plugin reads its own position off the location. The platform owns where a plugin starts and nothing after it — the same line mountPrefix already draws around its API. Co-Authored-By: Claude Opus 5 (1M context) --- src/apps/officer-web/App.tsx | 66 ++++++------------- .../Screens/Dashboard/PluginScreen.tsx | 11 +--- src/servers/plugins/discover.ts | 56 +--------------- src/servers/plugins/generate.ts | 30 +-------- src/servers/plugins/manifest.ts | 62 +---------------- 5 files changed, 24 insertions(+), 201 deletions(-) diff --git a/src/apps/officer-web/App.tsx b/src/apps/officer-web/App.tsx index 46bddd7f..3c0bd7cf 100644 --- a/src/apps/officer-web/App.tsx +++ b/src/apps/officer-web/App.tsx @@ -97,56 +97,28 @@ export function App() { 404 on a route that worked yesterday reads as the feature having been removed. */} } /> {/* Installed plugins. Core routes above stay hand-written; everything below is generated from - what is installed, because a bundler cannot follow a runtime import specifier. The wildcard - hands the whole subtree to the plugin's own router, which react-router nests natively. */} - {/* One pair per SCREEN, not per plugin — `screenId` is the key, because a plugin with two + what is installed, because a bundler cannot follow a runtime import specifier. */} + {/* One route per SCREEN, not per plugin — `screenId` is the key, because a plugin with two screens (caldav: Calendar and Contacts) contributes two entries sharing one `appName`. */} - {installedPlugins.flatMap((plugin) => - /* - A plugin that declared a route table gets exactly those routes. + {/* + A SPLAT, so the whole subtree is the plugin's. - It REPLACES the pair below rather than adding to it: the plugin has said what its URLs - are, so a guessed `/:section` alongside them would be a route nothing links to, matching - ahead of a declared one and rendering the wrong layout. + It was a pair — the bare route and `/:section` — which gave a plugin exactly one segment and + made the platform the author of what it meant. Anything deeper had to go in the query + string, and `/fofos-admin/layout/header` was not expressible at all. - The layout key carries the path pattern, so two routes arranged differently remember - their own dividers. A route on the screen's own layout keeps the screen's own key — its - sizes are the ones the user already set. - */ - plugin.routes.length - ? plugin.routes.map((route) => ( - - } - /> - )) - : [ - } - />, - // The section pair, exactly as the core screens do it (`/soulseek/:section`): the - // plugin's panels read `useParams` themselves, so which section is open is the URL - // rather than state passed between them. - } - />, - ], - )} + A splat matches zero segments too, so this is one route where there were two, and the + plugin reads its own position off the location. Routing inside a plugin is the plugin's, + which is the same line `mountPrefix` already draws around its API: the platform owns where + a plugin STARTS and nothing after it. + */} + {installedPlugins.map((plugin) => ( + } + /> + ))} } /> } /> {/* TEMPORARY / EXPERIMENTAL — offline file transfer over animated QR codes */} diff --git a/src/apps/officer-web/Screens/Dashboard/PluginScreen.tsx b/src/apps/officer-web/Screens/Dashboard/PluginScreen.tsx index 24e8aaae..a59038ec 100644 --- a/src/apps/officer-web/Screens/Dashboard/PluginScreen.tsx +++ b/src/apps/officer-web/Screens/Dashboard/PluginScreen.tsx @@ -21,7 +21,6 @@ export function PluginScreen({ screenId, panels, layout, - layoutKey, }: { /** * This SCREEN's id, not the plugin's — `caldav/calendar`, not `caldav`. @@ -33,14 +32,6 @@ export function PluginScreen({ screenId: string; panels: AppRegistryMeta[]; layout: LayoutNode; - /** - * What to persist sizes under, when this route's arrangement is not the screen's default. - * - * A plugin with a route table can give one route a different layout, and two arrangements sharing one - * key would each remember the other's dividers — which reads as a layout that will not stay put. The - * route's own path pattern is the suffix, so it is stable across every URL that matches it. - */ - layoutKey?: string; }) { // Per-user and per-plugin, so two plugins never share a layout and a user's arrangement is their own. // @@ -48,7 +39,7 @@ export function PluginScreen({ // `layout` on every render, while only the sizes are the user's and persisted. That is what lets a plugin // ship a new layout and have it reach people who have already opened the screen — and it is what stops // the `appTypes` repair below from being written back as though the user had chosen it. See the hook. - const workspace = useAuthoredLayout(`screens/plugin/${layoutKey ?? screenId}`, layout); + const workspace = useAuthoredLayout(`screens/plugin/${screenId}`, layout); const allowed = panels.map((panel) => panel.key); return ( diff --git a/src/servers/plugins/discover.ts b/src/servers/plugins/discover.ts index 80e994c8..b62e9b4d 100644 --- a/src/servers/plugins/discover.ts +++ b/src/servers/plugins/discover.ts @@ -1,5 +1,5 @@ import { existsSync, readdirSync, statSync } from 'node:fs'; -import { join, sep } from 'node:path'; +import { join } from 'node:path'; import { PLATFORM_DIR } from '../data-path'; import { manifestProblems, @@ -93,14 +93,10 @@ function findWeb( `Every plugin route renders a Workspace: contribute panels and a layout, not a screen.`, }; } - const problems: string[] = []; - const routes = routesOf(manifest?.routes, webDir, layout, problems); - if (problems.length) return { web: null, error: `${appName}: ${problems.join('; ')}.` }; - return { web: { screens: [ - { segment: null, label: manifest?.label ?? appName, icon: manifest?.icon, art: null, panels, layout, routes }, + { segment: null, label: manifest?.label ?? appName, icon: manifest?.icon, art: null, panels, layout }, ], // Optional and unannounced — presence is the declaration, as it is for panels and layout. fileActions: fileOrNull(join(webDir, 'file-actions.ts')), @@ -149,8 +145,6 @@ function findWeb( art: fileOrNull(join(dir, 'assets', `${screen.segment}.png`)), panels, layout, - // On the screen entry, not the manifest root: a route belongs to the screen it renders inside. - routes: routesOf(screen.routes, join(webDir, screen.segment), layout, problems), }); } @@ -168,52 +162,6 @@ function findWeb( return { web: { screens, fileActions: fileOrNull(join(webDir, 'file-actions.ts')) } }; } -/** - * A screen's route table, resolved against the plugin directory. - * - * ── Every layout is checked HERE ── - * - * A route naming a file that is not there would be a build error in generated code — a stack trace - * pointing at `Plugins.gen.tsx`, which nobody wrote. Refused at discovery it is a sentence naming the - * plugin and the missing file, which is the same trade every other check in this file makes. - * - * A path is not validated beyond being a string: it is react-router's syntax, not ours, and a - * half-understood copy of their grammar would refuse patterns that work. - */ -function routesOf( - declared: { path: string; layout?: string }[] | undefined, - webDir: string, - fallbackLayout: string, - problems: string[], -): { path: string; layout: string }[] { - const seen = new Set(); - const routes: { path: string; layout: string }[] = []; - - for (const route of declared ?? []) { - // Two entries on one path is one of them never matching, silently — react-router takes the first. - if (seen.has(route.path)) { - problems.push(`route ${JSON.stringify(route.path)} is declared twice`); - continue; - } - seen.add(route.path); - - if (!route.layout) { - routes.push({ path: route.path, layout: fallbackLayout }); - continue; - } - // Under web/, like everything else a screen declares. `join` on a traversal would leave the plugin - // directory, so the resolved path is checked to still be inside it. - const resolved = join(webDir, route.layout.replace(/^web\//, '')); - if (!resolved.startsWith(webDir + sep) || !existsSync(resolved)) { - problems.push(`route ${JSON.stringify(route.path)} names layout ${JSON.stringify(route.layout)}, which is not in web/`); - continue; - } - routes.push({ path: route.path, layout: resolved }); - } - - return routes; -} - const fileOrNull = (path: string): string | null => (existsSync(path) ? path : null); /** diff --git a/src/servers/plugins/generate.ts b/src/servers/plugins/generate.ts index f93bff1c..61d43012 100644 --- a/src/servers/plugins/generate.ts +++ b/src/servers/plugins/generate.ts @@ -96,25 +96,8 @@ export function generatePluginsModule(states: PluginState[]): string { // would accept the other's panels. const screenId = screen.segment ? `${plugin.appName}/${screen.segment}` : plugin.appName; const route = screen.segment ? `/${screen.segment}` : mountPrefix(plugin); - - /* - The screen's route table, each entry carrying the layout it renders. - - A route whose layout is the screen's own reuses `${alias}Layout` rather than importing the same - module twice — the specifier would be identical, and two imports of one file under two names is - two objects where the layout key assumes one. - */ - const routeEntries = screen.routes.map((route_, index) => { - let layoutRef = `${alias}Layout`; - if (route_.layout !== screen.layout) { - layoutRef = `${alias}Route${index}Layout`; - imports.push(`import { defaultLayout as ${layoutRef} } from '${specifier(route_.layout)}';`); - } - return `{ path: ${JSON.stringify(route_.path)}, layout: ${layoutRef} }`; - }); - entries.push( - ` { appName: '${plugin.appName}', screenId: '${screenId}', route: '${route}', panels: ${alias}Panels, layout: ${alias}Layout, routes: [${routeEntries.join(', ')}] },`, + ` { appName: '${plugin.appName}', screenId: '${screenId}', route: '${route}', panels: ${alias}Panels, layout: ${alias}Layout },`, ); } } @@ -150,17 +133,6 @@ export type GeneratedPlugin = { panels: AppRegistryMeta[]; /** How they are arranged. The shell renders \`WorkspaceView\` around them. */ layout: LayoutNode; - /** - * The screen's own route table, when it declared one. - * - * Empty means the screen takes the pair every plugin has always had — \`/\` and - * \`//:section\`. A table replaces that pair entirely: the plugin has said what its URLs are, - * so a guessed one alongside them would be a route nothing points at. - * - * \`path\` is react-router's, relative to \`route\`. It is also the LAYOUT KEY suffix, so two routes - * with different arrangements remember their own dividers. - */ - routes: { path: string; layout: LayoutNode }[]; }; export const plugins: GeneratedPlugin[] = [ diff --git a/src/servers/plugins/manifest.ts b/src/servers/plugins/manifest.ts index befa66a3..95913df1 100644 --- a/src/servers/plugins/manifest.ts +++ b/src/servers/plugins/manifest.ts @@ -323,30 +323,7 @@ export type PluginManifest = { * third parties to `/p//` — so a third-party plugin declaring `screens` is refused. Without * that check this field would be a way for any installed plugin to shadow a core route. */ - screens?: { segment: string; label: string; icon?: string; routes?: PluginRoute[] }[]; - - /** - * The routes THIS screen has, when its sections deserve to be URLs rather than a query string. - * - * For the flat single-screen plugin. A plugin declaring `screens` puts its table on the screen entry - * instead, because a route belongs to the screen it renders inside. - * - * ── Why a table and not a wildcard ── - * - * Every plugin screen already gets `/` and `//:section` — enough to put a section in the - * path and nothing more. What it cannot express is a SECOND segment (`/layout/header`, `/pages/home`) - * or the thing that follows from it: a route arranging its panels differently from its neighbour. - * - * The platform could have guessed a deeper wildcard, and that is the version this replaces. Guessing - * gives every plugin the same shape whether it wants it or not, and gives none of them a way to say - * which arrangement belongs to which URL. A declared table says both, in the plugin's own words. - * - * ── Absent means unchanged ── - * - * No table and the screen gets the pair it always had. That is every plugin but this one, and the - * reason nothing else had to change when this arrived — the same property `screens` was designed with. - */ - routes?: PluginRoute[]; + screens?: { segment: string; label: string; icon?: string }[]; /** * Paths under this plugin's own mount that are served ABOVE the account gate. @@ -675,35 +652,6 @@ export type DiscoveredPlugin = { } | null; }; -/** - * One route within a screen: a path, and optionally its own arrangement of the screen's panels. - * - * ── The path is react-router's, appended to the screen's own ── - * - * `'layout/:band?'` on a plugin mounted at `/fofos-admin` matches `/fofos-admin/layout` and - * `/fofos-admin/layout/header`. Optional segments are why one entry covers both; a plugin that wants - * them to differ writes two entries and gives them different layouts. - * - * `''` is the index — the bare screen route. - * - * ── The layout is optional, and usually absent ── - * - * Omitted, the route renders the screen's own `web/layout.ts`. That is the common case: a route table - * is worth declaring for the URLs alone, and most screens want one arrangement across all of them. - * - * Named, it is a path under `web/` to a module exporting `defaultLayout` — the same contract - * `web/layout.ts` has, so a per-route layout is a file of the kind the author already writes. - * - * A layout gets its OWN persisted sizes, keyed by this path pattern. Two arrangements sharing one key - * would have each remembering the other's dividers, which reads as a layout that will not stay put. - */ -export type PluginRoute = { - /** React-router path, relative to the screen's route. `''` is the index. */ - path: string; - /** `web/…` module exporting `defaultLayout`. Omitted = the screen's own. */ - layout?: string; -}; - /** * One frontend screen: a route, the panels on it, and how they are arranged. * @@ -740,14 +688,6 @@ export type DiscoveredScreen = { art: string | null; panels: string; layout: string; - /** - * The screen's route table, resolved. - * - * Empty for a screen that declared none, which is what keeps the old pair — `/` and - * `//:section` — the default. Each `layout` here is an absolute path, already checked to - * exist at discovery, so the generator can import it without deciding anything. - */ - routes: { path: string; layout: string }[]; }; /**