diff --git a/Dockerfile b/Dockerfile index 57155c4cd..6d70788d1 100644 --- a/Dockerfile +++ b/Dockerfile @@ -24,9 +24,10 @@ COPY dashboard/package.json dashboard/ COPY packages/shared/package.json packages/shared/ COPY e2e/package.json e2e/ RUN npm ci -# Vite's TypeScript transform follows the app's tsconfig `extends` chain, so the -# build needs this even though nothing here runs tsc. -COPY tsconfig.base.json ./ +# Vite's TypeScript transform follows each workspace's tsconfig `extends` chain +# (the console's and shared's) to these, so the build needs them even though +# nothing here runs tsc. +COPY tsconfig*.json ./ # The console imports from it, so it belongs to the shared layer beneath. COPY packages/ packages/ diff --git a/docs/production-hardening.md b/docs/production-hardening.md index 2c64a7bb7..903e61a36 100644 --- a/docs/production-hardening.md +++ b/docs/production-hardening.md @@ -236,7 +236,8 @@ response shapes and component expectations still surface at runtime. That matters most for the nested structures from `/api/radar/analytics`, `/api/radar/nodes` and the aircraft WebSocket, whose variant-heavy shapes are largely untyped. It can be tightened incrementally: `strictNullChecks` first, -then typing the API shapes file by file. +then typing the API shapes file by file. The e2e suite and `packages/shared` +already compile strictly, by extending `tsconfig.strict.json`. **Priority:** medium **Effort:** moderate; typing every API shape fully is a longer tail diff --git a/e2e/package.json b/e2e/package.json index 19e6d4d13..d02269327 100644 --- a/e2e/package.json +++ b/e2e/package.json @@ -4,7 +4,8 @@ "version": "1.0.0", "type": "module", "scripts": { - "typecheck": "tsc --noEmit", + "typecheck": "tsc --noEmit && npm run collect", + "collect": "for env in staging prod local; do echo \"collecting for $env\"; E2E_ENV=$env playwright test --list > /dev/null || exit 1; done", "test:e2e": "playwright test", "test:e2e:local": "E2E_ENV=local playwright test", "test:e2e:staging": "E2E_ENV=staging playwright test", diff --git a/e2e/playwright.config.ts b/e2e/playwright.config.ts index 6219c4ef3..4438b30a3 100644 --- a/e2e/playwright.config.ts +++ b/e2e/playwright.config.ts @@ -1,4 +1,4 @@ -import { defineConfig, devices } from "@playwright/test"; +import { defineConfig, devices, test } from "@playwright/test"; /** * Playwright E2E test configuration. @@ -31,8 +31,6 @@ import { defineConfig, devices } from "@playwright/test"; * server's admin console. */ -const ENV = (process.env.E2E_ENV ?? "staging") as "staging" | "prod" | "local"; - const HOSTS = { staging: { api: "https://staging-api.retina.fm", @@ -84,8 +82,32 @@ const HOSTS = { }, } as const; +// e2e's collect script lists these environments by name; a new one goes there too. +const isEnv = (name: string): name is keyof typeof HOSTS => Object.keys(HOSTS).includes(name); +const ENV = process.env.E2E_ENV ?? "staging"; +if (!isEnv(ENV)) throw new Error(`E2E_ENV=${ENV} is none of ${Object.keys(HOSTS).join(", ")}`); +const TABLE = HOSTS[ENV]; + export const env = ENV; -export const hosts = HOSTS[ENV]; +// The roles every environment has. The rest are reached through hostOrSkip. +export const hosts: Record<"api" | "map" | "dash", string> = { + api: TABLE.api, + map: TABLE.map, + dash: TABLE.dash, +}; + +/** + * The host for a role that is null on some environment, skipping where it is: + * the file at a spec's top level, the group in a describe or beforeAll, the test + * in a test or beforeEach. Call it from the spec itself: a shared module runs + * once, so only the first spec to import it would skip. Where it skips it + * returns an unroutable stand-in, so top-level code that parses it cannot throw. + */ +export function hostOrSkip(role: Exclude, reason: string): string { + const host = TABLE[role]; + test.skip(host === null, reason); + return host ?? `https://${role}.skipped.invalid`; +} /** * Cloudflare Access service-token headers, empty unless CI supplies both. diff --git a/e2e/specs/app-surface.spec.ts b/e2e/specs/app-surface.spec.ts index f4e537890..2f8dcdb32 100644 --- a/e2e/specs/app-surface.spec.ts +++ b/e2e/specs/app-surface.spec.ts @@ -6,42 +6,32 @@ * router lands where it should. The arrival at /map is react-router's own * redirect, so it needs the JavaScript to have loaded and run. * - * Skipped where hosts.app is null — see the table in playwright.config.ts for + * Skipped where the app role is null — see the table in playwright.config.ts for * why production and the dev server are. */ import { test, expect } from "@playwright/test"; -import { hosts } from "../playwright.config"; +import { hostOrSkip } from "../playwright.config"; -const APP = hosts.app; - -test.skip(APP === null, "no consolidated app surface in this environment"); - -// Only ever read inside a test body. test.skip aborts the tests, not this -// module: every top-level statement here still runs while the file is being -// collected, so touching this at module scope throws on the environments where -// it is null and takes the whole run down with it. -const BASE = APP as string; +const APP = hostOrSkip("app", "no consolidated app surface in this environment"); // A hostname is mostly dots, and an unescaped one matches any character. -function originPattern(): string { - return BASE.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); -} +const ORIGIN = APP.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); test.describe("the consolidated app surface", () => { test("opens on the map", async ({ page }) => { - await page.goto(`${BASE}/`, { waitUntil: "domcontentloaded" }); - await expect(page).toHaveURL(new RegExp(`^${originPattern()}/map`)); + await page.goto(`${APP}/`, { waitUntil: "domcontentloaded" }); + await expect(page).toHaveURL(new RegExp(`^${ORIGIN}/map`)); await expect(page.locator(".connection-badge")).toBeVisible({ timeout: 30_000 }); }); test("sends a private page to the login card", async ({ page }) => { - await page.goto(`${BASE}/overview`, { waitUntil: "domcontentloaded" }); + await page.goto(`${APP}/overview`, { waitUntil: "domcontentloaded" }); await expect(page.locator(".login-card")).toBeVisible({ timeout: 30_000 }); - await expect(page).toHaveURL(new RegExp(`^${originPattern()}/login/?$`)); + await expect(page).toHaveURL(new RegExp(`^${ORIGIN}/login/?$`)); }); test("opens the explorer on the filters its link carries", async ({ page }) => { - await page.goto(`${BASE}/data?from=2026-09-01&to=2026-09-03`, { waitUntil: "domcontentloaded" }); + await page.goto(`${APP}/data?from=2026-09-01&to=2026-09-03`, { waitUntil: "domcontentloaded" }); // The page renders its shareable link from the filters it read, so this // needs the bundle to have run. await expect(page.getByTestId("de-share")).toContainText("from=2026-09-01&to=2026-09-03", { diff --git a/e2e/specs/dashboard.spec.ts b/e2e/specs/dashboard.spec.ts index a5f7c5114..702a90f8c 100644 --- a/e2e/specs/dashboard.spec.ts +++ b/e2e/specs/dashboard.spec.ts @@ -29,7 +29,7 @@ * Authenticated flows are covered via API-level assumptions (see api.spec.ts). */ import { test, expect, request as playwrightRequest, type Page } from "@playwright/test"; -import { hosts } from "../playwright.config"; +import { hosts, hostOrSkip } from "../playwright.config"; // The origin the dashboard is served from, at its root. const DASH = hosts.dash; @@ -37,7 +37,6 @@ const DASH_PAGE = DASH; const LOGIN_PATH = "/login"; // A page that needs a session. The index does not: it forwards to the map. const PRIVATE_PAGE = `${DASH_PAGE}/overview`; -const ADMIN = hosts.admin; const API = hosts.api; type AuthMode = "oauth" | "bypass"; @@ -177,18 +176,18 @@ test.describe("Admin surface selection", () => { // serves, with a different route table, chosen client-side from the // hostname. Null on prod (see playwright.config.ts) so a wobble here cannot // roll production back. - test.skip(!ADMIN, "no admin surface on this environment"); + const admin = hostOrSkip("admin", "no admin surface on this environment"); // The mode is a property of the deployment, not of either test. beforeEach - // runs per test, so cache it; the skip itself has to stay in beforeEach, - // which is where Playwright accepts it. The value is cached rather than the + // runs per test, so cache it; the resolution skip stays in beforeEach, since + // it has to await a request. The value is cached rather than the // promise, so a request that fails leaves the next test free to try again // instead of inheriting a rejection. let authMode: AuthMode | undefined; let adminResolves: boolean | undefined; test.beforeEach(async () => { - adminResolves ??= await resolves(ADMIN!); - test.skip(!adminResolves, `${ADMIN} does not resolve`); + adminResolves ??= await resolves(admin); + test.skip(!adminResolves, `${admin} does not resolve`); authMode ??= await serverAuthMode(); }); @@ -201,8 +200,8 @@ test.describe("Admin surface selection", () => { // run can show is that the vhost is reachable and serving this bundle, which // is why these stay here once enforced auth puts a login card in the way. test("the admin vhost serves the admin console", async ({ page }) => { - await page.goto(ADMIN!); - await expectSurface(page, ADMIN!, authMode!, "Admin Console"); + await page.goto(admin); + await expectSurface(page, admin, authMode!, "Admin Console"); }); test("the app host serves the user dashboard", async ({ page }) => { diff --git a/e2e/specs/env.d.ts b/e2e/specs/env.d.ts index c11119ad6..5c2599c3c 100644 --- a/e2e/specs/env.d.ts +++ b/e2e/specs/env.d.ts @@ -1,4 +1,4 @@ -// Only tsconfig.e2e.json compiles this directory, and it must stay that way: +// Only e2e/tsconfig.json compiles this directory, and it must stay that way: // in the browser program this global would let src code use `process`, which // Vite does not polyfill, so it would typecheck and then throw at runtime. declare const process: { env: Record }; diff --git a/e2e/specs/live-map.spec.ts b/e2e/specs/live-map.spec.ts index ef754f3f2..bc93e3b8e 100644 --- a/e2e/specs/live-map.spec.ts +++ b/e2e/specs/live-map.spec.ts @@ -1,8 +1,8 @@ /** * Live Aircraft Map E2E tests, on the simulation surface. * - * This suite visits the admin console's /sim on whichever host `hosts.testmap` - * names: the local dev server's admin.localhost. It verifies the map page + * This suite visits the admin console's /sim on whichever host the `testmap` + * role names: the local dev server's admin.localhost. It verifies the map page * loads, WebSocket connects, aircraft appear, and key interactive elements work * correctly. * @@ -20,19 +20,12 @@ * build. */ import { test, expect, Page } from "@playwright/test"; -import { hosts } from "../playwright.config"; +import { hosts, hostOrSkip } from "../playwright.config"; -const TESTMAP = hosts.testmap; - -test.skip( - TESTMAP === null, +const SIM = `${hostOrSkip( + "testmap", "no synthetic map surface in this environment (only the test droplet runs a fleet)", -); - -// test.skip aborts the tests, not this module — every top-level statement still -// runs during collection — so nothing here may call a method on TESTMAP where it -// is null. Interpolating it is safe. -const BASE = `${TESTMAP}/sim`; +)}/sim`; // Helper: wait for the connection badge to show "LIVE" async function waitForLive(page: Page, timeoutMs = 15_000) { @@ -107,7 +100,7 @@ async function rowsOrSkip(page: Page) { test.describe("Live Map — page identity", () => { test("the console names the page Simulation Map", async ({ page }) => { - await page.goto(BASE); + await page.goto(SIM); // The header's title rather than the HTML , which is static across // every page of the console. "Simulation Map", not "Live Map": this suite // visits /sim, and DashboardLayout's page-title table names that page for @@ -117,7 +110,7 @@ test.describe("Live Map — page identity", () => { }); test("the console's brand is RETINA, not Tower Finder", async ({ page }) => { - await page.goto(BASE); + await page.goto(SIM); // Text content, not visibility: the map opens with the sidebar collapsed to // its icon rail, which hides the brand's labels. await expect(page.locator(".brand-text")).toHaveText(/RETINA/i); @@ -127,7 +120,7 @@ test.describe("Live Map — page identity", () => { test("no JavaScript errors on page load", async ({ page }) => { const errors: string[] = []; page.on("pageerror", (err) => errors.push(err.message)); - await page.goto(BASE); + await page.goto(SIM); await page.waitForLoadState("networkidle"); expect(errors).toHaveLength(0); }); @@ -135,18 +128,18 @@ test.describe("Live Map — page identity", () => { test.describe("Live Map — map rendering", () => { test("Leaflet map container is present", async ({ page }) => { - await page.goto(BASE); + await page.goto(SIM); await expect(page.locator(".leaflet-container")).toBeVisible({ timeout: 10_000 }); }); test("toolbar is rendered with connection badge", async ({ page }) => { - await page.goto(BASE); + await page.goto(SIM); await expect(page.locator(".live-map-toolbar")).toBeVisible({ timeout: 10_000 }); await expect(page.locator(".connection-badge")).toBeVisible(); }); test("toolbar shows Coverage / Labels / Trails toggle buttons", async ({ page }) => { - await page.goto(BASE); + await page.goto(SIM); await expect(page.locator(".live-map-toolbar")).toBeVisible({ timeout: 10_000 }); // "Coverage gaps" also matches a loose /Coverage/, so this must stay exact. await expect(page.getByRole("button", { name: "Coverage", exact: true })).toBeVisible(); @@ -155,7 +148,7 @@ test.describe("Live Map — map rendering", () => { }); test("Debug Truth toggle is present on the simulation surface", async ({ page }) => { - await page.goto(BASE); + await page.goto(SIM); await expect(page.locator(".live-map-toolbar")).toBeVisible({ timeout: 10_000 }); await expect(page.getByRole("button", { name: /Debug Truth/i })).toBeVisible(); }); @@ -163,19 +156,19 @@ test.describe("Live Map — map rendering", () => { test.describe("Live Map — WebSocket connectivity", { tag: "@live" }, () => { test("connection badge transitions to LIVE within 15s", async ({ page }) => { - await page.goto(BASE); + await page.goto(SIM); await waitForLive(page); await expect(page.locator(".connection-badge")).toHaveClass(/connected/); }); test("aircraft count is non-empty once connected", async ({ page }) => { - await page.goto(BASE); + await page.goto(SIM); await waitForLive(page); await expect(page.locator(".aircraft-count")).toBeVisible(); }); test("Pause button toggles to Resume and back", async ({ page }) => { - await page.goto(BASE); + await page.goto(SIM); await waitForLive(page); const pauseBtn = page.getByRole("button", { name: /Pause/i }); @@ -192,7 +185,7 @@ test.describe("Live Map — WebSocket connectivity", { tag: "@live" }, () => { test.describe("Live Map — aircraft list panel", { tag: "@live" }, () => { test("aircraft list panel renders within 20s of connection", async ({ page }) => { - await page.goto(BASE); + await page.goto(SIM); await waitForLive(page); // Panel should exist after aircraft start arriving @@ -200,7 +193,7 @@ test.describe("Live Map — aircraft list panel", { tag: "@live" }, () => { }); test("aircraft list shows rows once data arrives", async ({ page }) => { - await page.goto(BASE); + await page.goto(SIM); await waitForLive(page); const rows = await rowsOrSkip(page); @@ -208,7 +201,7 @@ test.describe("Live Map — aircraft list panel", { tag: "@live" }, () => { }); test("clicking an aircraft row opens the detail panel", async ({ page }) => { - await page.goto(BASE); + await page.goto(SIM); await waitForLive(page); const row = (await rowsOrSkip(page)).first(); @@ -230,7 +223,7 @@ const SYNTHETIC_IDENTITY = /^(?:synth|e2e|test|realnode)-\S+$/; test.describe("Live Map — node markers", { tag: "@live" }, () => { test("every node marker is a synthetic one", async ({ page }) => { - await page.goto(BASE); + await page.goto(SIM); await waitForLive(page); // `.node-marker` is the divIcon NodeMarkersLayer gives a NON-synthetic @@ -242,7 +235,7 @@ test.describe("Live Map — node markers", { tag: "@live" }, () => { }); test("node popup names a synthetic fleet id, not a real node's ref", async ({ page }) => { - await page.goto(BASE); + await page.goto(SIM); await waitForLive(page); const marker = page.locator(".node-marker-synthetic").first(); @@ -260,7 +253,7 @@ test.describe("Live Map — node markers", { tag: "@live" }, () => { test.describe("Live Map — toolbar toggles", () => { test("Coverage toggle adds/removes active class", async ({ page }) => { - await page.goto(BASE); + await page.goto(SIM); await expect(page.locator(".live-map-toolbar")).toBeVisible({ timeout: 10_000 }); const btn = page.getByRole("button", { name: "Coverage", exact: true }); @@ -272,7 +265,7 @@ test.describe("Live Map — toolbar toggles", () => { }); test("Arcs toggle adds/removes active class", async ({ page }) => { - await page.goto(BASE); + await page.goto(SIM); await expect(page.locator(".live-map-toolbar")).toBeVisible({ timeout: 10_000 }); const btn = page.getByRole("button", { name: /Arcs/i }); @@ -284,7 +277,7 @@ test.describe("Live Map — toolbar toggles", () => { }); test("Labels toggle adds/removes active class", async ({ page }) => { - await page.goto(BASE); + await page.goto(SIM); await expect(page.locator(".live-map-toolbar")).toBeVisible({ timeout: 10_000 }); const btn = page.getByRole("button", { name: /Labels/i }); @@ -298,7 +291,7 @@ test.describe("Live Map — toolbar toggles", () => { const errors: string[] = []; page.on("pageerror", (err) => errors.push(err.message)); - await page.goto(BASE); + await page.goto(SIM); await expect(page.locator(".live-map-toolbar")).toBeVisible({ timeout: 10_000 }); const fitBtn = page.getByRole("button", { name: /Fit/i }); diff --git a/e2e/tsconfig.json b/e2e/tsconfig.json index 7617272d8..6f4e6ba25 100644 --- a/e2e/tsconfig.json +++ b/e2e/tsconfig.json @@ -1,5 +1,5 @@ { - "extends": "../tsconfig.base.json", + "extends": "../tsconfig.strict.json", "compilerOptions": { "types": [] }, diff --git a/packages/shared/tsconfig.json b/packages/shared/tsconfig.json index 564a59900..8524d4248 100644 --- a/packages/shared/tsconfig.json +++ b/packages/shared/tsconfig.json @@ -1,4 +1,4 @@ { - "extends": "../../tsconfig.base.json", + "extends": "../../tsconfig.strict.json", "include": ["src"] } diff --git a/strict-probe.ts b/strict-probe.ts new file mode 100644 index 000000000..3b18cdae0 --- /dev/null +++ b/strict-probe.ts @@ -0,0 +1,21 @@ +// Compiled into every workspace whose tsconfig extends tsconfig.strict.json, +// and imported and run by nothing. Each line under @ts-expect-error has to stay +// a type error, so tsc fails there the day the setting it names stops applying. + +declare const maybe: string | null; + +// strictNullChecks +// @ts-expect-error a value that may be null has no method to call +maybe.toString(); + +// noImplicitAny +// @ts-expect-error a parameter with no type is an implicit any +export const untyped = (value) => value; + +// strict, through useUnknownInCatchVariables +try { + maybe?.toString(); +} catch (error) { + // @ts-expect-error a caught value is unknown + error.toString(); +} diff --git a/tsconfig.strict.json b/tsconfig.strict.json new file mode 100644 index 000000000..247b3ea2e --- /dev/null +++ b/tsconfig.strict.json @@ -0,0 +1,14 @@ +{ + // The base with strict type-checking, for a workspace to extend. Each flag is + // named because the base turns the last two off outright, which `strict` + // alone does not override. strict-probe.ts fails to compile in any workspace + // where one of them stops applying, as long as that workspace declares no + // `files` of its own, which would replace this list. + "extends": "./tsconfig.base.json", + "compilerOptions": { + "strict": true, + "noImplicitAny": true, + "strictNullChecks": true + }, + "files": ["./strict-probe.ts"] +}