fix(modules): guard the portal's nav icon, resolve MODULES_DIR absolutely
Both found by the §7.7 browser smoke, running the slice-3 pair together, and neither is visible to any test in either repo. `PlayerPortalLayout` rendered `<n.icon />` unguarded while `AdminLayout` guarded its equivalent. `icon` is optional in the nav contract, and every core row in that sidebar has always had one — so the difference cost nothing until a module registered a row without, and then it was not a missing glyph, it was React error #130 and a blank player portal. Guarded now, like its neighbour. `MODULES_DIR` is resolved absolute. `resolveClient` checks containment by comparing an absolute `path.resolve(dir, entry)` against the module directory, so a RELATIVE `MODULES_DIR` — which is what §7.7's own recipe produces when run from `server/` — failed every module with "client.entry escapes the module directory". A perfectly-placed entry, and a message pointing at the module. Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -137,7 +137,13 @@ export default function PlayerPortalLayout() {
|
|||||||
borderLeft: `2px solid ${isActive ? 'var(--accent)' : 'transparent'}`,
|
borderLeft: `2px solid ${isActive ? 'var(--accent)' : 'transparent'}`,
|
||||||
})}
|
})}
|
||||||
>
|
>
|
||||||
<n.icon />
|
{/* Guarded, like AdminLayout's. `icon` is optional in the nav
|
||||||
|
contract (§3.3) and every CORE row here has always had one, so
|
||||||
|
an unguarded `<n.icon />` was fine right up until a module
|
||||||
|
registered a row without — and then it was not a missing glyph,
|
||||||
|
it was React error #130 and a blank portal. Found by the §7.7
|
||||||
|
browser smoke; no DOM-less test can see it. */}
|
||||||
|
{n.icon && <n.icon />}
|
||||||
<span>{n.label}</span>
|
<span>{n.label}</span>
|
||||||
</NavLink>
|
</NavLink>
|
||||||
))}
|
))}
|
||||||
|
|||||||
@@ -45,7 +45,14 @@ const { splitStatements } = require('../utils/sqlStatements')
|
|||||||
const log = require('../utils/logger')('modules')
|
const log = require('../utils/logger')('modules')
|
||||||
|
|
||||||
const REPO_ROOT = path.join(__dirname, '..', '..', '..')
|
const REPO_ROOT = path.join(__dirname, '..', '..', '..')
|
||||||
const MODULES_DIR = process.env.MODULES_DIR || path.join(REPO_ROOT, 'modules')
|
// Resolved absolute, and the `path.resolve` is load-bearing rather than tidy.
|
||||||
|
// `resolveClient` compares an absolute `path.resolve(dir, entry)` against this
|
||||||
|
// directory to check containment, so a RELATIVE `MODULES_DIR` — which is what
|
||||||
|
// anyone following §7.7's smoke recipe from `server/` naturally types — makes
|
||||||
|
// that comparison fail for every module, with the thoroughly misleading
|
||||||
|
// "client.entry escapes the module directory". Found the first time the smoke
|
||||||
|
// was run against a module with a real client half.
|
||||||
|
const MODULES_DIR = path.resolve(process.env.MODULES_DIR || path.join(REPO_ROOT, 'modules'))
|
||||||
|
|
||||||
// One segment, lowercase, no parameters. A module prefix that could contain a
|
// One segment, lowercase, no parameters. A module prefix that could contain a
|
||||||
// `/` or a `:` would let a module reach outside the slot it was given.
|
// `/` or a `:` would let a module reach outside the slot it was given.
|
||||||
|
|||||||
Reference in New Issue
Block a user