Merge pull request 'docs(website): settle Phase 3's shape, correct the library build, record slice 0' (#135) from docs/module-phase3-plan into main

Reviewed-on: #135
This commit is contained in:
2026-08-11 06:43:13 +00:00
2 changed files with 167 additions and 14 deletions

View File

@@ -663,27 +663,33 @@ owns the paths it calls, which is correct: it owns the routes at the other end.
### 3.6 Vite library-mode build
The module's `vite.config.js`, and the four externals are the whole contract:
The module's `vite.config.js`, and the shared-dependency aliases are the whole contract:
```js
export default defineConfig({
plugins: [react()],
plugins: [react(), assertSharedNotBundled()],
resolve: {
// ARRAY form with ANCHORED regexes. The object form does PREFIX matching, so
// a `react` key silently also rewrites `react/jsx-runtime`.
alias: [
{ find: /^react$/, replacement: shim('react') },
{ find: /^react\/jsx-runtime$/, replacement: shim('jsx-runtime') },
{ find: /^react\/jsx-dev-runtime$/, replacement: shim('jsx-runtime') },
{ find: /^react-dom$/, replacement: shim('react-dom') },
{ find: /^react-dom\/client$/, replacement: shim('react-dom') },
{ find: /^react-router-dom$/, replacement: shim('react-router-dom') },
],
},
build: {
lib: { entry: 'src/entry.jsx', formats: ['es'], fileName: () => 'entry.js' },
outDir: 'dist',
modulePreload: { polyfill: false }, // same reason as core: no inline bootstrap under CSP
rollupOptions: {
external: ['react', 'react-dom', 'react-dom/client', 'react-router-dom'],
output: { paths: { /* rewritten to window.__rg by the shim below */ } },
},
rollupOptions: { external: [] }, // deliberately empty — see below
},
})
```
Rollup's `external` alone emits bare `import 'react'` specifiers, which the browser cannot resolve
without an import map — and CSP forbids the inline `<script type="importmap">` that would provide
one (`MODULE_SYSTEM.md` §1.14). The module therefore ships a two-line shim module that re-exports
from the global, and aliases the four externals to it:
Each aliased specifier resolves to a two-line shim that re-exports from the global:
```js
// src/shim/react.js
@@ -691,9 +697,35 @@ export default window.__rg.react
export const { useState, useEffect, useMemo, useCallback, useRef, createElement, Fragment } = window.__rg.react
```
**This is the highest-risk mechanical detail in the whole plan and it is exactly what the Phase 1
spike exists to prove.** If it does not hold, §2.6 of the design of record is wrong and the client
half needs rethinking before Phase 2 builds on it.
**Corrected 2026-08-11, Phase 3 slice 0: `external` and the aliases do not compose, and this section
used to show both.** Rollup asks `external` *before* Vite's alias resolver runs, so a specifier
listed there is marked external and never aliased. The chunk then emits bare `import 'react'`
specifiers, which the browser cannot resolve without an import map — and CSP forbids the inline
`<script type="importmap">` that would provide one (`MODULE_SYSTEM.md` §1.14). Slice 0 shipped with
both, built cleanly, and emitted exactly that chunk. So: **alias only, and `external` empty.**
What `external` was there to guard is real — an alias that misses means a second React welded into
the chunk, which loads fine and then throws about an invalid hook call somewhere unrelated. That is
guarded instead by a **resolution-time plugin that fails the build if a shared dependency resolves
into `node_modules`**. Two things about it are contract, because both were wrong first:
- **It hooks `transform`, not `load`.** `load` is first-wins, so an earlier plugin returning the
module's contents means the guard is never called for it. Written against `load` it sat in the
build doing nothing, and a deliberately-broken alias produced a 24 kB chunk with react-router
bundled and a green build.
- **Its forbidden-package list is stated, not derived from the alias list.** Deriving it "so the two
cannot disagree" means deleting an alias also deletes the guard against what that alias prevented
— which is exactly when it is needed. What may not be bundled is a fact about `window.__rg`; a
test asserts the aliases stay inside it.
The module's own boundary checks are `scripts/checkImports.js` (§5.1) and `scripts/checkExternals.js`,
which asks the **built chunk** whether any bare specifier survived. That question cannot be asked of
source: `import { useState } from 'react'` is correct in every file, and which React it becomes is
decided here.
**This is the highest-risk mechanical detail in the whole plan.** The Phase 1 spike proved the
approach; slice 0 proved the configuration, in a browser, under the enforced `script-src 'self'`,
by checking each imported binding is **identity-equal** to the one core published.
---
@@ -817,6 +849,25 @@ specifiers in the client bundle are the four declared externals. A hit fails the
Phase 3's acceptance criterion 1: no `shard`, `uoLink`, `cliloc`, `atlas` or `towncrier` outside
`modules/`, as a CI grep test rather than a review promise.
**Settled 2026-08-11: the grep reads code, not prose.** It covers four things, and each of them is a
thing a module owns:
1. **File and directory names** under `server/src/`, `server/scripts/`, `server/db/` and `client/src/`.
2. **Import and require specifiers** — the path in `require('…')` / `from '…'`.
3. **Route path literals** — the string arguments to `.get`/`.post`/`.put`/`.patch`/`.delete`/`.use`.
4. **Declared identifiers** — function, const, class and property names.
It does **not** read comments or string content generally, and that is not a loophole. Core's
marketing copy legitimately says "shard" — `About.jsx`, `Screenshots.jsx`, `SiteFooter.jsx`,
`heroLayout.js` — and a literal word grep would turn each of those into a CI failure while proving
nothing about the boundary. Worse, it would forbid a core comment from ever using the word as an
example, which is the sort of rule people work around rather than obey. The boundary this test exists
to defend is *structural*: core must not name a module's files, import them, route to them, or
declare their symbols. It can talk about them in English.
Core's UO-flavoured default copy is dealt with directly instead, as `MODULE_SYSTEM.md` §2.7.1's
slice 8 — a rewrite with its own review, not an exemption.
### 5.3 Zero-line route manifest diff (CI, both repos)
`npm run routes:manifest -- --check` in core; the module generates and freezes its own manifest in
@@ -824,6 +875,16 @@ its own repo, using the same script pointed at a core+module app. Phase 2 must p
diff in core's; Phase 3 moves the UO entries out of core's and into module-uo's, which is the one
diff the whole workstream is allowed.
**Settled 2026-08-11: `module-uo`'s CI checks core out at a pinned ref.** The module's workflow
clones `RunicGateway/website` at a ref recorded in the module repo, drops itself in as `modules/uo`,
and runs core's own `routeManifest.js`. Nothing else proves the URLs a module claims are the URLs it
actually serves — a manifest frozen by hand goes stale silently, and the failure it would have caught
is a route that moved.
Pinning the ref rather than tracking `edge` is what keeps this from being a source of unexplained red
Xes: core moves for reasons that have nothing to do with the module, and a bump is then a deliberate
commit that says which core the module was last proved against.
---
## Part 6 — Amendments to MODULE_SYSTEM.md

