The megabyte nobody asked for, and the click that now answers

apps/web/src/app/(app)/a-loading-boundary-swallows-the-refusal.test.ts

@@ -0,0 +1,141 @@
+import { readFileSync, readdirSync, statSync } from "node:fs"
+import path from "node:path"
+
+/**
+ * Why the app shell has no `loading.tsx`, written down so nobody adds one.
+ *
+ * ── The problem that wants one ──────────────────────────────────────────────
+ *
+ * It is real and it is not fixed. Every route in this group is `force-dynamic`
+ * and several resolve through many sequential awaits before emitting a byte.
+ * Next keeps the PREVIOUS page fully on screen for that entire time, and the
+ * side nav cannot help either — `usePathname` does not change until the
+ * navigation commits — so on campus wifi the officer who clicked Approvals sees
+ * an unchanged Dashboard and clicks again. `SideNav`'s `useLinkStatus` mark now
+ * answers the click; the content region still does not.
+ *
+ * ── Why the obvious fix cannot ship as-is ───────────────────────────────────
+ *
+ * MEASURED on a standalone Next 15.5.20 app built for this question — two
+ * pages, byte-identical bodies, differing only in whether a sibling
+ * `loading.tsx` existed:
+ *
+ *     notFound()   without a boundary → HTTP 404
+ *     notFound()   WITH a boundary    → HTTP 200   (not-found UI in the body)
+ *     redirect(…)  without a boundary → HTTP 307 + Location
+ *     redirect(…)  WITH a boundary    → HTTP 200, no Location header
+ *
+ * The mechanism is visible in Next's own source. `renderToInitialFizzStream`
+ * awaits `ReactDOMServer.renderToReadableStream`, which resolves at SHELL
+ * ready, and `continueFizzStream` only awaits `allReady` when
+ * `isStaticGeneration` — which a `force-dynamic` route never is. With a
+ * `loading.tsx` the shell is layout + fallback, so it is ready and the 200 is
+ * committed before the page has run. The refusal then arrives inside a boundary
+ * that has already flushed, and app-render's handler never sees it — its own
+ * comment says so: "If a bailout made it to this point, it means it wasn't
+ * wrapped inside a suspense boundary."
+ *
+ * Forty-one pages in this group refuse with `notFound()`, and all but one of
+ * the forty-two use `notFound()` or `redirect()`. A group-level `loading.tsx`
+ * would turn every one of those refusals into a 200. That is not a test
+ * artifact; it is the product answering "you may not see this club" with
+ * "success".
+ *
+ * ── What the real fix needs ─────────────────────────────────────────────────
+ *
+ * The refusal has to be decided ABOVE the boundary — in `(app)/layout.tsx`, in
+ * middleware, or by a route the boundary does not cover — or the wait has to be
+ * expressed some other way (a Suspense boundary INSIDE the page, below its
+ * authorization checks, which is per-page work rather than one file). Either is
+ * a design change, not the one-line file this test forbids.
+ */
+
+const APP_DIR = path.resolve(__dirname, "../../app")
+
+/**
+ * `reports/loading.tsx` predates this and is WRONG in exactly the way described
+ * above — `reports/page.tsx:46` and `reports/finance/page.tsx:34` both call
+ * `notFound()` for a non-OSE viewer, and both therefore answer 200 today. It is
+ * listed rather than deleted because removing it is its own decision with its
+ * own visual consequence, and nothing here measured what /reports looks like
+ * without it. It is the exception, not the precedent.
+ */
+const PRE_EXISTING = ["(app)/reports"]
+
+/**
+ * `path.relative` answers in the host's separator, so this walk yields
+ * `(app)\\reports` on Windows and `(app)/reports` everywhere else — while
+ * `PRE_EXISTING` and every assertion below are written with `/`. CI is
+ * ubuntu-latest, so the mismatch would never show there; it would show as a
+ * failure nobody else could reproduce on the one machine that runs Windows.
+ * Both walks below therefore state their answer in `/`, always.
+ */
+const posix = (p: string) => p.split(path.sep).join("/")
+
+/** Directories holding a `loading.tsx`, relative to `app/`. */
+function loadingBoundaries(): string[] {
+  const out: string[] = []
+  const walk = (dir: string) => {
+    const entries = readdirSync(dir)
+    if (entries.includes("loading.tsx")) out.push(posix(path.relative(APP_DIR, dir)) || "/")
+    for (const entry of entries) {
+      const child = path.join(dir, entry)
+      if (statSync(child).isDirectory()) walk(child)
+    }
+  }
+  walk(APP_DIR)
+  return out.sort()
+}
+
+/** Pages at or below `dir` that refuse by throwing, and would lose their status. */
+function refusalsUnder(dir: string): string[] {
+  const out: string[] = []
+  const walk = (d: string) => {
+    for (const entry of readdirSync(d)) {
+      const child = path.join(d, entry)
+      if (statSync(child).isDirectory()) walk(child)
+      else if (entry === "page.tsx") {
+        const src = readFileSync(child, "utf8")
+        if (/\bnotFound\(\)|\bredirect\(/.test(src)) out.push(posix(path.relative(APP_DIR, child)))
+      }
+    }
+  }
+  walk(path.join(APP_DIR, dir === "/" ? "" : dir))
+  return out.sort()
+}
+
+describe("a loading boundary must not sit above a refusal", () => {
+  it("keeps the app shell free of a group-level loading.tsx", () => {
+    // The specific file this whole note is about. It was written, measured,
+    // and taken back out.
+    expect(loadingBoundaries()).not.toContain("(app)")
+  })
+
+  it("adds no boundary above a page that refuses by throwing", () => {
+    const offending = loadingBoundaries()
+      .filter((dir) => !PRE_EXISTING.includes(dir))
+      .flatMap((dir) => refusalsUnder(dir).map((page) => `${dir}/loading.tsx swallows ${page}`))
+
+    expect(offending).toEqual([])
+  })
+
+  it("is looking at pages that really do refuse", () => {
+    // The negative control, and it is not decoration: every assertion above is
+    // satisfied by a scanner that found no refusals at all, which is what a
+    // wrong APP_DIR or a broken pattern produces — and it would report the rule
+    // held while measuring nothing.
+    //
+    // `grep -c` prints 0 and exits 1; this counts in-process for the same
+    // reason that idiom keeps costing this repo time.
+    const refusing = refusalsUnder("(app)")
+
+    expect(refusing.length).toBeGreaterThan(35)
+    expect(refusing).toContain("(app)/admin/metering/page.tsx")
+  })
+
+  it("names the one boundary that is grandfathered, and finds it still there", () => {
+    // If somebody removes `reports/loading.tsx`, this list must shrink with it
+    // rather than quietly permitting a future one under the same name.
+    expect(loadingBoundaries()).toContain("(app)/reports")
+  })
+})

apps/web/src/app/(app)/feed/page.tsx

@@ -63,20 +63,59 @@ export default async function FeedPage() {
           ),
         ]
 
