Merge pull request 'docs(website): settle the module loader's trigger and its collision check' (#127) from docs/module-loader into main
Reviewed-on: #127
This commit is contained in:
@@ -454,6 +454,24 @@ half needs rethinking before Phase 2 builds on it.
|
|||||||
finds (`MODULE_SYSTEM.md` §1.12). The database is not consulted. `MODULES_DIR` defaults to
|
finds (`MODULE_SYSTEM.md` §1.12). The database is not consulted. `MODULES_DIR` defaults to
|
||||||
`<repo>/modules` and is overridable by env for tests and for the Docker volume mount.
|
`<repo>/modules` and is overridable by env for tests and for the Docker volume mount.
|
||||||
|
|
||||||
|
**The trigger is one explicit call, and there is no lazy self-scan** (§7.6). `app.js` calls
|
||||||
|
|
||||||
|
```js
|
||||||
|
modules.load({ public: publicRouter, admin: adminRouter, player: playerRouter })
|
||||||
|
```
|
||||||
|
|
||||||
|
exactly once, and every accessor throws until it has run rather than answering with an empty list —
|
||||||
|
"no modules installed" is a real state, and a caller must not be handed it by accident. Its position
|
||||||
|
in `app.js` is load-bearing in both directions: **after** `app.use('/api', apiRouter)`, so every core
|
||||||
|
prefix is already on the tier routers when §4.3 asks them what core owns and so first-match-wins
|
||||||
|
means a module cannot shadow a core route; **before** the `/api` 404, so a module route reaches its
|
||||||
|
handler instead of the catch-all.
|
||||||
|
|
||||||
|
**Mounting is a second pass** over the modules that survived validation, not part of the scan loop.
|
||||||
|
Otherwise the first module's layers sit on the tier router while the second is being validated,
|
||||||
|
indistinguishable from core's — the second module would be told it collided with *core*, naming the
|
||||||
|
wrong culprit, and the module-versus-module check would be unreachable.
|
||||||
|
|
||||||
### 4.2 Order
|
### 4.2 Order
|
||||||
|
|
||||||
Alphabetical by `id`, deterministically. There is no dependency resolution between modules (§2.0 of
|
Alphabetical by `id`, deterministically. There is no dependency resolution between modules (§2.0 of
|
||||||
@@ -472,6 +490,14 @@ other order would imply a precedence that is not being computed.
|
|||||||
|
|
||||||
A failure at any step is that module's failure and nobody else's.
|
A failure at any step is that module's failure and nobody else's.
|
||||||
|
|
||||||
|
**Step 3 asks the live tier routers, not a list.** Whether core owns a prefix is answered by probing
|
||||||
|
the tier router's own stack with express's `layer.match()`, skipping root-mounted (`fast_slash`)
|
||||||
|
layers — `public/index.js` ends with `use('/', siteRouter)` and `admin/index.js` with the dashboard
|
||||||
|
router, and both match every path, so counting them would report every prefix as taken and no module
|
||||||
|
could ever mount. A hardcoded prefix table was tried in the spike and was already one prefix stale
|
||||||
|
when it was written; deriving it means the check cannot drift the first time core adds a capability
|
||||||
|
router, and needs no second declaration of the mount table.
|
||||||
|
|
||||||
### 4.4 `startup_failed` is a state, not a crash
|
### 4.4 `startup_failed` is a state, not a crash
|
||||||
|
|
||||||
Per `MODULE_SYSTEM.md` §2.4, the loader try/catches the **entire** lifecycle — require, validation,
|
Per `MODULE_SYSTEM.md` §2.4, the loader try/catches the **entire** lifecycle — require, validation,
|
||||||
@@ -690,10 +716,15 @@ They are recorded so nobody reads them as intended shape:
|
|||||||
3. **The module has no `swagger-fragment.json`** — §6.1a's obligation needs core's merge helper on
|
3. **The module has no `swagger-fragment.json`** — §6.1a's obligation needs core's merge helper on
|
||||||
the other side of it, which is Phase 2.
|
the other side of it, which is Phase 2.
|
||||||
|
|
||||||
### 7.6 A finding for Phase 2's loader
|
### 7.6 A finding for Phase 2's loader — **settled: an explicit `load()`**
|
||||||
|
|
||||||
`scan()` is lazy — requiring the loader does not run it. That is deliberate (app.js decides when
|
`scan()` was lazy — requiring the loader did not run it. That was deliberate (app.js decides when
|
||||||
modules are discovered) but it is a sharp edge: a caller that requires the loader and reads nothing
|
modules are discovered) but it is a sharp edge: a caller that requires the loader and reads nothing
|
||||||
gets an empty, *silent* module list. It cost one confusing test failure during the spike. Phase 2
|
gets an empty, *silent* module list. It cost one confusing test failure during the spike.
|
||||||
should either make the trigger explicit in the name or scan at require time and let app.js order the
|
|
||||||
require.
|
**Phase 2 PR 2 made the trigger explicit** rather than scanning at require time. `app.js` calls
|
||||||
|
`modules.load(tierRouters)` once, and `list()` throws until it has. Scanning on require was the
|
||||||
|
alternative and was rejected for two reasons: the loader now needs the tier routers *handed to it*
|
||||||
|
for the §4.3 collision check, which a require-time side effect cannot receive; and it would make the
|
||||||
|
ordering constraint invisible, enforced by where a `require` sits rather than by an argument that is
|
||||||
|
missing if it is wrong.
|
||||||
|
|||||||
@@ -453,9 +453,23 @@ too (API §7.2).
|
|||||||
Exit criterion: `routes.manifest.json` diff is zero lines and every existing test passes. If Phase 2
|
Exit criterion: `routes.manifest.json` diff is zero lines and every existing test passes. If Phase 2
|
||||||
changes one URL, it is wrong.
|
changes one URL, it is wrong.
|
||||||
|
|
||||||
**Progress: PR 1 done** — `installed_modules` and the state machine, with the stored shape and the
|
**Progress: PRs 1-2 done.**
|
||||||
boot rules settled in §2.4 above. No loader, no routes, no boot wiring yet, so it changes nothing an
|
|
||||||
operator or a client can see.
|
- **PR 1** — `installed_modules` and the state machine, with the stored shape and the boot rules
|
||||||
|
settled in §2.4 above.
|
||||||
|
- **PR 2** — `server/src/modules/loader.js`: the filesystem scan, manifest validation, prefix and
|
||||||
|
table-name collision rejection, per-module try/catch and the tier mount, behind the §4.5 dispatch
|
||||||
|
guard. Two decisions landed with it, both recorded in [`MODULE_API.md`](MODULE_API.md): the load
|
||||||
|
trigger is **one explicit `modules.load(tierRouters)` call in `app.js`**, never a lazy scan
|
||||||
|
(API §7.6); and the "does core own this prefix" check **probes the live tier routers** rather than
|
||||||
|
a hardcoded table, so it cannot drift when core adds a capability router (API §4.3). The three
|
||||||
|
de-entanglement registries and the two lifecycle hooks throw `not available until phase 2 PR 4/5`
|
||||||
|
rather than no-op — an accepting stub would let a module believe it had registered something.
|
||||||
|
28 tests, all on the failure paths.
|
||||||
|
|
||||||
|
There is still no module on the volume, no schema replay and no boot wiring, so this changes nothing
|
||||||
|
an operator or a client can see: 842 tests pass and `routes.manifest.json` is unchanged at 229
|
||||||
|
routes.
|
||||||
|
|
||||||
**Phase 3 — Extract `module-uo`.** Moves out of `website/`: the 8 model directories and their 25
|
**Phase 3 — Extract `module-uo`.** Moves out of `website/`: the 8 model directories and their 25
|
||||||
tables; the nine UO `utils/` files plus `newsGump.js`; the 13 router/controller files;
|
tables; the nine UO `utils/` files plus `newsGump.js`; the 13 router/controller files;
|
||||||
|
|||||||
Reference in New Issue
Block a user