View File

@@ -746,7 +746,95 @@ Acceptance, all four required:
ownership move, which changes no URL. After extraction the core manifest no longer contains UO
routes — `module-uo` generates and freezes its own in its own repo.
4. **A written `module-rust` dry run** — manifest, mounts, nav entries, one notification stream — not
implemented, to prove the contract generalises before more is built on it.
implemented, to prove the contract generalises before more is built on it. It lands as
`docs/modules/rust-dryrun.md`, where §2.10 already aggregates module documentation; Phase 5's
Integration Kit links to it rather than copying it, per the kit's own never-re-specify rule
(§2.11).
#### 2.7.1 Phase 3's shape — settled 2026-08-11
Measured against `edge` at the close of Phase 2, the surface is **72 server files / ~9,700 lines**,
**51 client files / ~3,700 lines**, and **32 of core's 82 server test files**. The counts in the
paragraph above were written in Phase 0 against a smaller tree and are superseded by the slice table
below.
**The finding that sets the order: the two halves are independent.** Because §1.2 preserves API URLs
exactly, core's client keeps calling `/api/v1/public/shard/status` after that route is served by the
module, and a module page calls the same URL while core still serves it. Nothing forces a feature's
server and client halves to move together, so the extraction is **server-first, then client**, sliced
by feature — which keeps each PR inside one layer and one review's worth of context.
**Merge order across the two repos: `module-uo` first, then `website`.** The loader's `ownedByCore`
probe means a module cannot *load* while core still owns its prefix — but `module-uo`'s own CI never
loads it into core, so its PR merges perfectly well beforehand. Taking that order means `edge` serves
the feature from core right up to the moment core drops it, and there is never a window where the
branch is missing a feature outright. The reverse order would break `edge` at every slice boundary
for the length of a review. Verification is unaffected either way: a slice is proved by running the
*pair* together locally — the module branch checked out into `website/modules/uo`, the deletion
branch checked out in `website/` — before either merges.
Each slice is one `module-uo` PR (adds), one `website` PR (deletes), and one `docs` PR:
| # | Slice | Moves |
| --- | --- | --- |
| 0 | **The bundle skeleton** | `module.json`, both `package.json`s, `server/index.js` registering nothing, the Vite library build + the four shared-dep shims, an empty `schema.sql`/`purge.sql`, CI armed, the §5.1 zero-internal-imports check. `website` untouched. |
| 1 | **Atlas + clilocs** | `shardAtlas`/`shardClilocs` models, `clilocParse`/`clilocSource`/`spawnAtlasParse`/`spawnAtlasSource`, `public/atlas.*`, `admin/shardAtlas.controller`/`shardClilocs.controller`, `scripts/importSpawnAtlas.js`, the art JSON |
| 2 | **The live shard** | `shardState`/`shardEvents`/`shardVisibility`/`uoLinkConfig` models, `uoLinkClient`/`uoLinkSocket`/`shardIngest`/`shardBroadcast`/`shardPush`/`shardVisibility` utils, `config/shardStreams.js`, `public/shard.*`, `admin/shard.router`/`shardOps`/`shardVisibility`/`uoLink.*` |
| 3 | **Market** | `shardMarket` model, `shardSales` util and their routes |
| 4 | **Account links + the extension slot** | `shardLinks` model, `player/shard.*`, `admin/usersShard.*` minus `getUser` — the first real filling of `admin.users.detail` |
| 5 | **The news leg** | `newsGump.js`, `shardAnnounce.js`, the town-crier `registerAnnounceLeg` |
| 6 | **Public pages** | `Shard`, `ShardActivity`, `Rules`, `Atlas`, `AtlasCreature`, `ChampSpawns`, `Market`, `MarketVendor`, `Governors`, `Guilds`, `Houses`, `Leaderboards`, `PlayersOnline`, `VendorSales`, `data/cityCrests.js`, `lib/shardEvents.js`, `lib/useShardFeed.js`, and their public nav rows — under `/uo/*` per §2.8 |
| 7 | **Admin + player pages** | `ShardAdmin`, `ShardOps`, `ShardVisibility`, `SpawnAtlas`, `HousesAdmin`, `AdminCharacter(s)`, `PlayerCharacter(s)`, `GameAccounts`, `CharacterSheet`, `CharacterStats`, `ShardAccountActions`, `CreateGameAccountForm`, `useShardFeatures` and the `useShardFlags` feature provider — under `/admin/uo/*` and `/player/uo/*` |
| 8 | **De-UO core's copy** | `About`, `Screenshots`, `Website`, `SiteFooter`, `heroLayout`'s defaults, `api/client.js`'s `shard`/`atlas` namespaces, and the comments in `navOverrides.js` — plus the §5.2 CI grep that keeps them out |
| 9 | **Close the phase** | `module-uo`'s frozen route manifest and release workflow; `docs/modules/uo/` and `docs/modules/rust-dryrun.md` |
**Criterion 1 is a grep over code, not over prose** — see [API §5.2](MODULE_API.md#52-zero-uo-identifiers-in-core-ci-website-repo)
for what that means precisely. Core's marketing copy says "shard" in a dozen places, and a literal
word grep would have made every one of them a CI failure while proving nothing about the boundary.
Slice 8 rewrites that copy anyway, because a core that still reads as a UO site is not the
game-agnostic platform this workstream is for — but it is a deliberate piece of work with its own
review, not an exemption hidden in a grep pattern.
**One small gap in the kit, deliberately not closed.** The UO client views import almost exactly the
seven §3.4 members — plus `lib/format.js`, a pure leaf formatter. The module **vendors a copy**
rather than core adding an eighth member: the kit is closed on purpose, and a function with no
props and no layout cannot drift the way a component can. The same is not true of `PublicLayout`,
which is why that one is in the kit.
#### Slice 0 — the bundle skeleton (Module-uo#2, 2026-08-11)
`module.json`, an entry point taking `(ctx, api)`, the Vite library build, four shims, and both
boundary checks. It **registers nothing**, and core is untouched — what it proves is the delivery
path itself, before a single UO file moves into it. 29 server tests and 9 client tests, both new.
Verified against a real core rather than asserted: the module loads, mounts its zero routes, reaches
`started` and is published by `/api/v1/public/modules`; its chunk serves from the entry's directory
with `Cache-Control: no-cache` while its server source, `module.json` and `package.json` all 404;
and in Chrome, under the enforced `script-src 'self'`, the chunk reports every shared dependency
**identity-equal** to core's, with zero CSP reports.
**Three findings, each of which had produced a green build that was wrong.** The first amends the
contract and is written up at [API §3.6](MODULE_API.md#36-vite-library-mode-build): `external` and
the aliases do not compose, so `external` is now empty and a resolution-time build guard replaces
it. The second is that guard's own two failures — hooking `load` (first-wins, so it never ran) and
deriving its forbidden list from the alias list (so deleting an alias deleted the guard). Both were
found by breaking an alias on purpose and checking the build actually went red, which is the only
way a guard's absence is visible.
The third is about the boundary check itself and generalises past this repo. **`checkImports.js`
failed on its own documentation** — the comment naming `require("../../etc/passwd")` as an example
of what to catch, and the entry point's comment explaining why a module must never
`require('express')`. A check that cannot survive being described is one people stop writing
comments around, so it strips comments and template literals with a character walk rather than a
regexp (a URL in a string contains a comment opener; a comment contains quotes) and carries its own
test suite. The same applies to slice 8's §5.2 grep, which will be read by a codebase that discusses
modules constantly.
**One thing to know before running a module locally: the loader skips a *symlinked* module directory
silently.** `readdirSync(…, { withFileTypes: true }).filter(e => e.isDirectory())` reports a Windows
junction as a symlink, so a module linked rather than copied into `modules/` is simply not there,
with nothing logged. Not a defect for a real install — `modules/` is a bind mount of real
directories (§2.5) — but it is the first thing to check when a module fails to appear.
**Phase 4 — Delivery.** The admin-panel Modules screen (install, enable, disable, retry, purge,
`startup_failed` with its recorded reason) and the Docker-environment path from §2.5. Deliberately
@@ -896,3 +984,7 @@ row for it — when it has content, not while it is an empty repo.
| 11 | Website work lands on `edge` and reaches `main` as one cutover at the end | §2.9 |
| 12 | The module repo is `RunicGateway/Module-uo`; the module id is `uo` | §2.3 |
| 13 | `RunicGateway/Integration-kit` is the module-builder's instruction book — module + sidecar + game plugin, teaching only, never re-specifying a contract | §2.11 |
| 14 | Phase 3 extracts **server-first, then client**, sliced by feature; `module-uo` merges before `website` in each pair | §2.7.1 |
| 15 | Criterion 1's grep reads **code, not prose**; core's UO copy is rewritten in its own slice instead | API §5.2, §2.7.1 |
| 16 | `module-uo`'s CI checks core out at a **pinned ref** to generate its frozen route manifest | API §5.3 |
| 17 | The `module-rust` dry run lands as `docs/modules/rust-dryrun.md`; the Integration Kit links to it | §2.7.1, §2.11 |