-    const posts = await db.feedPost.findMany({
-      where: { institutionId: { in: institutionIds }, isArchived: false },
-      orderBy: { createdAt: "desc" },
-      take: 30,
-      include: {
-        organization: { select: { id: true, name: true, slug: true } },
-        event: { select: { id: true, title: true, startAt: true } },
-        comments: { orderBy: { createdAt: "asc" }, take: 30 },
-        interests: {
-          include: { organization: { select: { name: true } } },
-          orderBy: { createdAt: "asc" },
+    /*
+     * The three reads this page can start at once.
+     *
+     * The feed itself, the clubs this person may post on behalf of, and those
+     * clubs' upcoming events are independent: each takes only ids already
+     * derived from `ctx`. They were three sequential `await`s on a
+     * `force-dynamic` route, so the reader waited for the sum.
+     *
+     * `authors` is NOT in here and cannot be: its `where` is built from the ids
+     * inside `posts`, so it is genuinely a second round trip and stays one.
+     *
+     * `viewerTimeZone` above is likewise left alone. Hoisting it alongside
+     * `getUserContext` looks like a fourth saving and is not one — its first
+     * statement is `await getUserContext(userId)`, and that call is `cache()`d,
+     * so it already awaits the very promise it would be racing.
+     */
+    const [posts, myClubs, myEvents] = await Promise.all([
+      db.feedPost.findMany({
+        where: { institutionId: { in: institutionIds }, isArchived: false },
+        orderBy: { createdAt: "desc" },
+        take: 30,
+        include: {
+          organization: { select: { id: true, name: true, slug: true } },
+          event: { select: { id: true, title: true, startAt: true } },
+          comments: { orderBy: { createdAt: "asc" }, take: 30 },
+          interests: {
+            include: { organization: { select: { name: true } } },
+            orderBy: { createdAt: "asc" },
+          },
         },
-      },
-    })
+      }),
+      // Clubs this user can post/collaborate on behalf of
+      activeOrgIds.length
+        ? db.organization.findMany({
+            where: { id: { in: activeOrgIds } },
+            select: { id: true, name: true },
+            orderBy: { name: "asc" },
+          })
+        : [],
+      // Upcoming events of those clubs (composer's optional link)
+      activeOrgIds.length
+        ? db.event.findMany({
+            where: {
+              organizationId: { in: activeOrgIds },
+              status: { not: "CANCELLED" },
+              startAt: { gte: new Date() },
+            },
+            select: { id: true, title: true, organizationId: true },
+            orderBy: { startAt: "asc" },
+            take: 20,
+          })
+        : [],
+    ])
 
     // Resolve author names in one pass
     const authorIds = [
@@ -94,28 +133,6 @@ export default async function FeedPage() {
       ).map((u) => [u.id, u.name ?? "Unknown"])
     )
 
-    // Clubs this user can post/collaborate on behalf of
-    const myClubs = activeOrgIds.length
-      ? await db.organization.findMany({
-          where: { id: { in: activeOrgIds } },
-          select: { id: true, name: true },
-          orderBy: { name: "asc" },
-        })
-      : []
-
-    // Upcoming events of those clubs (composer's optional link)
-    const myEvents = activeOrgIds.length
-      ? await db.event.findMany({
-          where: {
-            organizationId: { in: activeOrgIds },
-            status: { not: "CANCELLED" },
-            startAt: { gte: new Date() },
-          },
-          select: { id: true, title: true, organizationId: true },
-          orderBy: { startAt: "asc" },
-          take: 20,
-        })
-      : []
 
     const director = institutionIds.some((i) => isOseDirector(ctx, i))
     const pendingForDirector = director

apps/web/src/app/(app)/messages/page.tsx

@@ -44,39 +44,61 @@ export default async function MessagesPage() {
       .filter((r) => r.status === "ACTIVE" || r.status === "SHADOW")
       .map((r) => r.organizationId)
 
-    const conversations = await db.conversation.findMany({
-      where: {
-        OR: [
-          { participants: { some: { userId } } },
-          { type: "BOARD_CHANNEL", organizationId: { in: currentOrgIds } },
-          { type: "BOARD_CHANNEL", institutionId: { in: oseInstitutionIds } },
-        ],
-      },
-      orderBy: { updatedAt: "desc" },
-      take: 50,
-      include: {
-        organization: { select: { name: true } },
-        participants: { include: { user: { select: { id: true, name: true, image: true } } } },
-        messages: {
-          orderBy: { createdAt: "desc" },
-          take: 1,
-          include: { sender: { select: { name: true } } },
+    /*
+     * Three reads, none of which is an input to another.
+     *
+     * They were three sequential `await`s, so the page paid the sum of three
+     * round trips before it emitted a byte — on a `force-dynamic` route with no
+     * loading boundary, which is the whole of the delay a person feels after
+     * clicking Messages. Every argument below is derived from `ctx`, which is
+     * already in hand, so there is nothing to sequence them for.
+     *
+     * The board-channel list needs a club's `id` and its `name` and reads
+     * nothing else — `boardChannelList` below uses exactly `o.id` and `o.name`.
+     * Unselected, this pulled all fourteen scalar columns of every Organization
+     * at the institution (description, rosterNote, logoUrl, imageKey, …) to
+     * print a name in a button. `select` here is not a micro-optimisation of
+     * bytes so much as a statement of what this page is entitled to see.
+     */
+    const [conversations, unread, myOrgs] = await Promise.all([
+      db.conversation.findMany({
+        where: {
+          OR: [
+            { participants: { some: { userId } } },
+            { type: "BOARD_CHANNEL", organizationId: { in: currentOrgIds } },
+            { type: "BOARD_CHANNEL", institutionId: { in: oseInstitutionIds } },
+          ],
         },
-      },
-    })
-
-    // Unread counts per conversation
-    const unread = await db.delivery.groupBy({
-      by: ["participantId"],
-      where: { readAt: null, participant: { userId } },
-      _count: true,
-    })
+        orderBy: { updatedAt: "desc" },
+        take: 50,
+        include: {
+          organization: { select: { name: true } },
+          participants: { include: { user: { select: { id: true, name: true, image: true } } } },
+          messages: {
+            orderBy: { createdAt: "desc" },
+            take: 1,
+            include: { sender: { select: { name: true } } },
+          },
+        },
+      }),
+      // Unread counts per conversation
+      db.delivery.groupBy({
+        by: ["participantId"],
+        where: { readAt: null, participant: { userId } },
+        _count: true,
+      }),
+      currentOrgIds.length
+        ? db.organization.findMany({
+            where: { id: { in: currentOrgIds } },
+            select: { id: true, name: true },
+          })
+        : db.organization.findMany({
+            where: { institutionId: { in: oseInstitutionIds } },
+            select: { id: true, name: true },
+          }),
+    ])
     const unreadByParticipant = new Map(unread.map((u) => [u.participantId, u._count]))
 
-    const myOrgs = currentOrgIds.length
-      ? await db.organization.findMany({ where: { id: { in: currentOrgIds } } })
-      : await db.organization.findMany({ where: { institutionId: { in: oseInstitutionIds } } })
-
     const canBroadcast = oseInstitutionIds.length > 0
 
     const boardChannelList = (orgs: typeof myOrgs) => (