Answer stability round 1: 0 🔴, 4 🟡 (0 misread); drop the null built-in result, pin the owner's views and locale on the landing slots
This commit is contained in:
@@ -13,7 +13,7 @@ const LIMITS = { lines: 50, complexity: 20 };
|
|||||||
// Each function over a limit, at its measurement. A ceiling follows its function down, and a metric
|
// Each function over a limit, at its measurement. A ceiling follows its function down, and a metric
|
||||||
// leaves the entry once it fits, so a function that shrank can't grow back.
|
// leaves the entry once it fits, so a function that shrank can't grow back.
|
||||||
const grandfathered: Record<string, { lines?: number; complexity?: number }> = {
|
const grandfathered: Record<string, { lines?: number; complexity?: number }> = {
|
||||||
"src/http/app.ts createApp": { lines: 135, complexity: 26 },
|
"src/http/app.ts createApp": { lines: 133, complexity: 26 },
|
||||||
"src/auth/flow-view.ts buildFlowView": { complexity: 24 },
|
"src/auth/flow-view.ts buildFlowView": { complexity: 24 },
|
||||||
"src/auth/hydra-admin.ts createHydraAdmin": { lines: 87 },
|
"src/auth/hydra-admin.ts createHydraAdmin": { lines: 87 },
|
||||||
"src/auth/kratos-admin.ts createKratosAdmin": { lines: 68 },
|
"src/auth/kratos-admin.ts createKratosAdmin": { lines: 68 },
|
||||||
|
|||||||
@@ -78,12 +78,15 @@ test("a plugin that owns a landing page, or short-circuits a hook, translates fr
|
|||||||
mkdirSync(join(dir, "demo", "i18n"), { recursive: true });
|
mkdirSync(join(dir, "demo", "i18n"), { recursive: true });
|
||||||
t.after(() => rmSync(dir, { force: true, recursive: true }));
|
t.after(() => rmSync(dir, { force: true, recursive: true }));
|
||||||
writeFileSync(join(dir, "demo", "i18n", "en-US.ts"), 'const m = { "demo.hello": "Hello from the plugin" };\nexport default m;\n');
|
writeFileSync(join(dir, "demo", "i18n", "en-US.ts"), 'const m = { "demo.hello": "Hello from the plugin" };\nexport default m;\n');
|
||||||
|
mkdirSync(join(dir, "demo", "views"), { recursive: true });
|
||||||
|
writeFileSync(join(dir, "demo", "views", "landing.ejs"), '<%= t("demo.hello") %>');
|
||||||
|
|
||||||
// Every plugin-owned render path: the public landing, the gated dashboard, and a hook short-circuit.
|
// Every plugin-owned render path: the public landing, the gated dashboard, and a hook short-circuit.
|
||||||
|
// The landings return a view, so the host's choice of views and context shows in the output.
|
||||||
const demo: Plugin = {
|
const demo: Plugin = {
|
||||||
apiVersion: "1.0.0",
|
apiVersion: "1.0.0",
|
||||||
dashboard: (ctx) => ({ html: ctx.t("demo.hello") }),
|
dashboard: () => ({ view: "landing" }),
|
||||||
home: (ctx) => ({ html: ctx.t("demo.hello") }),
|
home: () => ({ view: "landing" }),
|
||||||
hooks: { onRequest: (ctx) => (ctx.url.pathname === "/hooked" ? { html: ctx.t("demo.hello") } : undefined) },
|
hooks: { onRequest: (ctx) => (ctx.url.pathname === "/hooked" ? { html: ctx.t("demo.hello") } : undefined) },
|
||||||
id: "demo",
|
id: "demo",
|
||||||
};
|
};
|
||||||
@@ -100,6 +103,17 @@ test("a plugin that owns a landing page, or short-circuits a hook, translates fr
|
|||||||
assert.equal(await (await fetch(`${url}/dashboard`, { headers: { cookie } })).text(), "Hello from the plugin");
|
assert.equal(await (await fetch(`${url}/dashboard`, { headers: { cookie } })).text(), "Hello from the plugin");
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test("a redirect from a plugin-owned landing keeps the visitor's chosen locale", async (t) => {
|
||||||
|
const demo: Plugin = { apiVersion: "1.0.0", home: () => ({ redirect: "/demo/next" }), id: "demo" };
|
||||||
|
const app = createApp({ i18n: createI18n(await loadI18n()), plugins: [demo] });
|
||||||
|
await new Promise<void>((r) => app.listen(0, r));
|
||||||
|
t.after(() => app.close());
|
||||||
|
const url = `http://localhost:${(app.address() as AddressInfo).port}`;
|
||||||
|
const res = await fetch(`${url}/?locale=sv-SE`, { redirect: "manual" });
|
||||||
|
assert.equal(res.status, 303);
|
||||||
|
assert.equal(res.headers.get("location"), "/demo/next?locale=sv-SE");
|
||||||
|
});
|
||||||
|
|
||||||
test("an error page renders without composing the menu — it exists for when the shell's data is what failed", async (t) => {
|
test("an error page renders without composing the menu — it exists for when the shell's data is what failed", async (t) => {
|
||||||
// The chrome getter is lazy on purpose; a render that reads no chrome must not trigger it, or a
|
// The chrome getter is lazy on purpose; a render that reads no chrome must not trigger it, or a
|
||||||
// broken menu takes the error pages down with it.
|
// broken menu takes the error pages down with it.
|
||||||
|
|||||||
+4
-7
@@ -143,7 +143,6 @@ export function createApp(options: AppOptions = {}): Server {
|
|||||||
const serveHome = async (ctx: RequestContext, csrf: RequestCsrf, contextFor: PluginContextFactory): Promise<RouteResult | PluginOwned> => {
|
const serveHome = async (ctx: RequestContext, csrf: RequestCsrf, contextFor: PluginContextFactory): Promise<RouteResult | PluginOwned> => {
|
||||||
csrf.setCookie();
|
csrf.setCookie();
|
||||||
if (!homePlugin) return { data: { chrome: ctx.chrome, user: ctx.user }, view: "home" };
|
if (!homePlugin) return { data: { chrome: ctx.chrome, user: ctx.user }, view: "home" };
|
||||||
// The plugin owns this page, so it runs on its own context — its catalog first, then core.
|
|
||||||
const pluginCtx = contextFor(homePlugin.id);
|
const pluginCtx = contextFor(homePlugin.id);
|
||||||
return { ctx: pluginCtx, pluginId: homePlugin.id, result: (await homePlugin.home(pluginCtx)) ?? null };
|
return { ctx: pluginCtx, pluginId: homePlugin.id, result: (await homePlugin.home(pluginCtx)) ?? null };
|
||||||
};
|
};
|
||||||
@@ -154,7 +153,7 @@ export function createApp(options: AppOptions = {}): Server {
|
|||||||
// The page carries the Sign-out form, so Set-Cookie a fresh CSRF token here when absent.
|
// The page carries the Sign-out form, so Set-Cookie a fresh CSRF token here when absent.
|
||||||
csrf.setCookie();
|
csrf.setCookie();
|
||||||
if (!dashboardPlugin) return { data: { chrome: ctx.chrome }, view: "dashboard" };
|
if (!dashboardPlugin) return { data: { chrome: ctx.chrome }, view: "dashboard" };
|
||||||
const pluginCtx = contextFor(dashboardPlugin.id); // as serveHome: the owner's own translator
|
const pluginCtx = contextFor(dashboardPlugin.id);
|
||||||
return { ctx: pluginCtx, pluginId: dashboardPlugin.id, result: (await dashboardPlugin.dashboard(pluginCtx)) ?? null };
|
return { ctx: pluginCtx, pluginId: dashboardPlugin.id, result: (await dashboardPlugin.dashboard(pluginCtx)) ?? null };
|
||||||
};
|
};
|
||||||
|
|
||||||
@@ -168,9 +167,8 @@ export function createApp(options: AppOptions = {}): Server {
|
|||||||
|
|
||||||
const host: Host = { anyRequestHooks, anyResponseHooks, assets, authOptions, builtinRoutes, csrfSecret, i18n, jwks, keto, kratos, kratosAdmin, menu, permissionCatalog, pluginViewsFor, plugins, secureCookies, settingsCatalog, system, viewsFor };
|
const host: Host = { anyRequestHooks, anyResponseHooks, assets, authOptions, builtinRoutes, csrfSecret, i18n, jwks, keto, kratos, kratosAdmin, menu, permissionCatalog, pluginViewsFor, plugins, secureCookies, settingsCatalog, system, viewsFor };
|
||||||
|
|
||||||
// Static files, the canonical-host redirect and prepareRequest come first; then the answering
|
// A stage that returns a boolean returns true once it has answered the request, and false to
|
||||||
// stages in order — onRequest hook, plugin route, built-in route, else 404/405. A stage that
|
// pass the request to the next stage.
|
||||||
// returns a boolean returns true once it has answered the request, and false to pass it on.
|
|
||||||
const handleRequest = async (req: IncomingMessage, res: ServerResponse, reqLog: Log): Promise<void> => {
|
const handleRequest = async (req: IncomingMessage, res: ServerResponse, reqLog: Log): Promise<void> => {
|
||||||
// Error pages can render before this request has a context at all (a throw on the way to one),
|
// Error pages can render before this request has a context at all (a throw on the way to one),
|
||||||
// so they start on the built-in English and switch to the visitor's locale once it is resolved.
|
// so they start on the built-in English and switch to the visitor's locale once it is resolved.
|
||||||
@@ -358,8 +356,7 @@ async function answerBuiltinRoute(host: Host, state: RequestState): Promise<bool
|
|||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
const answer = await builtin.handler(ctx, csrf, contextFor);
|
const answer = await builtin.handler(ctx, csrf, contextFor);
|
||||||
if (answer && "pluginId" in answer) {
|
if ("pluginId" in answer) {
|
||||||
// A plugin-owned landing slot answers as a plugin route does: its hooks observe it, its views render it.
|
|
||||||
if (host.anyResponseHooks) await runResponseHooks(host.plugins, contextFor, answer.result);
|
if (host.anyResponseHooks) await runResponseHooks(host.plugins, contextFor, answer.result);
|
||||||
await sendResult(ctx.res, answer.result, host.pluginViewsFor(answer.ctx, answer.pluginId), carryLocale);
|
await sendResult(ctx.res, answer.result, host.pluginViewsFor(answer.ctx, answer.pluginId), carryLocale);
|
||||||
} else {
|
} else {
|
||||||
|
|||||||
@@ -4,7 +4,7 @@ import { test } from "node:test";
|
|||||||
import assert from "node:assert/strict";
|
import assert from "node:assert/strict";
|
||||||
import { type BuiltinRoute, matchBuiltinRoute } from "./builtin-route.ts";
|
import { type BuiltinRoute, matchBuiltinRoute } from "./builtin-route.ts";
|
||||||
|
|
||||||
const entry = (method: "GET" | "POST", path: string): BuiltinRoute => ({ handler: () => null, method, path });
|
const entry = (method: "GET" | "POST", path: string): BuiltinRoute => ({ handler: () => ({ html: "" }), method, path });
|
||||||
|
|
||||||
test("matches exact path + method; GET also answers HEAD", () => {
|
test("matches exact path + method; GET also answers HEAD", () => {
|
||||||
const get = entry("GET", "/error");
|
const get = entry("GET", "/error");
|
||||||
|
|||||||
@@ -1,8 +1,6 @@
|
|||||||
// The host's internal route table: each built-in endpoint (the auth/OAuth2 group, the landing
|
// The host's internal route table: each built-in endpoint (the auth/OAuth2 group, the landing
|
||||||
// slots, /error) is a named handler with the plugin RouteHandler shape, plus the request's CSRF
|
// slots, /error) is a named handler with the plugin RouteHandler shape, plus the request's CSRF
|
||||||
// mint (host-only — a plugin reads the token via ctx.chrome instead). app.ts matches this table
|
// mint (host-only — a plugin reads the token via ctx.chrome instead).
|
||||||
// after plugin routes — exact path, a GET route also answering HEAD like the plugin router — and
|
|
||||||
// sends the result against the core views, or against the owning plugin's where it is a PluginOwned.
|
|
||||||
import type { Gate } from "../auth/gate.ts";
|
import type { Gate } from "../auth/gate.ts";
|
||||||
import type { RequestContext } from "./context.ts";
|
import type { RequestContext } from "./context.ts";
|
||||||
import type { RouteResult } from "../plugin-host/plugin.ts";
|
import type { RouteResult } from "../plugin-host/plugin.ts";
|
||||||
@@ -20,8 +18,8 @@ export interface RequestCsrf {
|
|||||||
// own context — otherwise the plugin's keys render as bare keys on the pages it owns.
|
// own context — otherwise the plugin's keys render as bare keys on the pages it owns.
|
||||||
export type PluginContextFactory = (pluginId: string) => RequestContext;
|
export type PluginContextFactory = (pluginId: string) => RequestContext;
|
||||||
|
|
||||||
// A landing slot's result when a plugin owns it: the host renders it against that plugin's views
|
// What a landing slot returns when a plugin owns it. The host renders `result` with that plugin's
|
||||||
// on `ctx`, and shows it to the response hooks like a plugin route's result.
|
// views and `ctx`, and passes it to every plugin's onResponse hook, as it does for a plugin route.
|
||||||
export interface PluginOwned {
|
export interface PluginOwned {
|
||||||
ctx: RequestContext; // the owner's own context, from PluginContextFactory
|
ctx: RequestContext; // the owner's own context, from PluginContextFactory
|
||||||
pluginId: string;
|
pluginId: string;
|
||||||
@@ -29,8 +27,7 @@ export interface PluginOwned {
|
|||||||
}
|
}
|
||||||
|
|
||||||
export interface BuiltinRoute extends Gate {
|
export interface BuiltinRoute extends Gate {
|
||||||
// Returns a RouteResult, or null when the handler wrote to ctx.res itself.
|
handler: (ctx: RequestContext, csrf: RequestCsrf, contextFor: PluginContextFactory) => Promise<RouteResult | PluginOwned> | RouteResult | PluginOwned;
|
||||||
handler: (ctx: RequestContext, csrf: RequestCsrf, contextFor: PluginContextFactory) => Promise<RouteResult | PluginOwned | null> | RouteResult | PluginOwned | null;
|
|
||||||
method: "GET" | "POST"; // a GET route also answers HEAD, like plugin routes
|
method: "GET" | "POST"; // a GET route also answers HEAD, like plugin routes
|
||||||
path: string; // exact pathname
|
path: string; // exact pathname
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user