From f4147d234753199b9fad71f07824a422ec0ff978 Mon Sep 17 00:00:00 2001 From: aXenDeveloper Date: Fri, 28 Aug 2026 15:39:35 +0200 Subject: [PATCH] feat: Add layout in tanstack start --- apps/docs/content/docs/dev/websocket.mdx | 29 ++ apps/web/src/components/header.tsx | 104 ++++++ apps/web/src/components/language-switcher.tsx | 71 ++-- .../src/components/layout/main-breadcrumb.tsx | 24 ++ .../web/src/components/layout/main-header.tsx | 26 ++ .../web/src/components/layout/user-header.tsx | 102 ++++++ .../web/src/components/realtime-listeners.tsx | 98 ++++++ apps/web/src/lib/auth/query.ts | 27 ++ apps/web/src/lib/auth/shared.ts | 11 +- apps/web/src/lib/breadcrumb.ts | 70 ++++ apps/web/src/lib/plugin-routes.ts | 40 ++- apps/web/src/lib/realtime.ts | 41 +++ apps/web/src/routeTree.gen.ts | 238 +++++++------- apps/web/src/router.tsx | 14 + apps/web/src/routes/__root.tsx | 11 + apps/web/src/routes/_main.tsx | 103 ++++++ .../src/routes/{ => _main}/_authenticated.tsx | 2 +- .../{ => _main}/_authenticated/account.tsx | 6 +- .../{ => _main}/_authenticated/files.tsx | 6 +- apps/web/src/routes/{ => _main}/discover.tsx | 6 +- apps/web/src/routes/{ => _main}/index.tsx | 6 +- apps/web/src/routes/{ => _main}/search.tsx | 6 +- apps/web/src/tests/discover-route.test.ts | 48 ++- apps/web/src/tests/env-plugin.test.ts | 4 + apps/web/src/tests/header-navigation.test.ts | 103 ++++++ apps/web/src/tests/isolation.test.ts | 21 +- apps/web/src/tests/locale-rewrite.test.ts | 2 +- apps/web/src/tests/main-shell.test.ts | 250 +++++++++++++++ apps/web/src/tests/my-files-route.test.ts | 7 +- apps/web/src/tests/plugin-routes.test.ts | 25 +- apps/web/src/tests/realtime-user-id.test.ts | 129 ++++++++ apps/web/src/tests/router-query.test.ts | 9 +- apps/web/src/tests/shell-config.test.ts | 2 +- apps/web/src/tests/source.ts | 26 ++ .../langs/language-switcher-content.tsx | 120 +++++++ .../switchers/langs/language-switcher.tsx | 108 ++++--- .../theme/header/header-boundaries.test.ts | 303 ++++++++++++++++++ .../layouts/theme/header/header-content.tsx | 100 ++++++ .../layouts/theme/header/header-nav.test.ts | 73 +++++ .../views/layouts/theme/header/header-nav.ts | 92 ++++++ .../layouts/theme/header/header-next.tsx | 38 +++ .../src/views/layouts/theme/header/header.tsx | 76 +++-- .../layouts/theme/header/user/auth/auth.tsx | 29 -- .../layouts/theme/header/user/auth/client.tsx | 85 ----- .../theme/header/user/next-user-header.tsx | 58 ++++ .../user/user-header-boundaries.test.ts | 206 ++++++++++++ .../theme/header/user/user-header-content.tsx | 177 ++++++++++ .../header/user/user-header-model.test.ts | 153 +++++++++ .../theme/header/user/user-header-model.ts | 226 +++++++++++++ .../views/layouts/theme/header/user/user.tsx | 49 +-- .../views/layouts/theme/layout-content.tsx | 49 +++ .../src/views/layouts/theme/layout.tsx | 32 +- .../layouts/theme/theme-boundaries.test.ts | 186 +++++++++++ .../layouts/theme/web-socket-auth-sync.tsx | 39 ++- packages/vitnode/src/ws/auth-sync.test.ts | 54 ++++ packages/vitnode/src/ws/auth-sync.ts | 62 ++++ plugins/example/src/routes/example-page.tsx | 12 +- 57 files changed, 3526 insertions(+), 468 deletions(-) create mode 100644 apps/web/src/components/header.tsx create mode 100644 apps/web/src/components/layout/main-breadcrumb.tsx create mode 100644 apps/web/src/components/layout/main-header.tsx create mode 100644 apps/web/src/components/layout/user-header.tsx create mode 100644 apps/web/src/components/realtime-listeners.tsx create mode 100644 apps/web/src/lib/breadcrumb.ts create mode 100644 apps/web/src/lib/realtime.ts create mode 100644 apps/web/src/routes/_main.tsx rename apps/web/src/routes/{ => _main}/_authenticated.tsx (98%) rename apps/web/src/routes/{ => _main}/_authenticated/account.tsx (96%) rename apps/web/src/routes/{ => _main}/_authenticated/files.tsx (98%) rename apps/web/src/routes/{ => _main}/discover.tsx (98%) rename apps/web/src/routes/{ => _main}/index.tsx (97%) rename apps/web/src/routes/{ => _main}/search.tsx (98%) create mode 100644 apps/web/src/tests/header-navigation.test.ts create mode 100644 apps/web/src/tests/main-shell.test.ts create mode 100644 apps/web/src/tests/realtime-user-id.test.ts create mode 100644 apps/web/src/tests/source.ts create mode 100644 packages/vitnode/src/components/switchers/langs/language-switcher-content.tsx create mode 100644 packages/vitnode/src/views/layouts/theme/header/header-boundaries.test.ts create mode 100644 packages/vitnode/src/views/layouts/theme/header/header-content.tsx create mode 100644 packages/vitnode/src/views/layouts/theme/header/header-nav.test.ts create mode 100644 packages/vitnode/src/views/layouts/theme/header/header-nav.ts create mode 100644 packages/vitnode/src/views/layouts/theme/header/header-next.tsx delete mode 100644 packages/vitnode/src/views/layouts/theme/header/user/auth/auth.tsx delete mode 100644 packages/vitnode/src/views/layouts/theme/header/user/auth/client.tsx create mode 100644 packages/vitnode/src/views/layouts/theme/header/user/next-user-header.tsx create mode 100644 packages/vitnode/src/views/layouts/theme/header/user/user-header-boundaries.test.ts create mode 100644 packages/vitnode/src/views/layouts/theme/header/user/user-header-content.tsx create mode 100644 packages/vitnode/src/views/layouts/theme/header/user/user-header-model.test.ts create mode 100644 packages/vitnode/src/views/layouts/theme/header/user/user-header-model.ts create mode 100644 packages/vitnode/src/views/layouts/theme/layout-content.tsx create mode 100644 packages/vitnode/src/views/layouts/theme/theme-boundaries.test.ts create mode 100644 packages/vitnode/src/ws/auth-sync.test.ts create mode 100644 packages/vitnode/src/ws/auth-sync.ts diff --git a/apps/docs/content/docs/dev/websocket.mdx b/apps/docs/content/docs/dev/websocket.mdx index fe14733aa..a53aedaa9 100644 --- a/apps/docs/content/docs/dev/websocket.mdx +++ b/apps/docs/content/docs/dev/websocket.mdx @@ -367,3 +367,32 @@ export const NotificationListener = () => { one are all built in. A user only receives notifications while signed in - a guest connection has no one to deliver to. + +## Follow sign-in and sign-out + +A connection learns who it belongs to exactly once, during its opening +handshake, from the sign-in cookie the browser sent with it. Nothing afterwards +can change the server's mind - so when someone signs in or out, the socket has +to be re-opened to say hello again as the new person. + +`WebSocketAuthSync` does that for you, and it is already mounted in the default +layout. You only need it if you build your own shell: + +```tsx title="your layout" +import { WebSocketAuthSync } from "@vitnode/core/views/layouts/theme/web-socket-auth-sync"; + +// `userId` is the signed-in user's id, or `null` for a guest. +; +``` + +It renders nothing and holds no state. Hand it the id, and it re-opens the +shared connection whenever that id changes - guest to user, user to guest, or +one account to another. + + + If your app reads the session in the browser rather than on the server, pass + `undefined` until you actually know the answer. `undefined` means "still + asking" and is left alone; `null` means "definitely nobody" and counts as a + sign-out. Collapsing the two would re-open the connection on every page load, + which the socket would find quite rude. + diff --git a/apps/web/src/components/header.tsx b/apps/web/src/components/header.tsx new file mode 100644 index 000000000..82f453f77 --- /dev/null +++ b/apps/web/src/components/header.tsx @@ -0,0 +1,104 @@ +import { useSuspenseQuery } from '@tanstack/react-query' +import { useLanguages } from '@vitnode/core/components/languages-provider' +import { LogoVitNode } from '@vitnode/core/components/logo-vitnode' +import { HeaderLayoutContent } from '@vitnode/core/views/layouts/theme/header/header-content' +import { + HEADER_NAV_MESSAGE_KEYS, + headerNavItems, +} from '@vitnode/core/views/layouts/theme/header/header-nav' +import { createTranslator } from 'use-intl' + +import type { Locale } from '#/lib/i18n/shared' + +import { LanguageSwitcher } from '#/components/language-switcher' +import { MigrationLink } from '#/components/migration-link' +import { useLocale } from '#/lib/i18n/client' +import { intlQueryOptions } from '#/lib/i18n/query' + +/** What the header renders strings from: the shell's set, plus the nav labels. */ +export const HEADER_NAMESPACES = ['core.global', 'core.search'] as const + +/** + * The messages the header renders, as a query the shell's loader can ensure. + * + * Exported so the loader and the component cannot ask for different sets: the + * namespace list is part of the query key, so a shell that warmed + * `["core.global"]` would leave the header suspending on a key nobody fetched. + */ +export const headerIntlQueryOptions = ({ locale }: { locale: Locale }) => + intlQueryOptions({ locale, namespaces: HEADER_NAMESPACES }) + +/** + * The main header, on TanStack Start. + * + * The bar, the logo, the nav and the action area are `HeaderLayoutContent` - the + * same module the Next.js pages render, so there is one copy of that markup + * rather than one per framework. What this supplies is the three things a shared + * component cannot resolve for itself: the link, the language switcher, and the + * translated nav. + * + * ## The link is `MigrationLink` + * + * Not the router's `Link` directly. `/`, `/discover` and `/search` are all + * routes this app owns today, so all three are client-side navigations with the + * locale prefix written by Stage 3's rewrite - no prefix is applied here, and + * applying one would produce `/pl/pl/discover`. `MigrationLink` asks the route + * tree that question per href, which is what makes a header link that later + * points at a route the Next.js app still serves (`/files`, `/admin`) a document + * load into that app instead of a TanStack not-found. There is no allowlist of + * migrated routes in that decision - the route tree is the list. + * + * ## The strings, and what the shell has to warm + * + * The labels are `core.search.nav.*` - the same keys the Next.js header reads, + * paired with their hrefs by the same `headerNavItems`. The header sits above + * every route, so it cannot rely on a route's own `RouteMessages`: it reads the + * messages out of the cache itself and translates them with `use-intl`'s + * framework-free `createTranslator`. + * + * That makes one demand on whoever mounts it: **the shell's loader must warm + * {@link headerIntlQueryOptions}**, which `_main`'s does. It is a + * `useSuspenseQuery` with no boundary between it and the document, so an + * unwarmed entry does not degrade - it suspends the whole response. Warming it + * is one line, and it is the same rule every migrated route already follows for + * its own namespaces. + * + * No provider is mounted here. `core.global` - which the theme switcher and the + * language switcher read - is provided by the root route, and the two extra + * words the nav needs are not worth replacing the message tree over. + */ +export const Header = ({ + logo = , + user, +}: { + /** The application's mark. Defaults to VitNode's, as both Next.js apps pass. */ + logo?: React.ReactNode + /** The session slot - avatar and menu when signed in, sign-in button when not. */ + user?: React.ReactNode +}) => { + const locale = useLocale() + const languages = useLanguages() + const { data } = useSuspenseQuery(headerIntlQueryOptions({ locale })) + + const t = createTranslator({ + locale, + messages: data.messages, + namespace: 'core.search', + }) + + return ( + 1 ? : null} + LinkComponent={MigrationLink} + logo={logo} + navigation={headerNavItems({ + discover: t(HEADER_NAV_MESSAGE_KEYS.discover), + search: t(HEADER_NAV_MESSAGE_KEYS.search), + })} + user={user} + /> + ) +} diff --git a/apps/web/src/components/language-switcher.tsx b/apps/web/src/components/language-switcher.tsx index 675118b42..8f203c02e 100644 --- a/apps/web/src/components/language-switcher.tsx +++ b/apps/web/src/components/language-switcher.tsx @@ -1,67 +1,38 @@ import { useLanguages } from '@vitnode/core/components/languages-provider' -import { Button } from '@vitnode/core/components/ui/button' -import { - DropdownMenu, - DropdownMenuContent, - DropdownMenuItem, - DropdownMenuTrigger, -} from '@vitnode/core/components/ui/dropdown-menu' -import { CheckIcon, LanguagesIcon } from 'lucide-react' -import { useTranslations } from 'use-intl' - -import type { Locale } from '#/lib/i18n/shared' +import { LanguageSwitcherContent } from '@vitnode/core/components/switchers/langs/language-switcher-content' import { useLocale, useSwitchLocale } from '#/lib/i18n/client' +import { isLocale } from '#/lib/i18n/shared' /** * VitNode's language switcher, for TanStack Router. * - * The same control as `@vitnode/core`'s - the same dropdown, the same icons, the - * same `core.global.language_switcher` label - over a different navigation - * layer. Core's version is built on `next-intl/navigation`'s `useRouter`, which - * is Next.js all the way down; this one is built on the router that is actually - * mounted here. Sharing the markup and forking the two lines that navigate is - * cheaper than a navigation abstraction that has to satisfy both. + * The same control as the Next.js app's - literally the same component now + * (`LanguageSwitcherContent`), so the dropdown, the icon, the check mark and the + * `core.global.language_switcher` label cannot drift between the two. What is + * forked is the two lines that navigate: core's Next half replaces the pathname + * through `next-intl`'s locale-aware router, and this one goes through Stage 3's + * `useSwitchLocale`, which pushes the public href and invalidates. * - * What it preserves is the whole point: the route, its params, its search - * string and its hash. Only the locale prefix changes. + * What it preserves is the whole point: the route, its params, its search string + * and its hash. Only the locale prefix changes - and on a route that carries no + * prefix (`/admin`) the cookie is the whole of the switch. All of that rule lives + * in `#/lib/i18n/client`, not here. */ export const LanguageSwitcher = () => { const languages = useLanguages() const locale = useLocale() const switchLocale = useSwitchLocale() - const t = useTranslations('core.global') return ( - - - } - > - - - - - {languages.map((language) => ( - { - switchLocale(language.code as Locale) - }} - > - {language.name} - - {language.code === locale && ( - - )} - - ))} - - + { + // The list comes from configuration, so this always holds - and narrowing + // it here is what keeps a locale cast out of a click handler. + if (isLocale(code)) switchLocale(code) + }} + options={languages} + /> ) } diff --git a/apps/web/src/components/layout/main-breadcrumb.tsx b/apps/web/src/components/layout/main-breadcrumb.tsx new file mode 100644 index 000000000..40fa0044d --- /dev/null +++ b/apps/web/src/components/layout/main-breadcrumb.tsx @@ -0,0 +1,24 @@ +import { useMatches } from '@tanstack/react-router' + +import { breadcrumbOf } from '#/lib/breadcrumb' + +/** + * The shell's breadcrumb area: whichever matched route declared the deepest + * crumb, and nothing at all when none did. + * + * `useMatches()` rather than a `select`: the whole match list changes on + * navigation, which is exactly when this has to re-render, and the router's + * structural sharing has nothing useful to say about a React element. + * + * The crumb owns its own markup, including the container the legacy slot uses + * (`BreadcrumbMain` renders `container mx-auto p-4`), so a route that moves here + * keeps the spacing it had. + */ +export const MainBreadcrumb = () => { + const breadcrumb = breadcrumbOf(useMatches()) + + // Wrapped rather than returned straight: `ReactNode` includes a promise in + // React 19's types, and a component whose inferred return type includes one + // reads as an async component to every rule that looks for one. + return <>{breadcrumb} +} diff --git a/apps/web/src/components/layout/main-header.tsx b/apps/web/src/components/layout/main-header.tsx new file mode 100644 index 000000000..ad025bcbd --- /dev/null +++ b/apps/web/src/components/layout/main-header.tsx @@ -0,0 +1,26 @@ +import { Header } from '#/components/header' +import { UserHeader } from '#/components/layout/user-header' + +/** + * The site header, as the main shell's slot for it. + * + * Two components and no logic, which is the point: `Header` is the bar - the + * logo, the nav, the language and theme switchers - and `UserHeader` is the + * session-dependent half that goes in its action area. Keeping them separate is + * what lets the bar render from the message cache alone while the user area + * reads a query that may still be in flight. + * + * The logo is `Header`'s default (`LogoVitNode`), as it is in both Next.js apps. + * An application with its own mark passes one here and changes nothing else. + * + * ## What the shell owes this + * + * Two warm cache entries, both ensured by `_main`'s loader: + * + * headerIntlQueryOptions -> a `useSuspenseQuery`, so this is required + * prefetchSession -> the first paint shows the visitor, not a gap + * + * See the loader in `routes/_main.tsx`, which states why one is `ensure` and the + * other `prefetch`. + */ +export const MainHeader = () =>
} /> diff --git a/apps/web/src/components/layout/user-header.tsx b/apps/web/src/components/layout/user-header.tsx new file mode 100644 index 000000000..29f1ef360 --- /dev/null +++ b/apps/web/src/components/layout/user-header.tsx @@ -0,0 +1,102 @@ +import { useQuery } from '@tanstack/react-query' +import { UserHeaderContent } from '@vitnode/core/views/layouts/theme/header/user/user-header-content' +import { userHeaderState } from '@vitnode/core/views/layouts/theme/header/user/user-header-model' +import { toast } from 'sonner' +import { useTranslations } from 'use-intl' + +import { MigrationLink } from '#/components/migration-link' +import { useSignOutAction } from '#/lib/auth/actions' +import { sessionQueryOptions } from '#/lib/auth/query' + +/** + * The user area of the main header, wired to this app. + * + * Everything visible is `UserHeaderContent`'s - the avatar, the menu, the guest + * buttons and the placeholder are the same components the Next.js header renders. + * What is here is the three things that component refuses to decide: + * + * the session -> sessionQueryOptions() the one canonical entry + * a link -> MigrationLink the route tree decides per href + * sign-out -> useSignOutAction() Stage 6's action, unchanged + * + * ## One session, read - not fetched + * + * `useQuery` over `sessionQueryOptions()`, which is the *same definition* that + * `_authenticated`'s guard calls through `ensureAuthState`. One key, one cache + * entry, one request: the header cannot show a visitor the guard has already + * turned away, and signing in or out replaces the value both of them read. There + * is no `AuthContext`, no module-level session and no second call to + * `/users/session` - see the long note in `#/lib/auth/query` for why the + * QueryClient's per-request lifetime is what a session needs. + * + * `useQuery` rather than `useSuspenseQuery` on purpose. The header is on every + * page, and suspending it would suspend the shell: with the entry warm both read + * from the cache with no round trip, but when it is *not* warm - a client-side + * arrival at a route whose loader did not ask for it - `useQuery` renders the + * placeholder while `useSuspenseQuery` would hold back the whole page for a + * session only the header needs. + * + * ## What the shell owes it: one line + * + * `_main`'s loader calls `prefetchSession(context.queryClient)`. With + * it, the entry is filled before anything renders, the SSR pass dehydrates it, + * and the very first paint shows the visitor - no placeholder, no shift, and no + * round trip after hydration. Without it this still works, and works correctly: + * the server renders the placeholder and the browser fills it in a moment later. + * So it is a performance requirement rather than a correctness one, which is why + * this component does not try to enforce it. + * + * It is deliberately `prefetchSession` and not `ensureAuthState`. The latter + * *rejects* when the session cannot be read - correct for a guard, because an + * outage must not sign anybody out - and in a shell's loader that same rejection + * would replace every page on the site with an error screen because the header + * could not name the visitor. Both go through `sessionQueryOptions()`, so it is + * still one cache entry either way. + * + * ## The three states are `userHeaderState`'s to name + * + * Including the one that matters here: a session already in hand wins over an + * error, so a failed *refetch* does not flicker a signed-in visitor to anonymous + * and back. A read that has failed with nothing cached shows the guest controls, + * which is what the Next.js header has always done - and which is emphatically + * not what a route guard does with the same failure. + */ +export const UserHeader = () => { + const { data, isError } = useQuery(sessionQueryOptions()) + const signOut = useSignOutAction() + const tErrors = useTranslations('core.global.errors') + + /** + * Sign-out, and the one thing Stage 6's action leaves to its caller. + * + * The action already does all of it - `DELETE /sign_out`, the cookie cleared + * by the API's own `Set-Cookie`, the anonymous session written into the + * canonical entry, that entry invalidated, and `router.invalidate()` so a + * visitor sitting behind `_authenticated` is redirected out by the guard that + * owns that rule. Nothing is repeated here: no second `invalidate`, no + * `setQueryData` of anything else, no navigation of our own. + * + * What is left is the failure, which the action reports rather than throws. A + * session that could not be ended is a server problem the visitor cannot act + * on, so it is the internal-error toast - the same one the sign-in form raises + * for the same class of answer - and the header stays as it was, which is + * honest: they are still signed in. + */ + const onSignOut = async () => { + const result = await signOut() + + if (result.ok) return + + toast.error(tErrors('title'), { + description: tErrors('internal_server_error'), + }) + } + + return ( + + ) +} diff --git a/apps/web/src/components/realtime-listeners.tsx b/apps/web/src/components/realtime-listeners.tsx new file mode 100644 index 000000000..3a7806321 --- /dev/null +++ b/apps/web/src/components/realtime-listeners.tsx @@ -0,0 +1,98 @@ +import { useQuery } from '@tanstack/react-query' +import { NotificationListener } from '@vitnode/core/views/layouts/theme/notification-listener' +import { WebSocketAuthSync } from '@vitnode/core/views/layouts/theme/web-socket-auth-sync' + +import { sessionQueryOptions } from '#/lib/auth/query' +import { socketUserIdFromSession } from '#/lib/realtime' + +/** + * The two behaviors that belong to the WebSocket connection: the notification + * toasts, and keeping the socket authenticated as whoever is signed in. + * + * __root VitNodeWebSocketProvider one connection, one lifetime + * __root RealtimeListeners the listener, and the identity + * + * Both components are `@vitnode/core`'s own, shared verbatim with the Next.js + * app, which mounts the same pair from `ThemeLayout`. Neither renders anything; + * this exists to give them the one thing they cannot get for themselves here - + * the visitor's id - and to keep that read in a single place rather than in each + * of them. + * + * ## Why this is at the root and not in the shell's `listeners` slot + * + * It was in the slot, which reads as the tidier answer: `ThemeLayoutContent` has + * a slot named `listeners`, the Next.js app fills it, so `_main` should fill it + * too. That is wrong here, and the reason is that the two applications disagree + * about where `/login` lives. + * + * In Next.js `/login` is inside `(main)`, so `WebSocketAuthSync` is mounted + * while a visitor signs in and stays mounted through the transition afterwards: + * it holds `previous = null`, sees `next = 42`, and reconnects. In this app + * `/login` is deliberately outside `_main` - an auth screen is a full-height + * card with no header - so the shell is *not* mounted while they sign in. It + * mounts for the first time at the destination, by which point the session query + * already answers `42`. `WebSocketAuthSync` seeds its ref from its first prop, + * `shouldReconnectForUser(42, 42)` is `false`, and the socket keeps the guest + * handshake it opened with until the next full page load. The visitor is signed + * in everywhere except the connection that delivers their notifications. + * + * The sharp edge is that it half-works without the shell's loader - the stale + * guest session is observed first, so the identity still moves `null -> 42` - + * and breaks the moment `prefetchSession` is added to `_main` for the header's + * sake. Which is to say: it would have been introduced by a one-line performance + * fix, in a component neither line mentions. + * + * So the rule this file follows is the one the provider already follows. + * Anything whose lifetime is *the connection's* is mounted where the connection + * is; anything whose lifetime is the visual shell's goes in the shell. The + * socket's identity is plainly the former - it must survive every navigation + * that the connection survives, including the navigation away from an auth + * screen. `ThemeLayoutContent`'s `listeners` slot stays exactly as it is, for + * the Next.js app that has somewhere to put it. + * + * It also restores parity in passing: `NotificationListener` is mounted on the + * login screen in the Next.js app, and mounting it here is what keeps that true. + * + * ## Where the id comes from + * + * `sessionQueryOptions()` - the one session definition in this app, the same one + * `_authenticated`'s guard and the header read. So this makes no second request + * and adds no second key: `/login` warms that entry in its guard, `_main` warms + * it in its loader, and this is a read of whichever one got there first. See + * `lib/auth/query.ts`, which explains why there is exactly one. + * + * `useQuery` rather than `useSuspenseQuery`, for two reasons. It must not + * suspend - it sits above every route, and suspending here would hold back the + * whole document for a value only an invisible effect wants. And it must not + * throw: the session read is `retry: false`, so an API outage records a failure + * in that entry, and a suspense read would rethrow it into the nearest boundary + * and take the entire application down with it. Read this way a failure arrives + * as `undefined`, which `socketUserIdFromSession` reports as "not known yet" and + * `WebSocketAuthSync` correctly does nothing about - the socket keeps whatever + * identity its handshake gave it. + * + * On a route that warms nothing - the SSO callback - the observer mounted here + * is what fetches the session at all, and that is the desired behaviour rather + * than a cost: completing an SSO sign-in invalidates that entry, this refetches + * it, the identity moves, and the socket re-handshakes. During SSR it renders + * with `undefined` and does nothing, which is correct: effects do not run there + * and there is no socket yet. + * + * ## What it does on sign-in and sign-out + * + * Nothing directly - it has no handlers. `lib/auth/actions.ts` brings that one + * cache entry back in step with the cookie before it navigates, this re-renders + * from it, and the identity change is what re-opens the socket so the server + * re-reads the cookie on a fresh handshake. Which is what stops the previous + * visitor's notifications from reaching this browser, with no page reload. + */ +export const RealtimeListeners = () => { + const { data: session } = useQuery(sessionQueryOptions()) + + return ( + <> + + + + ) +} diff --git a/apps/web/src/lib/auth/query.ts b/apps/web/src/lib/auth/query.ts index c95a364f9..2f26b2731 100644 --- a/apps/web/src/lib/auth/query.ts +++ b/apps/web/src/lib/auth/query.ts @@ -152,3 +152,30 @@ export const invalidateSession = async ( queryClient: QueryClient, ): Promise => await queryClient.invalidateQueries({ queryKey: SESSION_QUERY_KEY }) + +/** + * Fill the session entry without letting a failed read take the page down. + * + * What a layout loader calls so the header renders the visitor on the *first* + * paint. `ensureAuthState` is the wrong tool for that job in one specific way: + * it rejects when the session cannot be read, which is exactly right for a guard + * - an outage must not sign anybody out - and exactly wrong for a shell, where + * the same rejection would replace every page on the site with an error screen + * because the header could not name the visitor. + * + * `prefetchQuery` is the difference: same query definition, same key, same single + * in-flight request, and a failure is recorded in the cache entry instead of + * thrown. So the shell renders, the SSR pass dehydrates whatever was learned, and + * the header reads it back through `useQuery` - `data` when the read worked, + * `isError` when it did not, and `userHeaderState` decides what that looks like. + * + * Deliberately not a second query. Anything that guards a route still goes + * through {@link ensureAuthState}, and both reach the one entry this module owns + * - so a page under `_authenticated` and the header above it cannot disagree + * about who is signed in. + */ +export const prefetchSession = async ( + queryClient: QueryClient, +): Promise => { + await queryClient.prefetchQuery(sessionQueryOptions()) +} diff --git a/apps/web/src/lib/auth/shared.ts b/apps/web/src/lib/auth/shared.ts index 135703343..2a2dc2bb4 100644 --- a/apps/web/src/lib/auth/shared.ts +++ b/apps/web/src/lib/auth/shared.ts @@ -31,9 +31,14 @@ import type { SessionApi } from '#/lib/session' * role`). So there is no moderator authorization to model yet, and this state * exposes no `isModerator` flag: a guard written against one would read as * enforcement while being a constant, and would silently start granting access - * the day the API begins answering `true`. The field stays reachable as - * `auth.user.isModerator` for the shared header, which uses it to decide whether - * to draw a link - see `views/layouts/theme/header/user/auth/client.tsx`. + * the day the API begins answering `true`. + * + * Nothing renders it either, as of Stage 8. The user menu used to draw a + * `/mod_cp` link behind that flag, pointing at a page neither application + * serves; `userHeaderMenu` in + * `views/layouts/theme/header/user/user-header-model.ts` branches on `isAdmin` + * alone. The field stays reachable as `auth.user.isModerator` for whoever + * implements the role. */ /** diff --git a/apps/web/src/lib/breadcrumb.ts b/apps/web/src/lib/breadcrumb.ts new file mode 100644 index 000000000..309ea1baa --- /dev/null +++ b/apps/web/src/lib/breadcrumb.ts @@ -0,0 +1,70 @@ +/** + * Where a breadcrumb comes from, in a router that has no parallel routes. + * + * Next.js resolves the main breadcrumb through a `@breadcrumb` slot: a parallel + * route whose folder mirrors the page's, so the *deepest* folder with a + * `page.tsx` wins, one that returns `null` clears what a shallower one rendered, + * and everything unmatched falls through to `default.tsx`. This is the same + * rule, expressed with what a router already has - the list of matched routes, + * deepest last - and one optional field on each route's `staticData`. + * + * A `ReactNode`, exactly like the Next.js slot and like the `breadcrumb` prop of + * `ThemeLayoutContent`, so the shell renders it rather than deciding anything + * about it. Declaring it as an *element* is what lets a crumb use hooks - the + * label is translated, and on a dynamic route it comes from the loader - without + * the shell having to instantiate a component it was handed: + * + * staticData: { breadcrumb: } + * + * What this is *not*: a breadcrumb registry. There is no map from pathname to + * label anywhere, and no plugin registers into one. A route declares its own + * crumb next to its own component, and the shell renders whichever declared + * crumb is deepest. See `#/components/layout/main-breadcrumb`. + */ +declare module '@tanstack/react-router' { + interface StaticDataRouteOption { + /** + * What this route contributes to the shell's breadcrumb area. + * + * Absent means "whatever my parent said"; `null` means "nothing", which is + * how a child clears a crumb an ancestor declared. + * + * Optional, and it must stay optional: a required member here would make + * `staticData` required on every route in the app. + */ + breadcrumb?: React.ReactNode + } +} + +/** + * The narrowest shape of a route match this rule reads. + * + * A structural type rather than the router's `AnyRouteMatch`, so the rule is a + * function over plain data and can be tested as one - no router, no route tree. + */ +export interface BreadcrumbMatch { + staticData: { breadcrumb?: React.ReactNode } +} + +/** + * The deepest matched route that declares a breadcrumb, or nothing. + * + * Deepest wins, which is the whole rule: `/settings/security` shows the security + * crumb rather than the settings one, and a route that declares nothing inherits + * its parent's - including inheriting *nothing*, which is how `/` ends up + * without a breadcrumb having never mentioned one. + * + * `undefined` is "did not declare" and is the only value that falls through; + * `null` is a declaration, and the deliberate way to clear an ancestor's crumb. + */ +export const breadcrumbOf = ( + matches: readonly BreadcrumbMatch[], +): React.ReactNode => { + for (let index = matches.length - 1; index >= 0; index--) { + const declared = matches[index].staticData.breadcrumb + + if (declared !== undefined) return declared + } + + return null +} diff --git a/apps/web/src/lib/plugin-routes.ts b/apps/web/src/lib/plugin-routes.ts index d5585092c..42af1459f 100644 --- a/apps/web/src/lib/plugin-routes.ts +++ b/apps/web/src/lib/plugin-routes.ts @@ -43,11 +43,11 @@ import { * * Pathless, so it contributes no URL segment: a plugin route at `/example` is * served at `/example`, not at `/_plugins/example`. It earns its place by making - * the composition below **idempotent** - the plugin subtree is one child of the - * root, identifiable by this id, so re-running the composition replaces it - * instead of appending a second copy of every route. That is not a theoretical - * concern: in dev, Vite re-evaluates this module without re-evaluating - * `routeTree.gen.ts`, and the root route it mutates is the same object. + * the composition below **idempotent** - the plugin subtree is one child of its + * mount point, identifiable by this id, so re-running the composition replaces + * it instead of appending a second copy of every route. That is not a + * theoretical concern: in dev, Vite re-evaluates this module without + * re-evaluating `routeTree.gen.ts`, and the route it mutates is the same object. * * It also gives the whole plugin subtree one name in the router devtools, and * one place for a future stage to hang something every plugin page needs. @@ -241,27 +241,43 @@ const assertNoAppCollision = ( * Mounts the plugin routes on a route tree, and hands the same tree back. * * `addChildren` **replaces** a route's children and mutates the route in place, - * so the plugin subtree is rebuilt from the root's current children with any - * previous copy of itself removed. Calling this twice on one tree is therefore - * the same as calling it once, which is what makes it safe in a dev server that - * re-evaluates this module while `routeTree.gen.ts` stays cached. + * so the plugin subtree is rebuilt from the mount point's current children with + * any previous copy of itself removed. Calling this twice on one tree is + * therefore the same as calling it once, which is what makes it safe in a dev + * server that re-evaluates this module while `routeTree.gen.ts` stays cached. * * The route's component is a `lazyRouteComponent` over the registry's loader, * which is the supported way to code-split a code-based route: the plugin's page * gets its own Rollup chunk, stays out of the initial bundle, and the router * awaits `component.preload()` before it renders the match - so SSR and * hydration both have the module in hand rather than suspending on it. + * + * ## `mountUnder` + * + * Which route the subtree hangs from, defaulting to the tree's root - and the + * only reason it is a parameter is the application shell. Every plugin route + * declares `area: "main"` (see `@vitnode/core/routing`), which is a statement + * about *layout*: this page belongs on the public site, with the header and the + * breadcrumb area a page of the site has. In a router, a layout is a parent - + * so honouring that declaration is choosing a parent, and `src/router.tsx` + * passes the `_main` route. + * + * Nothing about the path changes: `_main` is pathless, so `/example` stays + * `/example`. And the collision check below still walks the whole tree from its + * root, because what a plugin route may not shadow is *any* URL the app answers, + * wherever in the tree it was declared. */ export const withPluginRoutes = ( routeTree: TRouteTree, specs: PluginRouteSpec[], + mountUnder: AnyRoute = routeTree, ): TRouteTree => { if (specs.length === 0) return routeTree assertNoAppCollision(specs, fileRoutePaths(routeTree)) const container = createRoute({ - getParentRoute: () => routeTree, + getParentRoute: () => mountUnder, id: PLUGIN_ROUTES_ROUTE_ID, }) @@ -277,11 +293,11 @@ export const withPluginRoutes = ( ), ) - const siblings: AnyRoute[] = (routeTree.children ?? []).filter( + const siblings: AnyRoute[] = (mountUnder.children ?? []).filter( (child: AnyRoute) => declaredOptions(child).id !== PLUGIN_ROUTES_ROUTE_ID, ) - routeTree.addChildren([...siblings, container]) + mountUnder.addChildren([...siblings, container]) return routeTree } diff --git a/apps/web/src/lib/realtime.ts b/apps/web/src/lib/realtime.ts new file mode 100644 index 000000000..21c223f2d --- /dev/null +++ b/apps/web/src/lib/realtime.ts @@ -0,0 +1,41 @@ +import type { VitNodeSocketUserId } from '@vitnode/core/ws/auth-sync' + +import type { SessionApi } from '#/lib/session' + +/** + * The shell's realtime layer, as the one derivation it needs. + * + * Pure by construction - both imports are type-only, so TypeScript erases them + * and this module has no runtime dependencies at all. That is what lets it be + * tested without the server function, the fetcher or the WebSocket behind either + * of them, and it is the same reason `lib/auth/shared.ts` is written this way. + */ + +/** + * The visitor the WebSocket should be authenticated as, from the canonical + * session. + * + * Three inputs and three distinct answers, which is the whole point of the + * function: + * + * undefined -> undefined the session is not known yet + * { user: null } -> null the API answered: nobody is signed in + * { user } -> user.id the API answered: this visitor + * + * `undefined` in means the query has not answered - the entry has not been + * warmed, or the read failed and `retry: false` left it in an error state with + * no data. Collapsing that to `null` would be this app inventing a guest, which + * is the mistake `lib/session.ts` exists to refuse: on a client-side session + * read it is also the *first* value of every page load, so `WebSocketAuthSync` + * would read a sign-out and re-open the shared connection each time. + * + * `null` out means signed out, and only that. `shouldReconnectForUser` in + * `@vitnode/core/ws/auth-sync` is the half that acts on the difference. + */ +export const socketUserIdFromSession = ( + session: SessionApi | undefined, +): undefined | VitNodeSocketUserId => { + if (!session) return undefined + + return session.user?.id ?? null +} diff --git a/apps/web/src/routeTree.gen.ts b/apps/web/src/routeTree.gen.ts index 75e434876..aa24150c8 100644 --- a/apps/web/src/routeTree.gen.ts +++ b/apps/web/src/routeTree.gen.ts @@ -9,55 +9,61 @@ // Additionally, you should also exclude this file from your linter and/or formatter to prevent it from being checked or modified. import { Route as rootRouteImport } from './routes/__root' -import { Route as IndexRouteImport } from './routes/index' -import { Route as AuthenticatedRouteImport } from './routes/_authenticated' -import { Route as DiscoverRouteImport } from './routes/discover' +import { Route as MainRouteImport } from './routes/_main' import { Route as LoginRouteImport } from './routes/login' -import { Route as SearchRouteImport } from './routes/search' -import { Route as AuthenticatedAccountRouteImport } from './routes/_authenticated/account' -import { Route as AuthenticatedFilesRouteImport } from './routes/_authenticated/files' +import { Route as MainIndexRouteImport } from './routes/_main/index' +import { Route as MainAuthenticatedRouteImport } from './routes/_main/_authenticated' +import { Route as MainDiscoverRouteImport } from './routes/_main/discover' +import { Route as MainSearchRouteImport } from './routes/_main/search' import { Route as ApiSplatRouteImport } from './routes/api/$' +import { Route as MainAuthenticatedAccountRouteImport } from './routes/_main/_authenticated/account' +import { Route as MainAuthenticatedFilesRouteImport } from './routes/_main/_authenticated/files' import { Route as LoginSsoProviderIdRouteImport } from './routes/login_.sso.$providerId' -const IndexRoute = IndexRouteImport.update({ +const MainRoute = MainRouteImport.update({ + id: '/_main', + getParentRoute: () => rootRouteImport, +} as any) +const LoginRoute = LoginRouteImport.update({ + id: '/login', + path: '/login', + getParentRoute: () => rootRouteImport, +} as any) +const MainIndexRoute = MainIndexRouteImport.update({ id: '/', path: '/', - getParentRoute: () => rootRouteImport, + getParentRoute: () => MainRoute, } as any) -const AuthenticatedRoute = AuthenticatedRouteImport.update({ +const MainAuthenticatedRoute = MainAuthenticatedRouteImport.update({ id: '/_authenticated', - getParentRoute: () => rootRouteImport, + getParentRoute: () => MainRoute, } as any) -const DiscoverRoute = DiscoverRouteImport.update({ +const MainDiscoverRoute = MainDiscoverRouteImport.update({ id: '/discover', path: '/discover', - getParentRoute: () => rootRouteImport, + getParentRoute: () => MainRoute, } as any) -const LoginRoute = LoginRouteImport.update({ - id: '/login', - path: '/login', - getParentRoute: () => rootRouteImport, -} as any) -const SearchRoute = SearchRouteImport.update({ +const MainSearchRoute = MainSearchRouteImport.update({ id: '/search', path: '/search', - getParentRoute: () => rootRouteImport, -} as any) -const AuthenticatedAccountRoute = AuthenticatedAccountRouteImport.update({ - id: '/account', - path: '/account', - getParentRoute: () => AuthenticatedRoute, -} as any) -const AuthenticatedFilesRoute = AuthenticatedFilesRouteImport.update({ - id: '/files', - path: '/files', - getParentRoute: () => AuthenticatedRoute, + getParentRoute: () => MainRoute, } as any) const ApiSplatRoute = ApiSplatRouteImport.update({ id: '/api/$', path: '/api/$', getParentRoute: () => rootRouteImport, } as any) +const MainAuthenticatedAccountRoute = + MainAuthenticatedAccountRouteImport.update({ + id: '/account', + path: '/account', + getParentRoute: () => MainAuthenticatedRoute, + } as any) +const MainAuthenticatedFilesRoute = MainAuthenticatedFilesRouteImport.update({ + id: '/files', + path: '/files', + getParentRoute: () => MainAuthenticatedRoute, +} as any) const LoginSsoProviderIdRoute = LoginSsoProviderIdRouteImport.update({ id: '/login_/sso/$providerId', path: '/login/sso/$providerId', @@ -65,102 +71,87 @@ const LoginSsoProviderIdRoute = LoginSsoProviderIdRouteImport.update({ } as any) export interface FileRoutesByFullPath { - '/': typeof IndexRoute - '/discover': typeof DiscoverRoute + '/': typeof MainIndexRoute '/login': typeof LoginRoute - '/search': typeof SearchRoute - '/account': typeof AuthenticatedAccountRoute - '/files': typeof AuthenticatedFilesRoute + '/discover': typeof MainDiscoverRoute + '/search': typeof MainSearchRoute '/api/$': typeof ApiSplatRoute + '/account': typeof MainAuthenticatedAccountRoute + '/files': typeof MainAuthenticatedFilesRoute '/login/sso/$providerId': typeof LoginSsoProviderIdRoute } export interface FileRoutesByTo { - '/': typeof IndexRoute - '/discover': typeof DiscoverRoute '/login': typeof LoginRoute - '/search': typeof SearchRoute - '/account': typeof AuthenticatedAccountRoute - '/files': typeof AuthenticatedFilesRoute + '/': typeof MainIndexRoute + '/discover': typeof MainDiscoverRoute + '/search': typeof MainSearchRoute '/api/$': typeof ApiSplatRoute + '/account': typeof MainAuthenticatedAccountRoute + '/files': typeof MainAuthenticatedFilesRoute '/login/sso/$providerId': typeof LoginSsoProviderIdRoute } export interface FileRoutesById { __root__: typeof rootRouteImport - '/': typeof IndexRoute - '/_authenticated': typeof AuthenticatedRouteWithChildren - '/discover': typeof DiscoverRoute + '/_main': typeof MainRouteWithChildren '/login': typeof LoginRoute - '/search': typeof SearchRoute - '/_authenticated/account': typeof AuthenticatedAccountRoute - '/_authenticated/files': typeof AuthenticatedFilesRoute + '/_main/_authenticated': typeof MainAuthenticatedRouteWithChildren + '/_main/discover': typeof MainDiscoverRoute + '/_main/search': typeof MainSearchRoute '/api/$': typeof ApiSplatRoute + '/_main/': typeof MainIndexRoute + '/_main/_authenticated/account': typeof MainAuthenticatedAccountRoute + '/_main/_authenticated/files': typeof MainAuthenticatedFilesRoute '/login_/sso/$providerId': typeof LoginSsoProviderIdRoute } export interface FileRouteTypes { fileRoutesByFullPath: FileRoutesByFullPath fullPaths: | '/' - | '/discover' | '/login' + | '/discover' | '/search' + | '/api/$' | '/account' | '/files' - | '/api/$' | '/login/sso/$providerId' fileRoutesByTo: FileRoutesByTo to: + | '/login' | '/' | '/discover' - | '/login' | '/search' + | '/api/$' | '/account' | '/files' - | '/api/$' | '/login/sso/$providerId' id: | '__root__' - | '/' - | '/_authenticated' - | '/discover' + | '/_main' | '/login' - | '/search' - | '/_authenticated/account' - | '/_authenticated/files' + | '/_main/_authenticated' + | '/_main/discover' + | '/_main/search' | '/api/$' + | '/_main/' + | '/_main/_authenticated/account' + | '/_main/_authenticated/files' | '/login_/sso/$providerId' fileRoutesById: FileRoutesById } export interface RootRouteChildren { - IndexRoute: typeof IndexRoute - AuthenticatedRoute: typeof AuthenticatedRouteWithChildren - DiscoverRoute: typeof DiscoverRoute + MainRoute: typeof MainRouteWithChildren LoginRoute: typeof LoginRoute - SearchRoute: typeof SearchRoute ApiSplatRoute: typeof ApiSplatRoute LoginSsoProviderIdRoute: typeof LoginSsoProviderIdRoute } declare module '@tanstack/react-router' { interface FileRoutesByPath { - '/': { - id: '/' - path: '/' - fullPath: '/' - preLoaderRoute: typeof IndexRouteImport - parentRoute: typeof rootRouteImport - } - '/_authenticated': { - id: '/_authenticated' + '/_main': { + id: '/_main' path: '' fullPath: '/' - preLoaderRoute: typeof AuthenticatedRouteImport - parentRoute: typeof rootRouteImport - } - '/discover': { - id: '/discover' - path: '/discover' - fullPath: '/discover' - preLoaderRoute: typeof DiscoverRouteImport + preLoaderRoute: typeof MainRouteImport parentRoute: typeof rootRouteImport } '/login': { @@ -170,26 +161,33 @@ declare module '@tanstack/react-router' { preLoaderRoute: typeof LoginRouteImport parentRoute: typeof rootRouteImport } - '/search': { - id: '/search' - path: '/search' - fullPath: '/search' - preLoaderRoute: typeof SearchRouteImport - parentRoute: typeof rootRouteImport + '/_main/': { + id: '/_main/' + path: '/' + fullPath: '/' + preLoaderRoute: typeof MainIndexRouteImport + parentRoute: typeof MainRoute } - '/_authenticated/account': { - id: '/_authenticated/account' - path: '/account' - fullPath: '/account' - preLoaderRoute: typeof AuthenticatedAccountRouteImport - parentRoute: typeof AuthenticatedRoute + '/_main/_authenticated': { + id: '/_main/_authenticated' + path: '' + fullPath: '/' + preLoaderRoute: typeof MainAuthenticatedRouteImport + parentRoute: typeof MainRoute } - '/_authenticated/files': { - id: '/_authenticated/files' - path: '/files' - fullPath: '/files' - preLoaderRoute: typeof AuthenticatedFilesRouteImport - parentRoute: typeof AuthenticatedRoute + '/_main/discover': { + id: '/_main/discover' + path: '/discover' + fullPath: '/discover' + preLoaderRoute: typeof MainDiscoverRouteImport + parentRoute: typeof MainRoute + } + '/_main/search': { + id: '/_main/search' + path: '/search' + fullPath: '/search' + preLoaderRoute: typeof MainSearchRouteImport + parentRoute: typeof MainRoute } '/api/$': { id: '/api/$' @@ -198,6 +196,20 @@ declare module '@tanstack/react-router' { preLoaderRoute: typeof ApiSplatRouteImport parentRoute: typeof rootRouteImport } + '/_main/_authenticated/account': { + id: '/_main/_authenticated/account' + path: '/account' + fullPath: '/account' + preLoaderRoute: typeof MainAuthenticatedAccountRouteImport + parentRoute: typeof MainAuthenticatedRoute + } + '/_main/_authenticated/files': { + id: '/_main/_authenticated/files' + path: '/files' + fullPath: '/files' + preLoaderRoute: typeof MainAuthenticatedFilesRouteImport + parentRoute: typeof MainAuthenticatedRoute + } '/login_/sso/$providerId': { id: '/login_/sso/$providerId' path: '/login/sso/$providerId' @@ -208,26 +220,38 @@ declare module '@tanstack/react-router' { } } -interface AuthenticatedRouteChildren { - AuthenticatedAccountRoute: typeof AuthenticatedAccountRoute - AuthenticatedFilesRoute: typeof AuthenticatedFilesRoute +interface MainAuthenticatedRouteChildren { + MainAuthenticatedAccountRoute: typeof MainAuthenticatedAccountRoute + MainAuthenticatedFilesRoute: typeof MainAuthenticatedFilesRoute +} + +const MainAuthenticatedRouteChildren: MainAuthenticatedRouteChildren = { + MainAuthenticatedAccountRoute: MainAuthenticatedAccountRoute, + MainAuthenticatedFilesRoute: MainAuthenticatedFilesRoute, +} + +const MainAuthenticatedRouteWithChildren = + MainAuthenticatedRoute._addFileChildren(MainAuthenticatedRouteChildren) + +interface MainRouteChildren { + MainAuthenticatedRoute: typeof MainAuthenticatedRouteWithChildren + MainDiscoverRoute: typeof MainDiscoverRoute + MainSearchRoute: typeof MainSearchRoute + MainIndexRoute: typeof MainIndexRoute } -const AuthenticatedRouteChildren: AuthenticatedRouteChildren = { - AuthenticatedAccountRoute: AuthenticatedAccountRoute, - AuthenticatedFilesRoute: AuthenticatedFilesRoute, +const MainRouteChildren: MainRouteChildren = { + MainAuthenticatedRoute: MainAuthenticatedRouteWithChildren, + MainDiscoverRoute: MainDiscoverRoute, + MainSearchRoute: MainSearchRoute, + MainIndexRoute: MainIndexRoute, } -const AuthenticatedRouteWithChildren = AuthenticatedRoute._addFileChildren( - AuthenticatedRouteChildren, -) +const MainRouteWithChildren = MainRoute._addFileChildren(MainRouteChildren) const rootRouteChildren: RootRouteChildren = { - IndexRoute: IndexRoute, - AuthenticatedRoute: AuthenticatedRouteWithChildren, - DiscoverRoute: DiscoverRoute, + MainRoute: MainRouteWithChildren, LoginRoute: LoginRoute, - SearchRoute: SearchRoute, ApiSplatRoute: ApiSplatRoute, LoginSsoProviderIdRoute: LoginSsoProviderIdRoute, } diff --git a/apps/web/src/router.tsx b/apps/web/src/router.tsx index 4c213e150..4e8269114 100644 --- a/apps/web/src/router.tsx +++ b/apps/web/src/router.tsx @@ -9,6 +9,7 @@ import { isTanStackOwnedPath } from './lib/migration-navigation' import { pluginRouteSpecs, withPluginRoutes } from './lib/plugin-routes' import { pluginRouteManifest } from './plugin-route-manifest.gen' import { pluginRouteModules } from './plugin-routes.gen' +import { Route as mainShellRoute } from './routes/_main' import { routeTree as fileRouteTree } from './routeTree.gen' /** @@ -22,10 +23,23 @@ import { routeTree as fileRouteTree } from './routeTree.gen' * The plugin half comes from two generated files and is joined by route id. No * plugin page is copied into `src/routes`, no route path is written by hand, and * nothing here knows which plugins are installed - see `lib/plugin-routes.ts`. + * + * They mount under `_main` rather than under the root, which is the whole of + * what "a plugin route renders in the application shell" amounts to here: a + * plugin declares `area: "main"`, `_main` is the route that renders the main + * shell, and being a child of it is what gives `/example` the header, the + * breadcrumb area and the one `
` that `/discover` has. No new field, no + * per-route layout metadata, and no second copy of the shell - route + * composition, which the area declaration already described. + * + * `_main` is imported for its route object, and it is the same object the + * generated tree holds: `createFileRoute` produces one instance per module and + * `routeTree.gen.ts` mutates it in place. */ const routeTree = withPluginRoutes( fileRouteTree, pluginRouteSpecs(pluginRouteManifest, pluginRouteModules), + mainShellRoute, ) /** diff --git a/apps/web/src/routes/__root.tsx b/apps/web/src/routes/__root.tsx index c70856907..4547a54e1 100644 --- a/apps/web/src/routes/__root.tsx +++ b/apps/web/src/routes/__root.tsx @@ -16,6 +16,7 @@ import { VitNodeWebSocketProvider } from '@vitnode/core/ws/provider' import { IntlProvider as NextIntlProvider } from 'next-intl' import { IntlProvider } from 'use-intl' +import { RealtimeListeners } from '#/components/realtime-listeners' import { publicPathnameOf, resolveLocale, useLocale } from '#/lib/i18n/client' import { intlQueryOptions } from '#/lib/i18n/query' import { vitNodeShellConfig } from '#/vitnode.shell.config' @@ -122,6 +123,15 @@ export const Route = createRootRouteWithContext()({ * * The QueryClient is deliberately absent: the router owns it and the SSR * integration mounts its provider above this tree. + * + * ## Why `RealtimeListeners` is here rather than in the shell + * + * It is the one non-provider in this tree, and it is here for the same reason + * every provider is: its lifetime is the WebSocket connection's, not any route's. + * The main shell is not mounted on `/login`, so a sync that lived there would + * miss the sign-in that happens on it - see the long note in + * `#/components/realtime-listeners`, which owns that argument. Inside the + * provider, because that is the context it reads. */ function RootComponent() { const locale = useLocale() @@ -137,6 +147,7 @@ function RootComponent() { + diff --git a/apps/web/src/routes/_main.tsx b/apps/web/src/routes/_main.tsx new file mode 100644 index 000000000..b792f724b --- /dev/null +++ b/apps/web/src/routes/_main.tsx @@ -0,0 +1,103 @@ +import { createFileRoute, Outlet } from '@tanstack/react-router' +import { ThemeLayoutContent } from '@vitnode/core/views/layouts/theme/layout-content' + +import { headerIntlQueryOptions } from '#/components/header' +import { MainBreadcrumb } from '#/components/layout/main-breadcrumb' +import { MainHeader } from '#/components/layout/main-header' +import { prefetchSession } from '#/lib/auth/query' + +/** + * The main application shell - the header, the breadcrumb area and the one + * `
` landmark that every public page renders inside. + * + * Pathless, so it contributes no URL segment: `/discover` is `/discover`, not + * `/_main/discover`. A page joins the shell by *where its file lives*, which is + * the same rule `_authenticated` uses for the session guard - and the reason + * `_authenticated` now lives underneath this one: a signed-in page is still a + * page on the public site, so it wants the shell *and* the guard rather than a + * second copy of the shell. + * + * ## What is deliberately outside it + * + * `/login` and `/login/sso/$providerId`. An auth screen is a full-height card on + * an otherwise empty document, and the header it would render is a header whose + * only interesting control is "sign in". Keeping them out is what makes this a + * shell that routes opt into rather than one every route is subject to - and it + * is what `/register` and the password-reset screens will want when they move. + * `routes/api/$` is outside for a different reason: it is a server route and + * renders no document at all. + * + * Note this is a visual difference from the Next.js app, where `/login` sits + * inside `(main)` and does render the header. + * + * ## The slots + * + * `ThemeLayoutContent`'s, and two of the same three the Next.js `ThemeLayout` + * fills: `header` and `breadcrumb`. + * + * `listeners` is deliberately left empty here. The Next.js app puts the + * notification toasts and the WebSocket's sign-in resync in it because its + * `/login` is inside the main shell; this app's is not, so a sync mounted here + * would not exist during the sign-in it has to notice. They are mounted by + * `__root` instead, next to the connection whose lifetime they share - see + * `#/components/realtime-listeners`. The slot stays in the shared component for + * the framework that has somewhere to put it. + * + * ## What it is not + * + * A provider. Every technical provider this app has - the QueryClient, the two + * intl records, the theme, the WebSocket - is mounted once by `__root`, above + * every route, because a login screen needs them just as much as a page under + * this shell does. What lives here is structure: markup, and where the slots go. + */ +export const Route = createFileRoute('/_main')({ + component: MainLayout, + /** + * What the header needs, warmed before anything renders. + * + * Both entries are the app's canonical ones - the same query definitions the + * routes below and the auth guards already use - so this adds no second key + * and no second request. What it adds is *timing*: the header sits above every + * page in the shell, so anything it reads has to be in hand before the first + * paint or the shell pays a round trip that the page below it did not. + * + * ## One `ensure`, one `prefetch`, and the difference matters + * + * `ensureQueryData` for the messages, because `Header` reads them with + * `useSuspenseQuery` and there is no Suspense boundary between it and the + * document. An unwarmed entry there does not degrade - it suspends the whole + * response. The namespace list is part of the query key, which is why the + * options come from `headerIntlQueryOptions` rather than being spelled out: a + * loader that warmed a different set would warm a key nobody reads. + * + * `prefetchQuery` for the session, through `prefetchSession`, because a + * failure must not take the page down with it. `ensureAuthState` is the wrong + * tool here in one specific way: it *rejects* when the session cannot be read, + * which is exactly right for a guard - an outage must not sign anybody out - + * and exactly wrong for a shell, where the same rejection would replace every + * page on the site with an error screen because the header could not name the + * visitor. Prefetching records the failure in the cache entry instead, and + * `userHeaderState` renders it as the guest controls. + * + * So the session is a performance concern here and a correctness one in + * `_authenticated`, and both reach the one entry `lib/auth/query.ts` owns. + * + * `Promise.all`, because neither read depends on the other. + */ + loader: async ({ context }) => { + await Promise.all([ + context.queryClient.ensureQueryData( + headerIntlQueryOptions({ locale: context.locale }), + ), + prefetchSession(context.queryClient), + ]) + }, +}) + +function MainLayout() { + return ( + } header={}> + + + ) +} diff --git a/apps/web/src/routes/_authenticated.tsx b/apps/web/src/routes/_main/_authenticated.tsx similarity index 98% rename from apps/web/src/routes/_authenticated.tsx rename to apps/web/src/routes/_main/_authenticated.tsx index c8fd57813..d0caf498b 100644 --- a/apps/web/src/routes/_authenticated.tsx +++ b/apps/web/src/routes/_main/_authenticated.tsx @@ -48,7 +48,7 @@ import { canAccessAuthenticatedRoute } from '#/lib/auth/shared' * decided on, from the same cache entry, so a page cannot disagree with the * guard that let it render. */ -export const Route = createFileRoute('/_authenticated')({ +export const Route = createFileRoute('/_main/_authenticated')({ beforeLoad: async ({ context, location }) => { const auth = await ensureAuthState(context.queryClient) diff --git a/apps/web/src/routes/_authenticated/account.tsx b/apps/web/src/routes/_main/_authenticated/account.tsx similarity index 96% rename from apps/web/src/routes/_authenticated/account.tsx rename to apps/web/src/routes/_main/_authenticated/account.tsx index 937eef949..30cbef011 100644 --- a/apps/web/src/routes/_authenticated/account.tsx +++ b/apps/web/src/routes/_main/_authenticated/account.tsx @@ -35,7 +35,7 @@ import { vitNodeShellConfig } from '#/vitnode.shell.config' * * Delete it when a real account page arrives. */ -export const Route = createFileRoute('/_authenticated/account')({ +export const Route = createFileRoute('/_main/_authenticated/account')({ // No loader and no `RouteMessages`: everything this page renders comes from // `core.global`, which the root route already warms and provides. head: () => ({ @@ -58,7 +58,7 @@ function AccountRoute() { const signOut = useSignOutAction() return ( -
+

{auth.user.name} @@ -98,6 +98,6 @@ function AccountRoute() {

-
+ ) } diff --git a/apps/web/src/routes/_authenticated/files.tsx b/apps/web/src/routes/_main/_authenticated/files.tsx similarity index 98% rename from apps/web/src/routes/_authenticated/files.tsx rename to apps/web/src/routes/_main/_authenticated/files.tsx index 7dff5bf0f..357e3e4d8 100644 --- a/apps/web/src/routes/_authenticated/files.tsx +++ b/apps/web/src/routes/_main/_authenticated/files.tsx @@ -83,7 +83,7 @@ import { vitNodeShellConfig } from '#/vitnode.shell.config' */ const FILES_NAMESPACES = ['core.files', 'core.global'] as const -export const Route = createFileRoute('/_authenticated/files')({ +export const Route = createFileRoute('/_main/_authenticated/files')({ component: MyFilesRoute, /** * The request, as the only thing the loader re-runs for. @@ -230,7 +230,7 @@ function MyFilesRoute() { return ( -
+
@@ -255,7 +255,7 @@ function MyFilesRoute() { onDeleteFiles={onDeleteFiles} /> -
+ ) } diff --git a/apps/web/src/routes/discover.tsx b/apps/web/src/routes/_main/discover.tsx similarity index 98% rename from apps/web/src/routes/discover.tsx rename to apps/web/src/routes/_main/discover.tsx index ca8102247..0e79571ba 100644 --- a/apps/web/src/routes/discover.tsx +++ b/apps/web/src/routes/_main/discover.tsx @@ -64,7 +64,7 @@ const DiscoverFeedLink = ({ ) -export const Route = createFileRoute('/discover')({ +export const Route = createFileRoute('/_main/discover')({ component: DiscoverRoute, /** * Both things this page needs, fetched in parallel before it renders. @@ -156,7 +156,7 @@ function DiscoverRoute() { return ( -
+
{/* @@ -171,7 +171,7 @@ function DiscoverRoute() { queryOptions={discoverFeedQueryOptions({ locale })} variant="timeline" /> -
+ ) } diff --git a/apps/web/src/routes/index.tsx b/apps/web/src/routes/_main/index.tsx similarity index 97% rename from apps/web/src/routes/index.tsx rename to apps/web/src/routes/_main/index.tsx index 2ebee757b..b310ab311 100644 --- a/apps/web/src/routes/index.tsx +++ b/apps/web/src/routes/_main/index.tsx @@ -24,7 +24,7 @@ import { vitNodeShellConfig } from '#/vitnode.shell.config' * * It is a scaffold. Stage 4 replaces it with the real homepage. */ -export const Route = createFileRoute('/')({ +export const Route = createFileRoute('/_main/')({ component: Home, // Per-route metadata, through the same title rule Next.js applies through // `title.template`: "Stage 3 - VitNode". @@ -68,7 +68,7 @@ function Home() { const { isFetching } = useQuery(intlQueryOptions({ locale })) return ( -
+

{vitNodeShellConfig.metadata.title} on TanStack Start @@ -144,6 +144,6 @@ function Home() { -

+ ) } diff --git a/apps/web/src/routes/search.tsx b/apps/web/src/routes/_main/search.tsx similarity index 98% rename from apps/web/src/routes/search.tsx rename to apps/web/src/routes/_main/search.tsx index 9e7164183..48b1d568b 100644 --- a/apps/web/src/routes/search.tsx +++ b/apps/web/src/routes/_main/search.tsx @@ -82,7 +82,7 @@ const SearchFeedLink = ({ children, className, href }: SearchFeedLinkProps) => ( ) -export const Route = createFileRoute('/search')({ +export const Route = createFileRoute('/_main/search')({ component: SearchRoute, /** * The loader re-runs when the term in the URL changes, and only then. @@ -182,7 +182,7 @@ function SearchRoute() { return ( -
+
{/* @@ -207,7 +207,7 @@ function SearchRoute() { LinkComponent={SearchFeedLink} variant="timeline" /> -
+ ) } diff --git a/apps/web/src/tests/discover-route.test.ts b/apps/web/src/tests/discover-route.test.ts index b78633fc5..ab1341918 100644 --- a/apps/web/src/tests/discover-route.test.ts +++ b/apps/web/src/tests/discover-route.test.ts @@ -119,8 +119,17 @@ const createSearchApi = () => { return c.json(answerFeed(url.searchParams)) }) + // The main shell's header reads the session, so a document render asks for it + // whether or not the page under the header cares. Answered as a guest, which + // is what `/discover` is rendered as here - and answered at all, because a + // fixture that 404s it is modelling an API outage rather than a page load, and + // the header would render its placeholder for the whole document. + const core = new Hono() + core.get('/users/session', (c) => c.json({ ai: { models: [] }, user: null })) + const app = new Hono().basePath('/api') app.route(`/${PLUGIN_ID}`, plugin) + app.route('/@vitnode/core', core) return app } @@ -144,12 +153,33 @@ const h1Of = (html: string): string | undefined => const titleOf = (html: string): string | undefined => /]*>([^<]*)`, for assertions about document metadata. */ +const headOf = (html: string): string => html.split('')[0] + const metaOf = (html: string, name: string): string | undefined => new RegExp(` - [...html.matchAll(/href="(\/[^"]*)"/g)].map(([, href]) => href) + [...html.matchAll(/href="([^"]*)"/g)].flatMap(([, href]) => { + try { + return [new URL(href, 'https://vitnode.invalid').pathname] + } catch { + // Not a URL at all - `href="#"` and friends. Nothing to say about a path. + return [] + } + }) /** * Runs `handler` inside a request the way the server runtime does, so the @@ -205,7 +235,7 @@ describe('one route serves both public URLs', () => { id.includes('discover'), ) - expect(ids).toEqual(['/discover']) + expect(ids).toEqual(['/_main/discover']) }) it('answers both URLs with the page rather than a 404', async () => { @@ -338,10 +368,16 @@ describe('the metadata is the request’s language', () => { ) }) - it('leaves one title in the document, not the shell’s as well', async () => { + it('leaves one title in the head, not the shell’s as well', async () => { const { html } = await renderDiscover('/discover') - expect(html.match(/`, + // and `<title>` is how an inline SVG gets an accessible name. What this + // guards against is the route's title and the root's default *both* being + // emitted - the failure `formatPageTitle` exists to prevent - which is a + // question about the head. + expect(headOf(html).match(/<title/g)).toHaveLength(1) }) it('asks to be indexed and followed, in both languages', async () => { @@ -536,7 +572,7 @@ describe('switching language stays on the page', () => { // Internally it never moved: `/discover` and `/pl/discover` are one route, // which is why the switch is an `invalidate` rather than a navigation. expect(router.state.location.pathname).toBe('/discover') - expect(router.state.matches.at(-1)?.routeId).toBe('/discover') + expect(router.state.matches.at(-1)?.routeId).toBe('/_main/discover') }) it('brings the loader context in step with the new URL', async () => { diff --git a/apps/web/src/tests/env-plugin.test.ts b/apps/web/src/tests/env-plugin.test.ts index 1b4479d31..9f5be5a5f 100644 --- a/apps/web/src/tests/env-plugin.test.ts +++ b/apps/web/src/tests/env-plugin.test.ts @@ -17,6 +17,10 @@ const ENV_FILE = [ const TOUCHED = [ 'CRON_SECRET', 'NEXT_PUBLIC_API_URL', + // Published to the client bundle like the two below, so it has to be cleared + // like them: left alone, a developer's own `.env` decides whether the define + // map this file asserts on says `undefined` or their legacy origin. + 'NEXT_PUBLIC_LEGACY_WEB_URL', 'NEXT_PUBLIC_UNLISTED', 'NEXT_PUBLIC_WEB_URL', 'POSTGRES_URL', diff --git a/apps/web/src/tests/header-navigation.test.ts b/apps/web/src/tests/header-navigation.test.ts new file mode 100644 index 000000000..207e26f9e --- /dev/null +++ b/apps/web/src/tests/header-navigation.test.ts @@ -0,0 +1,103 @@ +import { createMemoryHistory } from '@tanstack/react-router' +import { + HEADER_HREF, + headerNavItems, +} from '@vitnode/core/views/layouts/theme/header/header-nav' +import { describe, expect, it } from 'vitest' + +import { switchLocaleOn } from '#/lib/i18n/client' +import { isTanStackOwnedPath } from '#/lib/migration-navigation' +import { getRouter } from '#/router' + +/** + * A router on a given public URL, the way the server builds one per request - + * `createStartHandler` does exactly this, so what these tests drive is the real + * route tree rather than a stand-in. + */ +const routerAt = (publicHref: string) => { + const router = getRouter() + router.update({ + ...router.options, + history: createMemoryHistory({ initialEntries: [publicHref] }), + }) + + return router +} + +/** Everywhere the header points: the logo, then the nav, in render order. */ +const HEADER_LINKS = [ + HEADER_HREF.home, + ...headerNavItems({ discover: 'Discover', search: 'Search' }).map( + (item) => item.href, + ), +] + +/** + * Which mechanism each header link navigates by. + * + * The header renders `MigrationLink`, which asks the route tree per href rather + * than reading a list of migrated routes - so this is the question that decides + * whether clicking "Discover" is a client-side transition or a full document + * load into the Next.js app. All three of the header's destinations are this + * app's today; the assertion is here so that stops being an assumption. + */ +describe('every header link is a client-side navigation', () => { + it.each(HEADER_LINKS)('%s is served by this route tree', (href) => { + expect(isTanStackOwnedPath(routerAt('/'), href)).toBe(true) + }) + + it.each(HEADER_LINKS)('%s is still owned when locale-prefixed', (href) => { + // The prefix comes off before matching, which is the whole of Stage 3 - a + // header rendered on `/pl` must not decide its own links are somebody + // else's. + const prefixed = href === '/' ? '/pl' : `/pl${href}` + + expect(isTanStackOwnedPath(routerAt('/pl'), prefixed)).toBe(true) + }) + + it('does not claim a route the Next.js app still serves', () => { + // The control: without it, a rule that answered `true` for everything would + // satisfy every assertion above - and would turn a working blog post into a + // TanStack not-found. + expect(isTanStackOwnedPath(routerAt('/'), '/blog/post-30')).toBe(false) + }) +}) + +/** + * The language switcher, from the routes the header actually renders on. + * + * `locale-rewrite.test.ts` pins the rule itself on `/`; these are the two cases + * the header exists to make reachable, on a real page and with a query string + * and a hash to lose. Nothing here manipulates a path - `switchLocaleOn` is + * Stage 3's own function, and the point is that the header needs no path + * handling of its own. + */ +describe('switching language keeps the visitor where they are', () => { + it('adds the prefix, keeping the search string and the hash', async () => { + const router = routerAt('/discover?x=1#feed') + + await switchLocaleOn(router, 'pl') + + expect(router.latestLocation.publicHref).toBe('/pl/discover?x=1#feed') + // The route tree never saw a locale, before or after. + expect(router.latestLocation.pathname).toBe('/discover') + }) + + it('removes the prefix again for the default locale', async () => { + const router = routerAt('/pl/search') + + await switchLocaleOn(router, 'en') + + expect(router.latestLocation.publicHref).toBe('/search') + expect(router.latestLocation.pathname).toBe('/search') + }) + + it('is a history entry rather than a document load', async () => { + const router = routerAt('/discover') + const before = router.history.length + + await switchLocaleOn(router, 'pl') + + expect(router.history.length).toBe(before + 1) + }) +}) diff --git a/apps/web/src/tests/isolation.test.ts b/apps/web/src/tests/isolation.test.ts index bea90ee6c..620893352 100644 --- a/apps/web/src/tests/isolation.test.ts +++ b/apps/web/src/tests/isolation.test.ts @@ -361,8 +361,8 @@ describe('the whole graph this app imports stays Next-free', () => { 'apps/web/src/lib/auth/redirects.ts', 'apps/web/src/lib/auth/screens.ts', 'apps/web/src/lib/middleware-config.ts', - 'apps/web/src/routes/_authenticated.tsx', - 'apps/web/src/routes/_authenticated/account.tsx', + 'apps/web/src/routes/_main/_authenticated.tsx', + 'apps/web/src/routes/_main/_authenticated/account.tsx', 'apps/web/src/routes/login.tsx', 'apps/web/src/routes/login_.sso.$providerId.tsx', // Stage 7. `/files` renders the whole data table - eight columns, the @@ -372,7 +372,7 @@ describe('the whole graph this app imports stays Next-free', () => { // `React.lazy`, so it is exactly the graph worth walking here. 'apps/web/src/lib/files/my-files-route.ts', 'apps/web/src/lib/files/my-files.ts', - 'apps/web/src/routes/_authenticated/files.tsx', + 'apps/web/src/routes/_main/_authenticated/files.tsx', 'apps/web/src/server/my-files.server.ts', 'apps/web/src/lib/i18n/client.ts', 'apps/web/src/lib/i18n/query.ts', @@ -383,9 +383,12 @@ describe('the whole graph this app imports stays Next-free', () => { 'apps/web/src/lib/search/search-request.ts', 'apps/web/src/router.tsx', 'apps/web/src/routes/__root.tsx', - 'apps/web/src/routes/discover.tsx', - 'apps/web/src/routes/index.tsx', - 'apps/web/src/routes/search.tsx', + // Stage 8. The main shell, and with it the header and breadcrumb slots the + // pages under it render inside. + 'apps/web/src/routes/_main.tsx', + 'apps/web/src/routes/_main/discover.tsx', + 'apps/web/src/routes/_main/index.tsx', + 'apps/web/src/routes/_main/search.tsx', 'apps/web/src/server/search-feed.server.ts', 'apps/web/src/server/locale.server.ts', 'apps/web/src/server/messages.server.ts', @@ -443,7 +446,7 @@ describe('the whole graph this app imports stays Next-free', () => { * failure names the specifier rather than "something in this list". */ describe('the /discover runtime graph reaches no Next.js', () => { - const DISCOVER = ['apps/web/src/routes/discover.tsx'] + const DISCOVER = ['apps/web/src/routes/_main/discover.tsx'] it('walks into the shared components the route renders', () => { // Without this the assertions below would pass on a graph that stopped at @@ -504,7 +507,7 @@ describe('the whole graph this app imports stays Next-free', () => { * that reason until the controls became `SearchControlsContent`. */ describe('the /search runtime graph reaches no Next.js', () => { - const SEARCH = ['apps/web/src/routes/search.tsx'] + const SEARCH = ['apps/web/src/routes/_main/search.tsx'] it('walks into the shared controls the route renders', () => { // Without this the assertions below would pass on a graph that stopped at @@ -574,7 +577,7 @@ describe('the whole graph this app imports stays Next-free', () => { * was visible from the route file. */ describe('the /files runtime graph reaches no Next.js', () => { - const FILES = ['apps/web/src/routes/_authenticated/files.tsx'] + const FILES = ['apps/web/src/routes/_main/_authenticated/files.tsx'] it('walks into the table and the dialogs the route renders', () => { // Without this the assertions below would pass on a graph that stopped at diff --git a/apps/web/src/tests/locale-rewrite.test.ts b/apps/web/src/tests/locale-rewrite.test.ts index 9994e6ff3..e76200b8d 100644 --- a/apps/web/src/tests/locale-rewrite.test.ts +++ b/apps/web/src/tests/locale-rewrite.test.ts @@ -45,7 +45,7 @@ describe('one route tree, two public URL shapes', () => { expect(router.latestLocation.publicHref).toBe(publicHref) expect( router.matchRoutes(router.latestLocation.pathname).at(-1)?.routeId, - ).toBe('/') + ).toBe('/_main/') expect(router.state.location.pathname).toBe(pathname) // The locale the rest of the app reads, from the same location. expect( diff --git a/apps/web/src/tests/main-shell.test.ts b/apps/web/src/tests/main-shell.test.ts new file mode 100644 index 000000000..c7f198843 --- /dev/null +++ b/apps/web/src/tests/main-shell.test.ts @@ -0,0 +1,250 @@ +import { existsSync, readdirSync, statSync } from 'node:fs' +import { dirname, join, relative, resolve, sep } from 'node:path' +import { fileURLToPath } from 'node:url' +import { describe, expect, it } from 'vitest' + +import type { BreadcrumbMatch } from '#/lib/breadcrumb' + +import { breadcrumbOf } from '#/lib/breadcrumb' +import { getRouter } from '#/router' + +import { withoutComments } from './source' + +const here = dirname(fileURLToPath(import.meta.url)) +const routesDir = resolve(here, '../routes') +const pluginsDir = resolve(here, '../../../../plugins') + +/** The pathless route that renders the header, the breadcrumb area and `<main>`. */ +const MAIN_SHELL_ROUTE_ID = '/_main' + +const matchedIds = (pathname: string): string[] => + getRouter() + .matchRoutes(pathname, undefined) + .map((match) => match.routeId) + +/** + * Which routes render inside the main application shell, and which do not. + * + * A route joins the shell by where its file lives - `routes/_main/search.tsx` is + * `/search`, in the shell - so the policy is not written down anywhere except + * the route tree itself. That is the point of asserting it here: the tree is the + * declaration, and this is the sentence it declares, in one place, where moving + * a file out of the shell by accident fails a test rather than silently removing + * a page's header. + * + * `matchRoutes` runs no `beforeLoad`, so the two guarded pages are matched here + * without a session. What is being asserted is the parent chain, not access. + */ +describe('the main shell is what a public page renders inside', () => { + it.each([ + ['/', 'the front page'], + ['/discover', 'the discover feed'], + ['/search', 'the search page'], + ['/account', 'a page behind the session guard'], + ['/files', 'the files table, behind the same guard'], + ['/example', "a plugin's page, mounted by area rather than by file"], + ])('%s renders in the shell (%s)', (pathname) => { + expect(matchedIds(pathname)).toContain(MAIN_SHELL_ROUTE_ID) + }) + + /** + * An auth screen is a full-height card on an otherwise empty document, and the + * header it would render has one interesting control on it: "sign in". Keeping + * these out is what makes the shell something routes opt into - and it is the + * shape `/register` and the password-reset screens will want when they move. + */ + it.each([ + ['/login', 'the login screen'], + ['/login/sso/google', 'the SSO callback'], + ])('%s renders outside it (%s)', (pathname) => { + expect(matchedIds(pathname)).not.toContain(MAIN_SHELL_ROUTE_ID) + }) + + /** + * The guard sits *under* the shell rather than beside it, which is what stops + * `/files` from needing a second copy of the header. Asserted as order: the + * shell is matched before the guard, so it is the guard's parent. + */ + it('puts the session guard inside the shell rather than next to it', () => { + const matched = matchedIds('/files') + + expect(matched.indexOf(MAIN_SHELL_ROUTE_ID)).toBeGreaterThanOrEqual(0) + expect(matched.indexOf(MAIN_SHELL_ROUTE_ID)).toBeLessThan( + matched.indexOf(`${MAIN_SHELL_ROUTE_ID}/_authenticated`), + ) + }) +}) + +const routeFiles = (directory: string): string[] => + readdirSync(directory).flatMap((name) => { + const path = join(directory, name) + + if (statSync(path).isDirectory()) return routeFiles(path) + + return name.endsWith('.tsx') ? [path] : [] + }) + +/** + * One `<main>` per document, and the shell owns it. + * + * A page that renders its own `<main>` inside a shell that also renders one + * produces two: invalid HTML, and a screen reader with two "main" landmarks to + * choose from. A page under the shell keeps its container - its width, its + * padding, its vertical rhythm - as a `<div>`. + * + * `<main>` is not something a type can forbid, so this reads the source. Crude, + * and exactly as crude as the mistake it catches. + */ +describe('the shell owns the main landmark', () => { + /** + * The file's code, with its comments removed. + * + * Every one of these routes *documents* the landmark it does or does not + * render, so a scan of the raw source would find `<main>` in the prose above + * the component and fail on the explanation rather than on the markup. + */ + const landmarks = (code: string): string[] => code.match(/<main[\s>]/g) ?? [] + + const under = (directory: string) => + routeFiles(directory).map((path) => ({ + code: withoutComments(path), + name: relative(routesDir, path).split(sep).join('/'), + })) + + it.each(under(join(routesDir, '_main')))( + '$name renders no <main> of its own', + ({ code }) => { + expect(landmarks(code)).toEqual([]) + }, + ) + + it('renders no <main> in the shell route either - it comes from core', () => { + // `ThemeLayoutContent` is the one place the landmark is written, shared with + // the Next.js app so the two runtimes produce the same document. + const code = withoutComments(join(routesDir, '_main.tsx')) + + expect(landmarks(code)).toEqual([]) + expect(code).toContain('ThemeLayoutContent') + }) + + /** + * The routes outside the shell own theirs, and must: without a shell above + * them, a login screen with no `<main>` is a document with no main landmark at + * all. + */ + it.each(['login.tsx', 'login_.sso.$providerId.tsx'])( + '%s renders exactly one <main> of its own', + (name) => { + expect(landmarks(withoutComments(join(routesDir, name)))).toHaveLength(1) + }, + ) + + /** + * The same rule, for the pages this app does not own. + * + * A plugin route declares `area: "main"`, which mounts it inside the shell - + * so a plugin page that renders a `<main>` produces the nested landmark from + * source this app cannot edit. Scanned rather than trusted, because the + * failure is invisible: the page renders, the HTML is merely wrong. + */ + const pluginRouteFiles = readdirSync(pluginsDir) + .map((name) => join(pluginsDir, name, 'src', 'routes')) + .filter((path) => existsSync(path)) + .flatMap(routeFiles) + + it('finds the plugin route modules it means to scan', () => { + // Without this the assertion below passes on an empty list. + expect(pluginRouteFiles.length).toBeGreaterThan(0) + }) + + it.each(pluginRouteFiles)('%s renders no <main> of its own', (path) => { + expect(landmarks(withoutComments(path))).toEqual([]) + }) +}) + +/** + * The breadcrumb rule, as a function over plain data. + * + * Deepest declaring match wins, which is the same answer Next's `@breadcrumb` + * slot gives: the deepest folder with a `page.tsx`, with everything above it + * falling through. Tested without a router because it *is* a function over + * `{ staticData }` - see `#/lib/breadcrumb`. + */ +describe('a route declares its own breadcrumb, and the deepest one wins', () => { + const root = 'root crumb' + const leaf = 'leaf crumb' + + const match = (...declared: React.ReactNode[]): BreadcrumbMatch => ({ + // Spread rather than an optional parameter, so "declared `null`" and + // "declared nothing" are two different calls rather than one value. + staticData: declared.length > 0 ? { breadcrumb: declared[0] } : {}, + }) + + it('takes the deepest declaration, not the first', () => { + expect(breadcrumbOf([match(root), match(), match(leaf)])).toBe(leaf) + }) + + it('falls back to an ancestor when the leaf declares nothing', () => { + expect(breadcrumbOf([match(root), match(), match()])).toBe(root) + }) + + /** + * The legacy slot's `page.tsx` returning `null` - `/` has one, so that a + * client-side navigation home clears the crumb the previous page rendered. + */ + it('lets a child clear an ancestor’s crumb by declaring null', () => { + expect(breadcrumbOf([match(root), match(null)])).toBeNull() + }) + + it('answers with nothing when no match declares one', () => { + expect(breadcrumbOf([match(), match()])).toBeNull() + }) + + it('answers with nothing for no matches at all', () => { + expect(breadcrumbOf([])).toBeNull() + }) +}) + +/** + * What the shell warms before it renders, and why each one is the call it is. + * + * The header is above every page in the shell, so both of its reads have to be + * in hand before the first paint. They are also the two reads whose *failure + * modes* differ, and the pair is easy to get subtly wrong in either direction: + * + * headerIntlQueryOptions ensure a `useSuspenseQuery` with no boundary + * between it and the document + * session prefetch a rejection here would replace every + * page on the site with an error screen + * + * `ensureAuthState` is the tempting call for the second one - it is what + * `_authenticated` uses two routes down - and it is wrong here for exactly the + * reason it is right there. A source scan is the honest way to pin that: what is + * being asserted is which function the loader calls, and both reach the same + * cache entry, so no observable behaviour distinguishes them until the API is + * down. + */ +describe('the shell warms what the header reads', () => { + // The prose in that file discusses `ensureAuthState` at length in order to + // explain why it is the wrong call here, so a scan that read the comments + // would find the very thing it is asserting the absence of. + const shell = withoutComments(join(routesDir, '_main.tsx')) + + it('ensures the header’s messages, whose absence would suspend the document', () => { + expect(shell).toContain('headerIntlQueryOptions') + expect(shell).toContain('ensureQueryData') + }) + + it('takes the message options from the header rather than restating them', () => { + // The namespace list is part of the query key, so a loader that spelled its + // own out would warm a key the header never reads. + expect(shell).toContain( + "import { headerIntlQueryOptions } from '#/components/header'", + ) + }) + + it('prefetches the session rather than ensuring it', () => { + expect(shell).toContain('prefetchSession') + expect(shell).not.toContain('ensureAuthState') + }) +}) diff --git a/apps/web/src/tests/my-files-route.test.ts b/apps/web/src/tests/my-files-route.test.ts index 694bccdfd..eb1691a38 100644 --- a/apps/web/src/tests/my-files-route.test.ts +++ b/apps/web/src/tests/my-files-route.test.ts @@ -429,15 +429,16 @@ describe('`/files` is this app’s route now', () => { expect(isTanStackOwnedPath(router, '/files/12')).toBe(false) }) - it('sits under the pathless guard rather than at the top of the tree', () => { + it('sits under the main shell and the pathless guard, not at the top of the tree', () => { const matched = router.matchRoutes('/files', undefined) as { routeId: string }[] expect(matched.map((match) => match.routeId)).toEqual([ '__root__', - '/_authenticated', - '/_authenticated/files', + '/_main', + '/_main/_authenticated', + '/_main/_authenticated/files', ]) }) }) diff --git a/apps/web/src/tests/plugin-routes.test.ts b/apps/web/src/tests/plugin-routes.test.ts index 133a65027..55566fa0b 100644 --- a/apps/web/src/tests/plugin-routes.test.ts +++ b/apps/web/src/tests/plugin-routes.test.ts @@ -343,6 +343,9 @@ describe('fileRoutePaths', () => { }) }) +/** The pathless route that renders the main application shell. */ +const MAIN_SHELL_ROUTE_ID = '/_main' + describe("the app's real route tree", () => { /** * The exit criterion, asserted against what the app actually ships rather than @@ -359,7 +362,27 @@ describe("the app's real route tree", () => { expect( router.matchRoutes('/example', undefined).map((match) => match.routeId), - ).toContain(`/${PLUGIN_ROUTES_ROUTE_ID}/example`) + ).toContain(`${MAIN_SHELL_ROUTE_ID}/${PLUGIN_ROUTES_ROUTE_ID}/example`) + }) + + /** + * Stage 8. A plugin route declares `area: "main"`, and this is what that + * declaration buys: the page renders inside the same shell `/discover` does - + * the header, the breadcrumb area and the one `<main>` - because the plugin + * container is a child of the `_main` route rather than of the root. + * + * Asserted as route *structure*, which is what decides it. Nothing renders + * here; the parent chain is the whole claim. + */ + it('renders the example plugin’s page inside the main shell', () => { + const matched = getRouter() + .matchRoutes('/example', undefined) + .map((match) => match.routeId) + + expect(matched).toContain(MAIN_SHELL_ROUTE_ID) + expect(matched.indexOf(MAIN_SHELL_ROUTE_ID)).toBeLessThan( + matched.findIndex((id) => id.endsWith('/example')), + ) }) /** diff --git a/apps/web/src/tests/realtime-user-id.test.ts b/apps/web/src/tests/realtime-user-id.test.ts new file mode 100644 index 000000000..291a6991d --- /dev/null +++ b/apps/web/src/tests/realtime-user-id.test.ts @@ -0,0 +1,129 @@ +import { dirname, resolve } from 'node:path' +import { fileURLToPath } from 'node:url' +import { describe, expect, it } from 'vitest' + +import type { SessionApi } from '#/lib/session' + +import { socketUserIdFromSession } from '#/lib/realtime' + +import { withoutComments } from './source' + +const here = dirname(fileURLToPath(import.meta.url)) + +/** + * The Stage 8 realtime contract, on the app's side of it: + * + * session (canonical query) -> socketUserIdFromSession -> WebSocketAuthSync + * + * Only this derivation is tested, and it is the only part worth testing here. + * What follows it is `shouldReconnectForUser` in `@vitnode/core/ws/auth-sync`, + * which has its own tests, and below that a WebSocket - and a fake socket proves + * nothing about a real handshake, which is where the identity is actually + * decided. + * + * What can go wrong on this side is the distinction between "signed out" and + * "not known yet". Both are falsy, both would read as a guest, and collapsing + * them re-opens the shared connection on every page load for every signed-in + * visitor - with no error anywhere to say so. + * + * `SessionApi` is imported as a type only, so nothing here loads the server + * function or the fetcher behind it. + */ + +/** + * A session in the shape the API returns, narrowed to what this reads. + * + * `as` rather than a full literal: `SessionApi` is inferred from the route's Zod + * schema and carries fields this function has no opinion about, and writing them + * out would be a second copy of that schema which typechecks while disagreeing + * with the server. + */ +const sessionOf = (user: null | { id: number }): SessionApi => + ({ user }) as unknown as SessionApi + +describe('socketUserIdFromSession', () => { + it('reports the signed-in visitor', () => { + expect(socketUserIdFromSession(sessionOf({ id: 42 }))).toBe(42) + }) + + it('reports a guest as null, not as unknown', () => { + // The API answered. `null` is a fact, and it is what makes a sign-out a + // transition the socket follows. + expect(socketUserIdFromSession(sessionOf(null))).toBeNull() + }) + + it('reports an unread session as unknown, not as a guest', () => { + // No cache entry, or a failed read that `retry: false` left without data. + // Answering `null` here would be inventing a guest - the mistake + // `lib/session.ts` refuses - and on a client-side read it is the first + // value of every page load. + expect(socketUserIdFromSession(undefined)).toBeUndefined() + }) + + it('keeps the three answers distinguishable', () => { + // The property the two tests above exist for, stated once: none of the + // three inputs may collapse into another. + const answers = [ + socketUserIdFromSession(undefined), + socketUserIdFromSession(sessionOf(null)), + socketUserIdFromSession(sessionOf({ id: 1 })), + ] + + expect(new Set(answers).size).toBe(3) + }) +}) + +/** + * Where the realtime listeners are mounted, which is a correctness question + * rather than a tidiness one. + * + * `WebSocketAuthSync` follows a sign-in by seeing the identity *change*: it + * seeds its ref from its first prop and reconnects only once both sides are + * known and differ. So it has to already be mounted when the sign-in happens. + * + * This app's `/login` is outside `_main`, so the shell is not mounted while a + * visitor signs in - it mounts for the first time at the destination, by which + * point `_main`'s loader has already prefetched the *new* session. The sync + * would seed from `42`, compare it against `42`, and never re-handshake: the + * socket keeps the guest identity it opened with until a full page load. + * + * The Next.js app does not have that problem, because its `/login` is inside + * `(main)` and the sync stays mounted across the transition. That difference is + * the whole reason this is asserted here: the shared `ThemeLayoutContent` has a + * `listeners` slot, filling it is the obvious thing to do, and doing it in this + * app is silently wrong. + * + * A source scan rather than a render: what is being pinned is *which module* + * mounts them, and mounting a WebSocket in jsdom would prove nothing about a + * handshake either way. + */ +describe('the realtime listeners are mounted by the root, not by the shell', () => { + const root = withoutComments(resolve(here, '../routes/__root.tsx')) + + it('mounts them at the root', () => { + expect(root).toContain( + "import { RealtimeListeners } from '#/components/realtime-listeners'", + ) + expect(root).toContain('<RealtimeListeners />') + }) + + it('mounts them inside the WebSocket provider, whose context they read', () => { + const provider = root.indexOf('<VitNodeWebSocketProvider>') + const listeners = root.indexOf('<RealtimeListeners />') + const closed = root.indexOf('</VitNodeWebSocketProvider>') + + expect(provider).toBeGreaterThanOrEqual(0) + expect(listeners).toBeGreaterThan(provider) + expect(listeners).toBeLessThan(closed) + }) + + it('does not fill the shell’s `listeners` slot with them', () => { + // The slot itself stays in `ThemeLayoutContent` for the Next.js app. What + // must not happen is this app filling it - see above. + const shell = withoutComments(resolve(here, '../routes/_main.tsx')) + + expect(shell).not.toContain('listeners=') + expect(shell).not.toContain('WebSocketAuthSync') + expect(shell).not.toContain('RealtimeListeners') + }) +}) diff --git a/apps/web/src/tests/router-query.test.ts b/apps/web/src/tests/router-query.test.ts index 2e8f238ec..09bbd2237 100644 --- a/apps/web/src/tests/router-query.test.ts +++ b/apps/web/src/tests/router-query.test.ts @@ -109,10 +109,11 @@ describe('the Query SSR integration is installed', () => { describe('nothing but the router creates a query client', () => { const appFiles = [ 'routes/__root.tsx', - 'routes/_authenticated/files.tsx', - 'routes/discover.tsx', - 'routes/index.tsx', - 'routes/search.tsx', + 'routes/_main.tsx', + 'routes/_main/_authenticated/files.tsx', + 'routes/_main/discover.tsx', + 'routes/_main/index.tsx', + 'routes/_main/search.tsx', 'components/route-messages.tsx', 'lib/files/my-files-route.ts', 'lib/files/my-files.ts', diff --git a/apps/web/src/tests/shell-config.test.ts b/apps/web/src/tests/shell-config.test.ts index 45fc786ec..9a3814cb0 100644 --- a/apps/web/src/tests/shell-config.test.ts +++ b/apps/web/src/tests/shell-config.test.ts @@ -2,7 +2,7 @@ import { formatPageTitle, titleTemplate } from '@vitnode/core/lib/metadata' import { describe, expect, it } from 'vitest' import { Route as RootRoute } from '#/routes/__root' -import { Route as IndexRoute } from '#/routes/index' +import { Route as IndexRoute } from '#/routes/_main/index' import { vitNodeShellConfig } from '#/vitnode.shell.config' type HeadTag = Record<string, string | undefined> diff --git a/apps/web/src/tests/source.ts b/apps/web/src/tests/source.ts new file mode 100644 index 000000000..889fe8d3d --- /dev/null +++ b/apps/web/src/tests/source.ts @@ -0,0 +1,26 @@ +import { readFileSync } from 'node:fs' + +/** + * A source file's code, with its comments removed. + * + * Several assertions in this suite are about *what a module does* - which route + * it mounts a component under, which of two nearly identical cache calls its + * loader makes, whether it renders a landmark - and the honest way to ask that + * is to read the source. The catch is that this codebase explains itself at + * length, and those explanations name the very things being looked for: the + * shell's loader discusses `ensureAuthState` in order to say why it is the wrong + * call, and every route that does *not* render a `<main>` says so in prose above + * the component. A raw scan fails on the explanation rather than on the code. + * + * Shared rather than redefined per file because three tests wanted it and the + * copies had already started to differ. + * + * Deliberately naive: it does not know that `//` can appear inside a string or a + * regex, and it does not need to - it is used on this repository's own sources, + * where a false strip would only ever remove code an assertion then fails to + * find. It is not a parser and must not be used as one. + */ +export const withoutComments = (path: string): string => + readFileSync(path, 'utf8') + .replace(/\/\*[\s\S]*?\*\//g, '') + .replace(/\/\/.*$/gm, '') diff --git a/packages/vitnode/src/components/switchers/langs/language-switcher-content.tsx b/packages/vitnode/src/components/switchers/langs/language-switcher-content.tsx new file mode 100644 index 000000000..e88dc2708 --- /dev/null +++ b/packages/vitnode/src/components/switchers/langs/language-switcher-content.tsx @@ -0,0 +1,120 @@ +"use client"; + +import { CheckIcon, LanguagesIcon } from "lucide-react"; +import { useTranslations } from "use-intl"; + +import type { LocaleConfig } from "@/lib/i18n/types"; + +import { Button } from "../../ui/button"; +import { + DropdownMenu, + DropdownMenuContent, + DropdownMenuItem, + DropdownMenuTrigger, +} from "../../ui/dropdown-menu"; + +/** + * The language switcher, minus the one thing that differs between frameworks. + * + * The dropdown, the icon, the check mark on the current language and the + * `core.global.language_switcher` label are the same control everywhere. What is + * not the same is *how* switching language moves the URL: Next.js replaces the + * current pathname through `next-intl`'s locale-aware router, TanStack Start + * pushes the public href and invalidates (`useSwitchLocale`, Stage 3). That is + * two lines, and they are the only two that are passed in. + * + * Before this existed `apps/web` carried its own copy of the markup with a + * comment explaining that copying it was cheaper than an abstraction satisfying + * both. It was - until the header needed the same control in both apps, at which + * point one dropdown with an `onSelect` is smaller than either. + * + * Framework-free: `use-intl` rather than `next-intl`, and no navigation import at + * all. `core.global` is mounted by both apps' root providers, so the label + * resolves in either. + */ + +/** + * The switch is ready: the current language is known and selecting one does + * something. + */ +interface LanguageSwitcherReadyProps { + /** The language the page is currently in, for the check mark. */ + currentLocale: string; + /** Perform the switch. Given a locale code from `options`. */ + onSelect: (locale: string) => void; +} + +/** + * The switch is not ready, and the items render disabled. + * + * This is not a loading state for the *data* - the language list is + * configuration and is always in hand. It is for the framework's navigation: + * Next.js resolves the current pathname from `usePathname()`, which is URL data, + * and Next 16 refuses to prerender a client component that reads it outside a + * `<Suspense>`. So the Next.js half renders this shape as its fallback and the + * real one inside the boundary - which is exactly the structure this control had + * before it was shared. + * + * Written as the other half of a union rather than as two independent optional + * props: "a current locale but no handler" is not a state this control has, and + * a caller that produced one would render a check mark next to items that do + * nothing. + */ +interface LanguageSwitcherPendingProps { + currentLocale?: never; + onSelect?: never; +} + +export type LanguageSwitcherContentProps = ( + LanguageSwitcherPendingProps | LanguageSwitcherReadyProps +) & { + /** A switch in flight, shown on the trigger. Next.js drives this with a transition. */ + isPending?: boolean; + /** The languages to offer, in the order they render. */ + options: LocaleConfig[]; +}; + +export const LanguageSwitcherContent = ({ + currentLocale, + isPending, + onSelect, + options, +}: LanguageSwitcherContentProps) => { + const t = useTranslations("core.global"); + + return ( + <DropdownMenu> + <DropdownMenuTrigger + render={ + <Button + aria-label={t("language_switcher")} + className="relative" + isLoading={isPending} + size="icon" + variant="ghost" + /> + } + > + <LanguagesIcon /> + </DropdownMenuTrigger> + + <DropdownMenuContent> + {options.map(option => ( + <DropdownMenuItem + disabled={!onSelect} + key={option.code} + onClick={() => { + onSelect?.(option.code); + }} + > + {option.name} + + {option.code === currentLocale && ( + <CheckIcon aria-hidden className="ml-auto" /> + )} + </DropdownMenuItem> + ))} + </DropdownMenuContent> + </DropdownMenu> + ); +}; diff --git a/packages/vitnode/src/components/switchers/langs/language-switcher.tsx b/packages/vitnode/src/components/switchers/langs/language-switcher.tsx index 7613a3bdb..17bd8a2b4 100644 --- a/packages/vitnode/src/components/switchers/langs/language-switcher.tsx +++ b/packages/vitnode/src/components/switchers/langs/language-switcher.tsx @@ -1,25 +1,35 @@ "use client"; -import { CheckIcon, LanguagesIcon } from "lucide-react"; -import { useLocale, useTranslations } from "next-intl"; +import { useLocale } from "next-intl"; import React from "react"; import type { LocaleConfig } from "@/vitnode.config"; import { usePathname, useRouter } from "@/lib/navigation"; -import { Button } from "../../ui/button"; -import { - DropdownMenu, - DropdownMenuContent, - DropdownMenuItem, - DropdownMenuTrigger, -} from "../../ui/dropdown-menu"; +import { LanguageSwitcherContent } from "./language-switcher-content"; -const LanguageSwitcherItems = ({ +/** + * The switch itself, and the reason it is its own component. + * + * `useRouter()` and `usePathname()` both read the current URL - `next-intl`'s + * router calls `usePathname()` internally to sync the locale cookie - and Next 16 + * refuses to prerender a client component that reads URL data outside a + * `<Suspense>`: `CLIENT_HOOK_DYNAMIC`. The AdminCP sidebar renders this switcher + * on a prerendered route, so the boundary below is not defensive. It is what + * makes `next build` pass, and it is why the control was already split this way + * before it was shared. + * + * `pathname` here is `next-intl`'s: the *un-prefixed* one, so replacing it with a + * different locale cannot produce `/pl/pl/...`. The search string and hash are + * Next's to preserve, as they always were. + */ +const NextLanguageSwitcher = ({ + isPending, locales, startTransition, }: { + isPending: boolean; locales: LocaleConfig[]; startTransition: React.TransitionStartFunction; }) => { @@ -27,57 +37,49 @@ const LanguageSwitcherItems = ({ const { replace } = useRouter(); const pathname = usePathname(); - return locales.map(locale => ( - <DropdownMenuItem - key={locale.code} - onClick={() => { + return ( + <LanguageSwitcherContent + currentLocale={currentLocale} + isPending={isPending} + onSelect={locale => { startTransition(() => { - replace(pathname, { - locale: locale.code, - }); + replace(pathname, { locale }); }); }} - > - {locale.name} - {locale.code === currentLocale && <CheckIcon className="ml-auto" />} - </DropdownMenuItem> - )); + options={locales} + /> + ); }; +/** + * {@link LanguageSwitcherContent}, wired to Next.js. + * + * Everything visible moved to the shared control; what is left is the navigation + * and the boundary it has to render inside. The fallback is the same component + * with its items disabled - the trigger, the icon and the language names, so the + * bar reserves its width and reads correctly before the URL is available. + * + * `useTransition` lives out here, above the boundary, so the spinner survives the + * navigation it is reporting on. + * + * The TanStack Start half is `apps/web/src/components/language-switcher.tsx`, + * over Stage 3's `useSwitchLocale` - which needs no boundary, because the router + * it reads is not Next's. + */ export const LanguageSwitcher = ({ locales }: { locales: LocaleConfig[] }) => { const [isPending, startTransition] = React.useTransition(); - const t = useTranslations("core.global"); return ( - <DropdownMenu> - <DropdownMenuTrigger - render={ - <Button - aria-label={t("language_switcher")} - className="relative" - isLoading={isPending} - size="icon" - variant="ghost" - /> - } - > - <LanguagesIcon /> - </DropdownMenuTrigger> - - <DropdownMenuContent> - <React.Suspense - fallback={locales.map(locale => ( - <DropdownMenuItem disabled key={locale.code}> - {locale.name} - </DropdownMenuItem> - ))} - > - <LanguageSwitcherItems - locales={locales} - startTransition={startTransition} - /> - </React.Suspense> - </DropdownMenuContent> - </DropdownMenu> + <React.Suspense + fallback={ + <LanguageSwitcherContent isPending={isPending} options={locales} /> + } + > + <NextLanguageSwitcher + isPending={isPending} + locales={locales} + startTransition={startTransition} + /> + </React.Suspense> ); }; diff --git a/packages/vitnode/src/views/layouts/theme/header/header-boundaries.test.ts b/packages/vitnode/src/views/layouts/theme/header/header-boundaries.test.ts new file mode 100644 index 000000000..b80640003 --- /dev/null +++ b/packages/vitnode/src/views/layouts/theme/header/header-boundaries.test.ts @@ -0,0 +1,303 @@ +// @vitest-environment node +import { existsSync, readFileSync, statSync } from "node:fs"; +import { dirname, join, relative, resolve } from "node:path"; +import { fileURLToPath } from "node:url"; +import { describe, expect, it } from "vitest"; + +const here = dirname(fileURLToPath(import.meta.url)); +const srcRoot = resolve(here, "../../../.."); + +/** + * The main header, split down the middle. + * + * The same boundary `theme-boundaries.test.ts` draws around the shell one level + * up, and it is the header that makes it worth drawing twice: the shell is four + * slots and no imports, while the header is the design system - a link, a + * button, two dropdowns and a theme toggle. One import that only resolves inside + * a Next.js app anywhere in that graph turns the whole `apps/web` shell into a + * build error nobody sees until they try it. That is not hypothetical: + * `HeaderContent` was Next-only for one back button, and `use-captcha` made + * every `AutoForm` Next-only for one navigation import. + * + * The shared half is the bar, the logo placement, the nav and the action area. + * The Next half is `getTranslations`, `next-intl`'s locale-aware `Link` and the + * async user slot - and it is the control that proves this scan can see them. + */ +const SHARED = { + header: join(here, "header-content.tsx"), + languageSwitcher: join( + srcRoot, + "components/switchers/langs/language-switcher-content.tsx", + ), + /** The theme toggle, reused unchanged rather than extracted - see below. */ + themeSwitcher: join( + srcRoot, + "components/switchers/themes/theme-switcher.tsx", + ), +}; + +const NEXT_WRAPPERS = { + header: join(here, "header.tsx"), + headerLink: join(here, "header-next.tsx"), + languageSwitcher: join( + srcRoot, + "components/switchers/langs/language-switcher.tsx", + ), +}; + +const resolveSpecifier = (specifier: string, from: string): null | string => { + let base: string; + + if (specifier.startsWith("@/")) base = join(srcRoot, specifier.slice(2)); + else if (specifier.startsWith(".")) base = resolve(dirname(from), specifier); + else return null; + + for (const suffix of [".ts", ".tsx", "/index.ts", "/index.tsx"]) { + const candidate = base + suffix; + if (existsSync(candidate) && statSync(candidate).isFile()) return candidate; + } + + return existsSync(base) && statSync(base).isFile() ? base : null; +}; + +/** Every specifier a file imports at runtime; `import type` is erased first. */ +const runtimeImports = (path: string): string[] => { + const source = readFileSync(path, "utf8").replace( + /(^|[\n;])\s*import\s+type\s[\s\S]*?from\s*["'][^"']+["']/g, + "$1", + ); + + return [ + ...source.matchAll( + /(?:^|[^\w$.])from\s*["']([^"']+)["']|import\s*\(\s*["']([^"']+)["']|(?:^|[\n;}])\s*import\s*["']([^"']+)["']/g, + ), + ] + .map(match => match[1] ?? match[2] ?? match[3]) + .filter((specifier): specifier is string => Boolean(specifier)); +}; + +/** Every external specifier reachable from an entry, with the chain that got there. */ +const externalGraph = (entry: string): Map<string, string[]> => { + const found = new Map<string, string[]>(); + const parents = new Map<string, string>(); + const seen = new Set<string>(); + + const chain = (file: string): string => { + const parts: string[] = []; + for (let at: string | undefined = file; at; at = parents.get(at)) { + parts.unshift(relative(srcRoot, at)); + } + + return parts.join(" -> "); + }; + + const walk = (file: string) => { + if (seen.has(file)) return; + seen.add(file); + + for (const specifier of runtimeImports(file)) { + const target = resolveSpecifier(specifier, file); + + if (target) { + if (!parents.has(target)) parents.set(target, file); + walk(target); + continue; + } + + found.set(specifier, [...(found.get(specifier) ?? []), chain(file)]); + } + }; + + walk(entry); + + return found; +}; + +const matches = (specifier: string, forbidden: string): boolean => + specifier === forbidden || specifier.startsWith(`${forbidden}/`); + +const offenders = (entry: string, forbidden: string[]): string[] => + [...externalGraph(entry)] + .filter(([specifier]) => forbidden.some(one => matches(specifier, one))) + .flatMap(([specifier, chains]) => chains.map(at => `${specifier} in ${at}`)) + .sort(); + +/** Anything that only resolves inside a Next.js app. */ +const NEXT_ONLY = ["next", "server-only"]; + +/** + * `next-intl`'s Next-only halves. The root entry is absent on purpose: it + * re-exports `use-intl`, which is framework-free - and the theme switcher reads + * its label through it, which is why it renders in `apps/web` unchanged. + */ +const NEXT_INTL_RUNTIME = [ + "next-intl/middleware", + "next-intl/navigation", + "next-intl/plugin", + "next-intl/server", +]; + +const sharedEntries = Object.entries(SHARED).map(([name, path]) => ({ + name, + path, +})); + +/** A file's code, with the prose stripped - which talks about the very imports + * these assertions look for. */ +const withoutComments = (path: string): string => + readFileSync(path, "utf8") + .replace(/\/\*[\s\S]*?\*\//g, "") + .replace(/\/\/.*$/gm, ""); + +describe("the import scan finds what it is looking for", () => { + // Every assertion below is a "found nothing" one, which a scanner that + // silently matches nothing also satisfies. The Next wrappers are the control: + // they provably import the things the shared half must not. + it("finds `next-intl/server` in the Next header", () => { + expect(offenders(NEXT_WRAPPERS.header, NEXT_INTL_RUNTIME)).not.toEqual([]); + }); + + it("walks past the entry file into its dependencies", () => { + // `lib/navigation` is two hops from the header, never one - by way of + // `header-next.tsx` for the links and `language-switcher.tsx` for the + // switch. A scan that only read the entry file would find neither. + const chains = offenders(NEXT_WRAPPERS.header, ["next-intl/navigation"]); + + expect(chains).not.toEqual([]); + expect(chains.every(one => one.includes(" -> "))).toBe(true); + }); +}); + +describe("the shared header is framework-neutral", () => { + it.each(sharedEntries)("$name reaches nothing from next/*", ({ path }) => { + expect(offenders(path, NEXT_ONLY)).toEqual([]); + }); + + it.each(sharedEntries)( + "$name reaches none of next-intl's Next-only entrypoints", + ({ path }) => { + expect(offenders(path, NEXT_INTL_RUNTIME)).toEqual([]); + }, + ); + + it.each(sharedEntries)( + "$name never reaches the locale-aware navigation module", + ({ path }) => { + const reached = [...externalGraph(path).keys()]; + + expect(reached.some(one => one.includes("navigation"))).toBe(false); + }, + ); + + it.each(sharedEntries)("$name never reaches a server action", ({ path }) => { + // A `"use server"` module is the other way Next.js gets in: importing one + // pulls the fetcher, `next/headers` and the whole API module graph behind + // it. The header's one mutation - sign-out - lives in the user slot, which + // is a prop. + const reached = [...externalGraph(path).keys()]; + + expect(reached.some(one => one.endsWith(".server"))).toBe(false); + expect(runtimeImports(path).some(one => one.includes(".server"))).toBe( + false, + ); + }); + + it("never reaches a router", () => { + // The mirror of the Next assertions: the shared header must not import + // TanStack Router either, or the Next.js app stops being able to render it. + const reached = [...externalGraph(SHARED.header).keys()]; + + expect(reached.some(one => one.startsWith("@tanstack/"))).toBe(false); + }); +}); + +describe("the shared header takes its framework parts as props", () => { + const code = withoutComments(SHARED.header); + + it("takes its links as a component rather than importing one", () => { + expect(code).toContain("LinkComponent"); + }); + + it.each(["logo", "navigation", "languageSwitcher", "user"])( + "asks for %s rather than resolving it", + slot => { + expect(code).toContain(slot); + }, + ); + + it("translates nothing itself", () => { + // The nav labels arrive as data. Next.js resolves them on the server, where + // they cost the client bundle nothing; `apps/web` resolves them from the + // message cache. A `useTranslations` here would force `core.search` into + // every Next.js page's client provider for two words. + expect(code).not.toContain("useTranslations"); + expect(code).not.toContain("getTranslations"); + }); + + it("renders the theme switcher itself", () => { + // Not a prop: it was already framework-neutral - the assertions above are + // over its real import graph - so injecting it would be a prop every caller + // has to pass and nobody gets to answer differently. + expect(code).toContain("<ThemeSwitcher />"); + }); +}); + +describe("the shared language switcher takes the navigation as a callback", () => { + const code = withoutComments(SHARED.languageSwitcher); + + it("asks for a select handler rather than moving the URL itself", () => { + expect(code).toContain("onSelect"); + expect(code).not.toContain("useRouter"); + expect(code).not.toContain("usePathname"); + }); + + it("is the only copy of the dropdown", () => { + // The Next wrapper is the navigation and nothing else - if it grows a + // `DropdownMenu` again, `apps/web` is rendering different markup. + expect(withoutComments(NEXT_WRAPPERS.languageSwitcher)).not.toContain( + "DropdownMenu", + ); + }); + + /** + * The one piece of Next.js structure the shared control has to make room for. + * + * `useRouter()` and `usePathname()` read the current URL, and Next 16 refuses + * to prerender a client component that reads URL data outside a `<Suspense>` - + * so the AdminCP's prerendered routes fail `next build` outright without this + * boundary. It was there before the control was shared, and moving the hooks + * up out of it while extracting the markup is exactly how it got lost once. + */ + it("keeps the URL-reading hooks inside a Suspense boundary", () => { + const next = withoutComments(NEXT_WRAPPERS.languageSwitcher); + const at = next.indexOf("export const LanguageSwitcher ="); + const exported = next.slice(at); + const inner = next.slice(0, at); + + // The exported component renders the boundary and reads no URL itself. + expect(exported).toContain("React.Suspense"); + expect(exported).not.toContain("usePathname"); + expect(exported).not.toContain("useRouter"); + + // Everything that does read one is in the component below it. + expect(inner).toContain("usePathname"); + expect(inner).toContain("useRouter"); + }); +}); + +describe("the Next wrappers keep the Next-only pieces", () => { + it.each( + Object.entries(NEXT_WRAPPERS).map(([name, path]) => ({ name, path })), + )("the $name wrapper is where Next.js enters", ({ path }) => { + expect(offenders(path, [...NEXT_ONLY, ...NEXT_INTL_RUNTIME])).not.toEqual( + [], + ); + }); + + it("is the only half that knows about next-intl navigation", () => { + expect( + offenders(NEXT_WRAPPERS.headerLink, ["next-intl/navigation"]), + ).not.toEqual([]); + expect(offenders(SHARED.header, ["next-intl/navigation"])).toEqual([]); + }); +}); diff --git a/packages/vitnode/src/views/layouts/theme/header/header-content.tsx b/packages/vitnode/src/views/layouts/theme/header/header-content.tsx new file mode 100644 index 000000000..d8b69fba8 --- /dev/null +++ b/packages/vitnode/src/views/layouts/theme/header/header-content.tsx @@ -0,0 +1,100 @@ +"use client"; + +import { ThemeSwitcher } from "@/components/switchers/themes/theme-switcher"; +import { buttonVariants } from "@/components/ui/button"; +import { cn } from "@/lib/utils"; + +import type { HeaderLinkComponent, HeaderNavItem } from "./header-nav"; + +import { HEADER_HREF } from "./header-nav"; + +/** + * The main header - the bar itself, the logo, the nav and the action area. + * + * Presentation only, and framework-free on purpose: it reaches nothing from + * `next/*`, nothing from `next-intl`'s Next-only entries, nothing from + * `@/lib/navigation` and nothing from TanStack Router. So a TanStack Start route + * renders exactly the header the Next.js pages render, down to the class names, + * instead of a second copy of this markup drifting alongside it. + * + * Not to be confused with `components/ui/header-content.tsx`, which is a *page* + * heading (`<h1>`, description, back button). This is the site header. + * + * ## What it takes, and why each one is a prop + * + * - `LinkComponent` - the only genuinely framework-specific piece. See + * {@link HeaderLinkComponent}. + * - `logo` - an element, because the mark is the application's, not core's. + * - `navigation` - data rather than translated in here, because the two + * frameworks resolve strings in different places: Next.js on the server, where + * they cost the client bundle nothing, and TanStack Start in the browser. + * Built by `headerNavItems` on both sides, so the links and their order are + * shared even though the lookup is not. + * - `languageSwitcher` - an element, because *switching* language is navigation + * and therefore framework-specific. The dropdown itself is shared + * (`components/switchers/langs/language-switcher-content.tsx`); only the two + * lines that move the URL differ. Omitted entirely when a deployment serves + * one language, which is the caller's question to answer. + * - `user` - an element. In Next.js it is an async Server Component inside its + * own `<Suspense>`; in TanStack Start it is whatever the session slot renders. + * Either way the header only needs somewhere to put it. + * + * `ThemeSwitcher` is *not* a prop: it reads the theme from `VitNodeProviders`, + * which both apps mount, and translates through `use-intl`'s context, which both + * apps provide. It was already framework-neutral - `apps/web` renders it + * unchanged - so injecting it would be a prop every caller has to pass and + * nobody gets to answer differently. + */ +export interface HeaderLayoutContentProps extends Omit< + React.ComponentProps<"header">, + "children" +> { + languageSwitcher?: React.ReactNode; + LinkComponent: HeaderLinkComponent; + logo: React.ReactNode; + navigation: HeaderNavItem[]; + user?: React.ReactNode; +} + +export const HeaderLayoutContent = ({ + LinkComponent, + className, + languageSwitcher, + logo, + navigation, + user, + ...props +}: HeaderLayoutContentProps) => ( + <header + className={cn("sticky top-0 z-20 w-full sm:top-2 sm:mb-2", className)} + {...props} + > + <div className="dark:bg-background/75 bg-card/75 container mx-auto flex h-14 items-center border-b px-4 py-2 backdrop-blur sm:rounded-lg sm:border sm:shadow-sm"> + <LinkComponent href={HEADER_HREF.home}>{logo}</LinkComponent> + + {/* + Hidden below `sm`, as it has always been: at that width the bar holds the + logo and the action area and nothing else fits. Restoring the links to + small screens needs a mobile menu, which is a design question rather than + a migration one. + */} + <nav className="ms-4 hidden items-center gap-1 sm:flex"> + {navigation.map(item => ( + <LinkComponent + className={buttonVariants({ size: "sm", variant: "ghost" })} + href={item.href} + key={item.href} + > + {item.label} + </LinkComponent> + ))} + </nav> + + <div className="ml-auto flex items-center gap-2"> + {languageSwitcher} + <ThemeSwitcher /> + {user} + </div> + </div> + </header> +); diff --git a/packages/vitnode/src/views/layouts/theme/header/header-nav.test.ts b/packages/vitnode/src/views/layouts/theme/header/header-nav.test.ts new file mode 100644 index 000000000..3a087e90a --- /dev/null +++ b/packages/vitnode/src/views/layouts/theme/header/header-nav.test.ts @@ -0,0 +1,73 @@ +import { describe, expect, it } from "vitest"; + +import { + HEADER_HREF, + HEADER_NAV_MESSAGE_KEYS, + headerNavItems, +} from "./header-nav"; + +/** + * The main nav, as the two frameworks build it. + * + * `headerNavItems` is the whole of what they share: a label from a translator + * each resolves in its own way, paired with an href neither is allowed to spell + * itself. What is pinned here is that pairing - the destinations, the order, and + * that the hrefs stay internal - because a difference in any of the three is a + * header that looks migrated and navigates somewhere else. + */ +describe("the main nav", () => { + const labels = { discover: "Discover", search: "Search" }; + + it("is Discover then Search", () => { + expect(headerNavItems(labels)).toEqual([ + { href: "/discover", label: "Discover" }, + { href: "/search", label: "Search" }, + ]); + }); + + it("carries the label it was given, untouched", () => { + // Both frameworks translate `core.search.nav.*`, so a nav rendered in + // Polish is Polish because of what was passed in and nothing else - there is + // no fallback string in here to mask a namespace nobody warmed. + expect(headerNavItems({ discover: "Odkrywaj", search: "Szukaj" })).toEqual([ + { href: "/discover", label: "Odkrywaj" }, + { href: "/search", label: "Szukaj" }, + ]); + }); + + it("points at internal paths with no locale prefix", () => { + // The prefix is the router's to write - `rewrite.output` in `apps/web`, the + // locale-aware `Link` in Next.js. A prefix here would be a second one. + for (const { href } of headerNavItems(labels)) { + expect(href.startsWith("/")).toBe(true); + expect(href).not.toMatch(/^\/(en|pl)\b/); + } + }); + + it("gives every link a distinct key", () => { + // `href` is the React key, so a duplicate is a silently dropped link. + const hrefs = headerNavItems(labels).map(item => item.href); + + expect(new Set(hrefs).size).toBe(hrefs.length); + }); +}); + +/** + * The logo, which is not in the nav list and is still the same destination in + * both frameworks. + */ +describe("the header's destinations", () => { + it("sends the logo home", () => { + expect(HEADER_HREF.home).toBe("/"); + }); + + it("reads its labels from the namespace that already owns them", () => { + // `core.search.nav.*`, where the Next.js header has always read them. + // Shared as literals so a typed translator still checks them at each call + // site - the two translator *types* are not interchangeable. + expect(HEADER_NAV_MESSAGE_KEYS).toEqual({ + discover: "nav.discover", + search: "nav.search", + }); + }); +}); diff --git a/packages/vitnode/src/views/layouts/theme/header/header-nav.ts b/packages/vitnode/src/views/layouts/theme/header/header-nav.ts new file mode 100644 index 000000000..dbcaf48c8 --- /dev/null +++ b/packages/vitnode/src/views/layouts/theme/header/header-nav.ts @@ -0,0 +1,92 @@ +/** + * The main header's links, as data. + * + * Two routes today - Discover and Search - and the reason they are a list rather + * than two hard-coded `<Link>`s is that the header is now rendered by two + * frameworks. Both have to agree on *where* the nav points and *what order* it + * is in; only the component that turns an href into a navigation differs. So the + * destinations live here, the labels are resolved by whoever has a translator, + * and {@link headerNavItems} puts the two together in one place. + * + * This is deliberately not a navigation framework and not a route registry. + * There is no active-state resolution, no nesting and no permissions here: the + * header renders the same two links it always has, and a plugin that wants a + * third one is a design question this stage does not answer. + */ + +/** + * The anchor a header link ends up rendering. + * + * Every prop of one, not just `href`: a caller may hand the logo link a class + * name, and the nav links get `buttonVariants(...)`. The same shape + * `AuthLinkProps` and `HeaderContentBackLinkProps` already use, for the same + * reason - a wrapper that accepted only `href` would silently drop the rest. + */ +export interface HeaderLinkProps extends Omit< + React.ComponentProps<"a">, + "href" +> { + href: string; +} + +/** + * The one thing the header cannot decide for itself. + * + * Turning `/discover` into a client-side navigation is the single question whose + * answer differs between the two frameworks: Next.js wants `next-intl`'s + * locale-aware `Link` (`@/lib/navigation`), TanStack Start wants the router's own + * - and during the migration it wants one that decides per href whether this app + * can render the destination at all (`MigrationLink`). Both are a component + * taking {@link HeaderLinkProps}, so the header takes one and stops caring - and + * importing neither is what lets a TanStack Start route render it. + */ +export type HeaderLinkComponent = (props: HeaderLinkProps) => React.ReactNode; + +/** Where the main header points. Internal paths, with no locale prefix in them. */ +export const HEADER_HREF = { + discover: "/discover", + home: "/", + search: "/search", +} as const; + +/** + * The keys the labels come from, under `core.search`. + * + * Named here so the two wrappers cannot spell them differently, and left as + * literals (`as const`) so a typed translator still checks them at the call + * site - which is why the *keys* are shared and the translator is not. Next.js + * resolves them on the server with `getTranslations`, TanStack Start in the + * browser with `useTranslations`; the two translator types are not + * interchangeable, and passing one through a shared signature would mean giving + * up key checking in both. + */ +export const HEADER_NAV_MESSAGE_KEYS = { + discover: "nav.discover", + search: "nav.search", +} as const; + +/** One link in the main nav. */ +export interface HeaderNavItem { + href: string; + label: string; +} + +/** The labels {@link headerNavItems} needs, already translated. */ +export interface HeaderNavLabels { + discover: string; + search: string; +} + +/** + * The main nav, in the order it renders. + * + * Pure, and the only place that pairs a destination with its label - so the two + * frameworks cannot drift into a different set of links or a different order. + */ +export const headerNavItems = ({ + discover, + search, +}: HeaderNavLabels): HeaderNavItem[] => [ + { href: HEADER_HREF.discover, label: discover }, + { href: HEADER_HREF.search, label: search }, +]; diff --git a/packages/vitnode/src/views/layouts/theme/header/header-next.tsx b/packages/vitnode/src/views/layouts/theme/header/header-next.tsx new file mode 100644 index 000000000..a250f527c --- /dev/null +++ b/packages/vitnode/src/views/layouts/theme/header/header-next.tsx @@ -0,0 +1,38 @@ +"use client"; + +import { Link } from "@/lib/navigation"; + +import type { HeaderLayoutContentProps } from "./header-content"; +import type { HeaderLinkProps } from "./header-nav"; + +import { HeaderLayoutContent } from "./header-content"; + +/** + * The header's links, the Next.js way: `next-intl`'s locale-aware `Link`. + * + * One module-scope component rather than one per link, so the logo and both nav + * items render the same component type and React reconciles the header instead + * of remounting it. + */ +export const NextHeaderLink = ({ + children, + href, + ...props +}: HeaderLinkProps) => ( + <Link href={href} {...props}> + {children} + </Link> +); + +/** + * {@link HeaderLayoutContent}, wired to Next.js. + * + * A client component whose only job is to choose the link, because a component + * type cannot cross the server/client boundary as a prop - `header.tsx` is an + * async Server Component, so the choice has to be made on this side. Everything + * it reads a request for (the logo, the user slot, the translated nav) arrives as + * props: elements and plain data, both of which do cross. + */ +export const NextHeaderContent = ( + props: Omit<HeaderLayoutContentProps, "LinkComponent">, +) => <HeaderLayoutContent {...props} LinkComponent={NextHeaderLink} />; diff --git a/packages/vitnode/src/views/layouts/theme/header/header.tsx b/packages/vitnode/src/views/layouts/theme/header/header.tsx index 78160163b..102de93a9 100644 --- a/packages/vitnode/src/views/layouts/theme/header/header.tsx +++ b/packages/vitnode/src/views/layouts/theme/header/header.tsx @@ -4,58 +4,56 @@ import React from "react"; import type { VitNodeConfig } from "@/vitnode.config"; import { LanguageSwitcher } from "@/components/switchers/langs/language-switcher"; -import { ThemeSwitcher } from "@/components/switchers/themes/theme-switcher"; -import { buttonVariants } from "@/components/ui/button"; -import { Skeleton } from "@/components/ui/skeleton"; -import { Link } from "@/lib/navigation"; -import { cn } from "@/lib/utils"; +import { HEADER_NAV_MESSAGE_KEYS, headerNavItems } from "./header-nav"; +import { NextHeaderContent } from "./header-next"; import { UserHeader } from "./user/user"; +import { UserHeaderSkeleton } from "./user/user-header-content"; +/** + * The main header on Next.js. + * + * The markup moved to {@link HeaderLayoutContent}, which `apps/web` renders too. + * What is left here is the half only a Next.js request can produce: + * + * - **The nav labels**, through `getTranslations`. Resolved on the server, so + * `core.search` never has to be shipped to the client provider for the sake of + * two words - which is why the nav is passed as data rather than translated + * inside the shared header. + * - **The user slot**, an async Server Component streaming inside its own + * `<Suspense>`, exactly as before. The fallback is the shared header's own + * skeleton, so the space it reserves is the size of what replaces it. + * - **The language switcher**, or nothing at all when the deployment serves one + * language. + */ export const HeaderLayout = async ({ logo, - className, vitNodeConfig, ...props -}: React.ComponentProps<"header"> & { +}: Omit<React.ComponentProps<"header">, "children"> & { logo: React.ReactNode; vitNodeConfig: VitNodeConfig; }) => { const t = await getTranslations("core.search"); return ( - <header - className={cn("sticky top-0 z-20 w-full sm:top-2 sm:mb-2", className)} + <NextHeaderContent {...props} - > - <div className="dark:bg-background/75 bg-card/75 container mx-auto flex h-14 items-center border-b px-4 py-2 backdrop-blur sm:rounded-lg sm:border sm:shadow-sm"> - <Link href="/">{logo}</Link> - - <nav className="ms-4 hidden items-center gap-1 sm:flex"> - <Link - className={buttonVariants({ variant: "ghost", size: "sm" })} - href="/discover" - > - {t("nav.discover")} - </Link> - <Link - className={buttonVariants({ variant: "ghost", size: "sm" })} - href="/search" - > - {t("nav.search")} - </Link> - </nav> - - <div className="ml-auto flex items-center gap-2"> - {vitNodeConfig.i18n.locales.length > 1 && ( - <LanguageSwitcher locales={vitNodeConfig.i18n.locales} /> - )} - <ThemeSwitcher /> - <React.Suspense fallback={<Skeleton className="h-9 w-32" />}> - <UserHeader /> - </React.Suspense> - </div> - </div> - </header> + languageSwitcher={ + vitNodeConfig.i18n.locales.length > 1 ? ( + <LanguageSwitcher locales={vitNodeConfig.i18n.locales} /> + ) : null + } + logo={logo} + navigation={headerNavItems({ + discover: t(HEADER_NAV_MESSAGE_KEYS.discover), + search: t(HEADER_NAV_MESSAGE_KEYS.search), + })} + user={ + <React.Suspense fallback={<UserHeaderSkeleton />}> + <UserHeader /> + </React.Suspense> + } + /> ); }; diff --git a/packages/vitnode/src/views/layouts/theme/header/user/auth/auth.tsx b/packages/vitnode/src/views/layouts/theme/header/user/auth/auth.tsx deleted file mode 100644 index 8e5225693..000000000 --- a/packages/vitnode/src/views/layouts/theme/header/user/auth/auth.tsx +++ /dev/null @@ -1,29 +0,0 @@ -import { Avatar } from "@/components/avatar"; -import { Button } from "@/components/ui/button"; -import { - DropdownMenu, - DropdownMenuContent, - DropdownMenuTrigger, -} from "@/components/ui/dropdown-menu"; -import { getSessionApi } from "@/lib/api/get-session-api"; - -import { ClientAuthUserHeader } from "./client"; - -export const AuthUserHeader = async () => { - const { user } = await getSessionApi(); - if (!user) return null; - - return ( - <DropdownMenu> - <DropdownMenuTrigger - render={<Button aria-label={user.name} size="icon" variant="ghost" />} - > - <Avatar size={24} user={user} /> - </DropdownMenuTrigger> - - <DropdownMenuContent align="end" className="w-64 p-2"> - <ClientAuthUserHeader user={user} /> - </DropdownMenuContent> - </DropdownMenu> - ); -}; diff --git a/packages/vitnode/src/views/layouts/theme/header/user/auth/client.tsx b/packages/vitnode/src/views/layouts/theme/header/user/auth/client.tsx deleted file mode 100644 index d11f2ac33..000000000 --- a/packages/vitnode/src/views/layouts/theme/header/user/auth/client.tsx +++ /dev/null @@ -1,85 +0,0 @@ -"use client"; - -import { - FileIcon, - KeyRoundIcon, - LogOutIcon, - Settings, - ShieldIcon, - UserIcon, -} from "lucide-react"; -import { useTranslations } from "next-intl"; - -import type { SessionApi } from "@/lib/api/get-session-api"; - -import { - DropdownMenuGroup, - DropdownMenuItem, - DropdownMenuSeparator, -} from "@/components/ui/dropdown-menu"; -import { Link } from "@/lib/navigation"; - -import { logOutMutationApi } from "./log-out-mutation-api.server"; - -export const ClientAuthUserHeader = ({ - user, -}: { - user: NonNullable<SessionApi["user"]>; -}) => { - const t = useTranslations("core.global.user_bar"); - - return ( - <> - <DropdownMenuGroup> - <DropdownMenuItem render={<Link href={`/users/${user.nameCode}`} />}> - <UserIcon /> - <span>{t("my_profile")}</span> - </DropdownMenuItem> - - <DropdownMenuItem render={<Link href="/files" />}> - <FileIcon /> - <span>{t("files")}</span> - </DropdownMenuItem> - - <DropdownMenuItem render={<Link href="/settings" />}> - <Settings /> - <span>{t("settings")}</span> - </DropdownMenuItem> - </DropdownMenuGroup> - - <DropdownMenuSeparator /> - - {(user.isAdmin || user.isModerator) && ( - <> - <DropdownMenuGroup> - {user.isModerator && ( - <DropdownMenuItem render={<Link href="/mod_cp" />}> - <ShieldIcon /> - <span>{t("mod_cp")}</span> - </DropdownMenuItem> - )} - {user.isAdmin && ( - <DropdownMenuItem render={<Link href="/admin" target="_blank" />}> - <KeyRoundIcon /> - <span>{t("admin_cp")}</span> - </DropdownMenuItem> - )} - </DropdownMenuGroup> - - <DropdownMenuSeparator /> - </> - )} - - <DropdownMenuGroup> - <DropdownMenuItem - onClick={async () => { - await logOutMutationApi({}); - }} - > - <LogOutIcon /> - <span>{t("log_out")}</span> - </DropdownMenuItem> - </DropdownMenuGroup> - </> - ); -}; diff --git a/packages/vitnode/src/views/layouts/theme/header/user/next-user-header.tsx b/packages/vitnode/src/views/layouts/theme/header/user/next-user-header.tsx new file mode 100644 index 000000000..e7104fc99 --- /dev/null +++ b/packages/vitnode/src/views/layouts/theme/header/user/next-user-header.tsx @@ -0,0 +1,58 @@ +"use client"; + +import { Link } from "@/lib/navigation"; + +import type { UserHeaderLinkProps, UserHeaderUser } from "./user-header-model"; + +import { logOutMutationApi } from "./auth/log-out-mutation-api.server"; +import { UserHeaderContent } from "./user-header-content"; + +/** + * {@link UserHeaderContent}, wired to Next.js. + * + * The only place Next.js enters the user header: `next-intl`'s locale-aware + * `Link`, and the `"use server"` sign-out. Everything visible is the shared + * component's. + * + * A client component, and it has to be one - a component type such as + * `LinkComponent` cannot cross the server/client boundary as a prop, so the + * choice of link is made here rather than in the Server Component above. The + * session does cross it, as plain data. + */ + +/** + * The user header's link, the Next.js way. + * + * Module scope rather than inline, so the component type is stable across + * renders and the menu items are not remounted on every one of them. Every prop + * is forwarded, `ref` included: Base UI's `render` clones this element with the + * class name and ref the menu item needs. + */ +const NextUserHeaderLink = ({ + children, + href, + ...props +}: UserHeaderLinkProps) => ( + <Link href={href} {...props}> + {children} + </Link> +); + +/** + * Signing out of the main site. + * + * `logOutMutationApi` is the same server action the AdminCP sidebar calls with + * `isAdmin: true`; here the site session is the one being ended, so the + * revalidation and the redirect it performs are the main layout's. + */ +const onSignOut = async () => { + await logOutMutationApi({}); +}; + +export const NextUserHeader = ({ user }: { user: null | UserHeaderUser }) => ( + <UserHeaderContent + LinkComponent={NextUserHeaderLink} + onSignOut={onSignOut} + state={user ? { status: "authenticated", user } : { status: "anonymous" }} + /> +); diff --git a/packages/vitnode/src/views/layouts/theme/header/user/user-header-boundaries.test.ts b/packages/vitnode/src/views/layouts/theme/header/user/user-header-boundaries.test.ts new file mode 100644 index 000000000..4b48356c9 --- /dev/null +++ b/packages/vitnode/src/views/layouts/theme/header/user/user-header-boundaries.test.ts @@ -0,0 +1,206 @@ +// @vitest-environment node +import { existsSync, readFileSync, statSync } from "node:fs"; +import { dirname, join, relative, resolve } from "node:path"; +import { fileURLToPath } from "node:url"; +import { describe, expect, it } from "vitest"; + +const here = dirname(fileURLToPath(import.meta.url)); +const srcRoot = resolve(here, "../../../../.."); + +/** + * The user header, split down the middle. + * + * The same boundary `theme-boundaries.test.ts` and `auth-boundaries.test.ts` + * draw, for the same reason and with the same machinery: `UserHeaderContent` is + * rendered by a TanStack Start route as well as by Next.js, and one import that + * only resolves inside a Next.js app turns that route into a failure nobody sees + * until they try it. This is the header slot most likely to acquire one - it is + * the part with links, a session and a mutation in it. + * + * `next-user-header.tsx` is the control: it provably reaches the locale-aware + * `Link` and the sign-out server action, which is exactly what the shared half + * must not. + */ +const SHARED = { + content: join(here, "user-header-content.tsx"), + model: join(here, "user-header-model.ts"), +}; + +const NEXT_WRAPPER = join(here, "next-user-header.tsx"); + +const resolveSpecifier = (specifier: string, from: string): null | string => { + let base: string; + + if (specifier.startsWith("@/")) base = join(srcRoot, specifier.slice(2)); + else if (specifier.startsWith(".")) base = resolve(dirname(from), specifier); + else return null; + + for (const suffix of [".ts", ".tsx", "/index.ts", "/index.tsx"]) { + const candidate = base + suffix; + if (existsSync(candidate) && statSync(candidate).isFile()) return candidate; + } + + return existsSync(base) && statSync(base).isFile() ? base : null; +}; + +/** + * Every specifier a file imports **at runtime**. + * + * `import type` statements are stripped first: the wrapper imports the *type* of + * the user it renders, which TypeScript erases and which never reaches a bundle. + */ +const runtimeImports = (path: string): string[] => { + const source = readFileSync(path, "utf8").replace( + /(^|[\n;])\s*import\s+type\s[\s\S]*?from\s*["'][^"']+["']/g, + "$1", + ); + + return [ + ...source.matchAll( + /(?:^|[^\w$.])from\s*["']([^"']+)["']|import\s*\(\s*["']([^"']+)["']|(?:^|[\n;}])\s*import\s*["']([^"']+)["']/g, + ), + ] + .map(match => match[1] ?? match[2] ?? match[3]) + .filter((specifier): specifier is string => Boolean(specifier)); +}; + +/** Every external specifier reachable from an entry, with the chain that got there. */ +const externalGraph = (entry: string): Map<string, string[]> => { + const found = new Map<string, string[]>(); + const parents = new Map<string, string>(); + const seen = new Set<string>(); + + const chain = (file: string): string => { + const parts: string[] = []; + for (let at: string | undefined = file; at; at = parents.get(at)) { + parts.unshift(relative(srcRoot, at)); + } + + return parts.join(" -> "); + }; + + const walk = (file: string) => { + if (seen.has(file)) return; + seen.add(file); + + for (const specifier of runtimeImports(file)) { + const target = resolveSpecifier(specifier, file); + + if (target) { + if (!parents.has(target)) parents.set(target, file); + walk(target); + continue; + } + + found.set(specifier, [...(found.get(specifier) ?? []), chain(file)]); + } + }; + + walk(entry); + + return found; +}; + +const matches = (specifier: string, forbidden: string): boolean => + specifier === forbidden || specifier.startsWith(`${forbidden}/`); + +const offenders = (entry: string, forbidden: string[]): string[] => + [...externalGraph(entry)] + .filter(([specifier]) => forbidden.some(one => matches(specifier, one))) + .flatMap(([specifier, chains]) => chains.map(at => `${specifier} in ${at}`)) + .sort(); + +/** Anything that only resolves inside a Next.js app. */ +const NEXT_ONLY = ["next", "server-only"]; + +/** `next-intl`'s Next-only halves - `lib/navigation` is built on two of them. */ +const NEXT_INTL_RUNTIME = [ + "next-intl/middleware", + "next-intl/navigation", + "next-intl/plugin", + "next-intl/server", +]; + +const sharedEntries = Object.entries(SHARED).map(([name, path]) => ({ + name, + path, +})); + +describe("the import scan finds what it is looking for", () => { + // Every assertion below is a "found nothing" one, which a scanner that + // silently matches nothing also satisfies. The wrapper is the control. + it("finds the Next-only imports in the Next wrapper", () => { + expect( + offenders(NEXT_WRAPPER, [...NEXT_ONLY, ...NEXT_INTL_RUNTIME]), + ).not.toEqual([]); + }); + + it("walks past the entry file into its dependencies", () => { + // The server action is one hop from the wrapper; `next/cache` is two. + expect([...externalGraph(NEXT_WRAPPER).keys()]).toContain("next/cache"); + }); +}); + +describe("the shared user header is framework-neutral", () => { + it.each(sharedEntries)("$name reaches nothing from next/*", ({ path }) => { + expect(offenders(path, NEXT_ONLY)).toEqual([]); + }); + + it.each(sharedEntries)( + "$name reaches none of next-intl's Next-only entrypoints", + ({ path }) => { + expect(offenders(path, NEXT_INTL_RUNTIME)).toEqual([]); + }, + ); + + it.each(sharedEntries)( + "$name never reaches the locale-aware navigation module", + ({ path }) => { + const reached = [...externalGraph(path).keys()]; + + expect(reached.some(one => one.includes("navigation"))).toBe(false); + }, + ); + + it.each(sharedEntries)("$name never reaches a server action", ({ path }) => { + const reached = [...externalGraph(path).keys()]; + + expect(reached.some(one => one.endsWith(".server"))).toBe(false); + expect(runtimeImports(path).some(one => one.includes(".server"))).toBe( + false, + ); + }); + + it("never reaches the session read either", () => { + // The whole point of taking a state instead of fetching one: a shared + // component that imported `getSessionApi` would pull `next/headers` in + // behind it, and would be a second source of truth in the app that already + // has a canonical session query. + const reached = [...externalGraph(SHARED.content).keys()]; + + expect(reached.some(one => one.includes("get-session-api"))).toBe(false); + }); +}); + +describe("the shared user header takes its framework parts as props", () => { + const withoutComments = (path: string): string => + readFileSync(path, "utf8") + .replace(/\/\*[\s\S]*?\*\//g, "") + .replace(/\/\/.*$/gm, ""); + + const code = withoutComments(SHARED.content); + + it("takes its links as a component", () => { + expect(code).toContain("LinkComponent"); + }); + + it("asks for a sign-out callback rather than calling a mutation", () => { + expect(code).toContain("onSignOut"); + expect(code).not.toContain("logOutMutationApi"); + }); + + it("renders a state rather than reading a session", () => { + expect(code).toContain("state: UserHeaderState;"); + expect(code).not.toContain("useQuery"); + }); +}); diff --git a/packages/vitnode/src/views/layouts/theme/header/user/user-header-content.tsx b/packages/vitnode/src/views/layouts/theme/header/user/user-header-content.tsx new file mode 100644 index 000000000..82d208f3b --- /dev/null +++ b/packages/vitnode/src/views/layouts/theme/header/user/user-header-content.tsx @@ -0,0 +1,177 @@ +"use client"; + +import { LogOutIcon } from "lucide-react"; +import React from "react"; +import { useTranslations } from "use-intl"; + +import { Avatar } from "@/components/avatar"; +import { Button, buttonVariants } from "@/components/ui/button"; +import { + DropdownMenu, + DropdownMenuContent, + DropdownMenuGroup, + DropdownMenuItem, + DropdownMenuSeparator, + DropdownMenuTrigger, +} from "@/components/ui/dropdown-menu"; +import { Skeleton } from "@/components/ui/skeleton"; + +import type { + UserHeaderLinkComponent, + UserHeaderState, + UserHeaderUser, +} from "./user-header-model"; + +import { USER_HEADER_HREF, userHeaderMenu } from "./user-header-model"; + +/** + * The user area of the main header, rendered by both applications. + * + * Presentation only, and framework-free on purpose: it reaches nothing from + * `next/*`, nothing from `next-intl`'s Next-only entries and no server action, + * so a TanStack Start route renders exactly what the Next.js header renders. The + * three things it cannot decide for itself - the session, how a path becomes a + * navigation, and what ends a session - arrive as props. + * + * UserHeaderContent + * state === "loading" -> the placeholder, at the size of the real thing + * state === "anonymous" -> log in, register + * state === "authenticated" -> avatar -> account links, staff link, sign out + * + * It does not fetch the session. That is the whole reason it is reusable: the two + * applications get it from places that have nothing in common - a Server + * Component awaiting `getSessionApi()`, and the one canonical session query a + * router's guards already read - and a component that asked for it itself would + * be a second source of truth in the app that already has one. + * + * The same boundary `SearchFeedContent`, `HeaderContent` and the auth screens + * draw, for the same reason. + */ + +/** + * What stands in for the user area while the session is unknown. + * + * `h-9 w-32` is the size of the two guest buttons, which is the wider of the two + * outcomes - so the header settles into its final width rather than growing when + * the session lands. Exported because the Next.js header renders it as a + * `<Suspense>` fallback *above* this component, before any state exists. + */ +export const UserHeaderSkeleton = () => <Skeleton className="h-9 w-32" />; + +/** + * Ending the session, as the header asks for it. + * + * Nothing is returned, because what happens next is entirely the caller's + * business and the two answers share nothing: a Next.js server action + * revalidates the layout and redirects, while TanStack Start replaces the cached + * session and invalidates the router so the guards notice. The header's only job + * is to say when. + */ +export type UserHeaderSignOut = () => Promise<void> | void; + +const AnonymousUserHeader = ({ + LinkComponent, +}: { + LinkComponent: UserHeaderLinkComponent; +}) => { + const t = useTranslations("core.global"); + + return ( + <> + <LinkComponent + className={buttonVariants({ variant: "ghost" })} + href={USER_HEADER_HREF.signIn} + > + {t("login")} + </LinkComponent> + + <LinkComponent + className={buttonVariants()} + href={USER_HEADER_HREF.signUp} + > + {t("register")} + </LinkComponent> + </> + ); +}; + +const AuthenticatedUserHeader = ({ + LinkComponent, + onSignOut, + user, +}: { + LinkComponent: UserHeaderLinkComponent; + onSignOut: UserHeaderSignOut; + user: UserHeaderUser; +}) => { + const t = useTranslations("core.global.user_bar"); + + return ( + <DropdownMenu> + <DropdownMenuTrigger + render={<Button aria-label={user.name} size="icon" variant="ghost" />} + > + <Avatar size={24} user={user} /> + </DropdownMenuTrigger> + + <DropdownMenuContent align="end" className="w-64 p-2"> + {userHeaderMenu(user).map(group => ( + // Every group is followed by a separator, and the sign-out group + // below is what the last one separates from. `userHeaderMenu` never + // returns an empty group, so this cannot draw a stray rule. + <React.Fragment key={group[0].key}> + <DropdownMenuGroup> + {group.map(({ href, Icon, key, newTab }) => ( + <DropdownMenuItem + key={key} + render={ + <LinkComponent + href={href} + target={newTab ? "_blank" : undefined} + /> + } + > + <Icon /> + <span>{t(key)}</span> + </DropdownMenuItem> + ))} + </DropdownMenuGroup> + + <DropdownMenuSeparator /> + </React.Fragment> + ))} + + <DropdownMenuGroup> + <DropdownMenuItem onClick={onSignOut}> + <LogOutIcon /> + <span>{t("log_out")}</span> + </DropdownMenuItem> + </DropdownMenuGroup> + </DropdownMenuContent> + </DropdownMenu> + ); +}; + +export const UserHeaderContent = ({ + LinkComponent, + onSignOut, + state, +}: { + LinkComponent: UserHeaderLinkComponent; + onSignOut: UserHeaderSignOut; + state: UserHeaderState; +}) => { + if (state.status === "loading") return <UserHeaderSkeleton />; + + if (state.status === "anonymous") { + return <AnonymousUserHeader LinkComponent={LinkComponent} />; + } + + return ( + <AuthenticatedUserHeader + LinkComponent={LinkComponent} + onSignOut={onSignOut} + user={state.user} + /> + ); +}; diff --git a/packages/vitnode/src/views/layouts/theme/header/user/user-header-model.test.ts b/packages/vitnode/src/views/layouts/theme/header/user/user-header-model.test.ts new file mode 100644 index 000000000..69a464895 --- /dev/null +++ b/packages/vitnode/src/views/layouts/theme/header/user/user-header-model.test.ts @@ -0,0 +1,153 @@ +import { describe, expect, it } from "vitest"; + +import type { UserHeaderUser } from "./user-header-model"; + +import { + USER_HEADER_HREF, + userHeaderMenu, + userHeaderState, + userProfileHref, +} from "./user-header-model"; + +const user = (overrides: Partial<UserHeaderUser> = {}): UserHeaderUser => ({ + avatarColor: "ff0000", + isAdmin: false, + name: "Ada", + nameCode: "ada", + ...overrides, +}); + +/** Every item across every group, in the order they are drawn. */ +const keysOf = (groups: ReturnType<typeof userHeaderMenu>): string[] => + groups.flat().map(item => item.key); + +describe("the menu a signed-in visitor gets", () => { + it("is the three account links, in order", () => { + expect(keysOf(userHeaderMenu(user()))).toEqual([ + "my_profile", + "files", + "settings", + ]); + }); + + it("points each one at the path that owns it", () => { + const items = userHeaderMenu(user({ nameCode: "ada" })).flat(); + const href = (key: string) => items.find(item => item.key === key)?.href; + + expect(href("my_profile")).toBe("/users/ada"); + expect(href("files")).toBe(USER_HEADER_HREF.files); + expect(href("settings")).toBe(USER_HEADER_HREF.settings); + }); + + it("never returns an empty group, so a separator always has items above it", () => { + for (const isAdmin of [false, true]) { + for (const group of userHeaderMenu(user({ isAdmin }))) { + expect(group.length).toBeGreaterThan(0); + } + } + }); + + it("gives every item a distinct key", () => { + const keys = keysOf(userHeaderMenu(user({ isAdmin: true }))); + + expect(new Set(keys).size).toBe(keys.length); + }); + + // The moderator item pointed at `/mod_cp`, which no application serves, behind + // a flag the session route hardcodes to `false`. It is not carried over. + it("has no moderator item", () => { + expect(keysOf(userHeaderMenu(user({ isAdmin: true })))).not.toContain( + "mod_cp", + ); + }); +}); + +describe("the AdminCP item", () => { + it("is absent for a visitor who is not an admin", () => { + expect(keysOf(userHeaderMenu(user({ isAdmin: false })))).not.toContain( + "admin_cp", + ); + expect(userHeaderMenu(user({ isAdmin: false }))).toHaveLength(1); + }); + + it("is present for an admin, in its own group after the account links", () => { + const groups = userHeaderMenu(user({ isAdmin: true })); + + expect(groups).toHaveLength(2); + expect(keysOf([groups[1]])).toEqual(["admin_cp"]); + }); + + it("points at the AdminCP and opens in a new tab", () => { + const adminCp = userHeaderMenu(user({ isAdmin: true })) + .flat() + .find(item => item.key === "admin_cp"); + + expect(adminCp?.href).toBe(USER_HEADER_HREF.adminCp); + expect(adminCp?.newTab).toBe(true); + }); + + it("does not change the account links it is added to", () => { + expect(keysOf([userHeaderMenu(user({ isAdmin: true }))[0]])).toEqual( + keysOf(userHeaderMenu(user({ isAdmin: false }))), + ); + }); +}); + +describe("a profile href", () => { + it("escapes the name code rather than interpolating it raw", () => { + expect(userProfileHref("a b/c")).toBe("/users/a%20b%2Fc"); + }); + + it("leaves an ordinary name code alone", () => { + expect(userProfileHref("ada-lovelace_1")).toBe("/users/ada-lovelace_1"); + }); +}); + +describe("the state the header renders", () => { + it("is authenticated when the session names a user", () => { + const session = { user: user() }; + + expect(userHeaderState({ session })).toEqual({ + status: "authenticated", + user: session.user, + }); + }); + + it("is anonymous when the session answered with nobody", () => { + expect(userHeaderState({ session: { user: null } })).toEqual({ + status: "anonymous", + }); + }); + + it("is loading while nothing has been read yet", () => { + expect(userHeaderState({})).toEqual({ status: "loading" }); + expect(userHeaderState({ isError: false, session: undefined })).toEqual({ + status: "loading", + }); + }); + + // The Next.js header has always rendered the guest controls here, because + // `getSessionApi()` answers `{ user: null }` for any non-200. Note this is a + // *rendering* decision and not the one a route guard makes with the same + // failure - `ensureAuthState` rejects rather than signing anybody out. + it("is anonymous when the read failed with nothing cached", () => { + expect(userHeaderState({ isError: true })).toEqual({ + status: "anonymous", + }); + }); + + it("keeps a signed-in visitor through a failed refetch", () => { + const session = { user: user() }; + + expect(userHeaderState({ isError: true, session })).toEqual({ + status: "authenticated", + user: session.user, + }); + }); + + it("shows the guest controls, not a stuck placeholder, on a failed refetch of an anonymous session", () => { + expect(userHeaderState({ isError: true, session: { user: null } })).toEqual( + { status: "anonymous" }, + ); + }); +}); diff --git a/packages/vitnode/src/views/layouts/theme/header/user/user-header-model.ts b/packages/vitnode/src/views/layouts/theme/header/user/user-header-model.ts new file mode 100644 index 000000000..52b2f0d57 --- /dev/null +++ b/packages/vitnode/src/views/layouts/theme/header/user/user-header-model.ts @@ -0,0 +1,226 @@ +import type { LucideIcon } from "lucide-react"; + +import { FileIcon, KeyRoundIcon, SettingsIcon, UserIcon } from "lucide-react"; + +/** + * The user area of the main header, as data. + * + * No JSX, no framework and no I/O, so the three questions the header actually + * answers - who is asking, which items they get, and where each one leads - can + * be stated and tested without a router, a session or a DOM. The component in + * `user-header-content.tsx` renders exactly what this returns and decides + * nothing itself. + * + * That split is what makes the same header work in both applications. The + * Next.js app resolves the session in a Server Component; the TanStack Start app + * reads it from the canonical session query (`#/lib/auth/query` in `apps/web`). + * Both end up handing a {@link UserHeaderState} to one component. + * + * ## What it is not + * + * Not an authorization rule. `isAdmin` decides whether to *draw a link*, and + * nothing more: the AdminCP runs on its own session with its own sign-in, and + * every private read is authorized by Hono from the session cookie. A visitor + * who edits a cached session gets an extra menu item and an API that still + * refuses them. + */ + +/** + * The anchor a user-header link ends up rendering. + * + * Every prop of one, not just `href`: the menu items put a link inside a Base UI + * `render`, which clones the element with the children, the class name and the + * ref it needs to stay a menu item. A wrapper accepting only `href` would drop + * all three, so the type says so. + */ +export interface UserHeaderLinkProps extends Omit< + React.ComponentProps<"a">, + "href" +> { + href: string; +} + +/** + * The one thing this header cannot decide for itself. + * + * Turning `/settings` into a navigation is the single question whose answer + * differs between the two frameworks: Next.js wants `next-intl`'s locale-aware + * `Link`, and TanStack Start wants one that asks the route tree whether *this* + * application can render the destination at all - because half of VitNode is + * still served by the other one. Both are a component taking + * {@link UserHeaderLinkProps}, so the header takes one and stops caring. + * + * Required rather than defaulting to `<a>`: a missing wrapper would degrade + * silently into a full document reload on every menu item. + */ +export type UserHeaderLinkComponent = ( + props: UserHeaderLinkProps, +) => React.ReactNode; + +/** + * The visitor, as the header needs them - four fields and no more. + * + * A *requirement* rather than a copy of the session response: both applications' + * `SessionApi["user"]` satisfy it structurally, so neither has to reshape + * anything and a field renamed in `api/modules/users/routes/session.route.ts` + * fails at the two call sites rather than being silently rendered as + * `undefined`. The same shape `Avatar` already asks for, plus the one flag the + * menu branches on. + */ +export interface UserHeaderUser { + avatarColor: string; + isAdmin: boolean; + name: string; + nameCode: string; +} + +/** + * A session as the state the header renders. + * + * Three states rather than a nullable user, because "we do not know yet" is a + * real one and the header is on every page: the Next.js app answers it with a + * `<Suspense>` fallback while the Server Component awaits the session, and the + * TanStack Start app with a query that has not resolved. Both need a placeholder + * of the right size, and a `user: null` that meant both "signed out" and "still + * loading" would render the login buttons for a moment to somebody who is signed + * in. + */ +export type UserHeaderState = + | { status: "anonymous" } + | { status: "authenticated"; user: UserHeaderUser } + | { status: "loading" }; + +/** + * Where the header links to. + * + * Ordinary data, not a route table - nothing here knows or cares which + * application currently serves a path. During the migration `/files` and + * `/login` are rendered by TanStack Start and `/settings`, `/register`, + * `/admin` and the profile page by Next.js, and the *link component* is what + * decides that, per href, by asking the route tree. So a route that moves needs + * no edit here. + */ +export const USER_HEADER_HREF = { + adminCp: "/admin", + files: "/files", + settings: "/settings", + signIn: "/login", + signUp: "/register", +} as const; + +/** + * A visitor's own profile page. + * + * `encodeURIComponent` because a name code reaches this from the API and a path + * segment is not a place to interpolate an unescaped string. Today's codes are + * slug-safe and it is a no-op for all of them. + */ +export const userProfileHref = (nameCode: string): string => + `/users/${encodeURIComponent(nameCode)}`; + +/** + * One item in the user menu. + * + * `key` is both the React key and the `core.global.user_bar` message key, which + * is deliberate: an item cannot exist without a label, and a second field + * holding the same string is a second thing to keep in step. + */ +export interface UserHeaderMenuItem { + href: string; + Icon: LucideIcon; + key: UserHeaderMenuItemKey; + /** Opens in a new tab, as the AdminCP link always has. */ + newTab?: boolean; +} + +export type UserHeaderMenuItemKey = + "admin_cp" | "files" | "my_profile" | "settings"; + +/** + * The signed-in visitor's menu, grouped exactly as it is drawn. + * + * Groups rather than a flat list because the separators are part of the design + * and are not per-item: the account links are one block, the staff link its own, + * and sign-out - which is not a link and so is not here - a third that the + * component always appends. An empty group is never returned, so the component + * can put a separator after every one of them without ever drawing a stray rule. + * + * ## Only `isAdmin` branches + * + * `isModerator` exists in the session response and is hardcoded `false` (see + * `session.route.ts`: `// TODO: implement moderator role`), and the `/mod_cp` + * page it used to link to does not exist in either application. So the item was + * unreachable copy pointing at a 404, and it is deliberately not carried over - + * there is no moderator role to model yet, and a menu that renders one would + * start linking to a missing page the day the API answers `true`. + */ +export const userHeaderMenu = ( + user: UserHeaderUser, +): UserHeaderMenuItem[][] => { + const account: UserHeaderMenuItem[] = [ + { + href: userProfileHref(user.nameCode), + Icon: UserIcon, + key: "my_profile", + }, + { href: USER_HEADER_HREF.files, Icon: FileIcon, key: "files" }, + { href: USER_HEADER_HREF.settings, Icon: SettingsIcon, key: "settings" }, + ]; + + if (!user.isAdmin) return [account]; + + return [ + account, + [ + { + href: USER_HEADER_HREF.adminCp, + Icon: KeyRoundIcon, + key: "admin_cp", + newTab: true, + }, + ], + ]; +}; + +/** + * A session read as the state the header renders. + * + * Total and pure, and the one place the three states are decided: + * + * a session -> its `user` decides: signed in, or anonymous + * no session yet -> loading, unless the read has already failed + * a failed read -> anonymous + * + * ## A failed read shows the guest controls + * + * Which is the existing behaviour rather than a new decision: the Next.js + * `getSessionApi()` answers `{ user: null }` for any non-200, so an outage has + * always rendered the login buttons. The header has to draw *something* and a + * permanent skeleton is not it. + * + * Note what it is not: this is not a route guard, and it must not be used as + * one. `#/lib/auth/query`'s `ensureAuthState` deliberately *rejects* on a failed + * read so that a guard never signs anybody out because of a 500 - see the long + * note in `#/lib/auth/shared`. Drawing a login button for a visitor who is + * actually signed in costs them one click; sending them to the login page costs + * them the page they were on. + * + * A session already in hand wins over an error, so a signed-in visitor keeps + * their header through a failed *refetch* rather than flickering to anonymous + * and back. + */ +export const userHeaderState = ({ + isError = false, + session, +}: { + isError?: boolean; + session?: undefined | { user: null | UserHeaderUser }; +}): UserHeaderState => { + if (session) { + return session.user + ? { status: "authenticated", user: session.user } + : { status: "anonymous" }; + } + + return isError ? { status: "anonymous" } : { status: "loading" }; +}; diff --git a/packages/vitnode/src/views/layouts/theme/header/user/user.tsx b/packages/vitnode/src/views/layouts/theme/header/user/user.tsx index c72556331..958362c84 100644 --- a/packages/vitnode/src/views/layouts/theme/header/user/user.tsx +++ b/packages/vitnode/src/views/layouts/theme/header/user/user.tsx @@ -1,38 +1,23 @@ -import { getTranslations } from "next-intl/server"; - -import { buttonVariants } from "@/components/ui/button"; import { getSessionApi } from "@/lib/api/get-session-api"; -import { Link } from "@/lib/navigation"; -import { cn } from "@/lib/utils"; -import { AuthUserHeader } from "./auth/auth"; +import { NextUserHeader } from "./next-user-header"; +/** + * The user area of the main header, in the Next.js app. + * + * A Server Component whose whole job is the session: it is `await`ed here, once, + * and handed down as data. `getSessionApi()` is wrapped in React's `cache()`, so + * the layout, this header and the page share one round trip - and there is + * deliberately no second read further down, which is what the old + * `AuthUserHeader` did. + * + * Everything rendered is {@link NextUserHeader}'s, and everything *visible* is + * the shared `UserHeaderContent`'s. `HeaderLayout` wraps this in a `<Suspense>` + * whose fallback is that component's own `UserHeaderSkeleton`, so the header + * reserves the right width before the session arrives. + */ export const UserHeader = async () => { - const [t, session] = await Promise.all([ - getTranslations("core.global"), - getSessionApi(), - ]); - - if (!session.user) { - return ( - <> - <Link - className={cn( - buttonVariants({ - variant: "ghost", - }), - )} - href="/login" - > - {t("login")} - </Link> - - <Link className={cn(buttonVariants())} href="/register"> - {t("register")} - </Link> - </> - ); - } + const { user } = await getSessionApi(); - return <AuthUserHeader />; + return <NextUserHeader user={user} />; }; diff --git a/packages/vitnode/src/views/layouts/theme/layout-content.tsx b/packages/vitnode/src/views/layouts/theme/layout-content.tsx new file mode 100644 index 000000000..bb1d88918 --- /dev/null +++ b/packages/vitnode/src/views/layouts/theme/layout-content.tsx @@ -0,0 +1,49 @@ +/** + * The main application shell, as a document structure and nothing else. + * + * Four slots and one `<main>`. Everything that decides *what* goes in a slot - + * reading the session, subscribing to a WebSocket, resolving a breadcrumb from + * the router - is the framework's business and stays in the framework's half: + * `layout.tsx` fills these slots from Server Components, `apps/web`'s `_main` + * route fills them from its router. What both of them get from here is the same + * element order and the same semantic `<main>`, so the two runtimes cannot + * quietly drift into different documents. + * + * It imports nothing. That is deliberate rather than incidental: the moment this + * reaches for `@/lib/navigation`, `next-intl/server` or a `"use server"` module + * it stops being renderable outside Next.js, which is the failure + * `theme-boundaries.test.ts` exists to catch. + * + * ## Why `<main>` is here rather than in each page + * + * There is exactly one `<main>` per document, and a page that renders its own + * inside a shell that also renders one produces two - invalid HTML, and a + * screen reader that now has two "main" landmarks to choose from. The shell owns + * the landmark; a page owns its width, its padding and its vertical rhythm, in + * whatever container it likes. + */ +export const ThemeLayoutContent = ({ + breadcrumb, + children, + header, + listeners, +}: { + /** Rendered between the header and `<main>`, or nothing. */ + breadcrumb?: React.ReactNode; + children: React.ReactNode; + /** The site header. A slot, because its contents are framework-bound. */ + header?: React.ReactNode; + /** + * Components that render nothing and only subscribe - notification toasts, + * the WebSocket's sign-in/sign-out resync. First in the tree so they are + * mounted before anything that can produce an event for them. + */ + listeners?: React.ReactNode; +}) => ( + <> + {listeners} + {header} + {breadcrumb} + <main>{children}</main> + </> +); diff --git a/packages/vitnode/src/views/layouts/theme/layout.tsx b/packages/vitnode/src/views/layouts/theme/layout.tsx index 1be246aaf..00378153d 100644 --- a/packages/vitnode/src/views/layouts/theme/layout.tsx +++ b/packages/vitnode/src/views/layouts/theme/layout.tsx @@ -5,6 +5,7 @@ import { getSessionApi } from "@/lib/api/get-session-api"; import type { VitNodeConfig } from "../../../vitnode.config"; import { HeaderLayout } from "./header/header"; +import { ThemeLayoutContent } from "./layout-content"; import { NotificationListener } from "./notification-listener"; import { WebSocketAuthSync } from "./web-socket-auth-sync"; @@ -14,6 +15,14 @@ const WebSocketAuthSyncSession = async () => { return <WebSocketAuthSync userId={session?.user?.id ?? null} />; }; +/** + * The main shell for Next.js. + * + * The structure - the slot order and the `<main>` landmark - is + * `ThemeLayoutContent`, shared with the TanStack Start app. What stays here is + * the half that is genuinely Next.js: a header that is an async Server + * Component, and a session read that only a Server Component can await. + */ export const ThemeLayout = ({ children, logo, @@ -25,14 +34,19 @@ export const ThemeLayout = ({ vitNodeConfig: VitNodeConfig; }) => { return ( - <> - <NotificationListener /> - <Suspense> - <WebSocketAuthSyncSession /> - </Suspense> - <HeaderLayout logo={logo} vitNodeConfig={vitNodeConfig} /> - {breadcrumb} - <main>{children}</main> - </> + <ThemeLayoutContent + breadcrumb={breadcrumb} + header={<HeaderLayout logo={logo} vitNodeConfig={vitNodeConfig} />} + listeners={ + <> + <NotificationListener /> + <Suspense> + <WebSocketAuthSyncSession /> + </Suspense> + </> + } + > + {children} + </ThemeLayoutContent> ); }; diff --git a/packages/vitnode/src/views/layouts/theme/theme-boundaries.test.ts b/packages/vitnode/src/views/layouts/theme/theme-boundaries.test.ts new file mode 100644 index 000000000..bf44b074a --- /dev/null +++ b/packages/vitnode/src/views/layouts/theme/theme-boundaries.test.ts @@ -0,0 +1,186 @@ +// @vitest-environment node +import { existsSync, readFileSync, statSync } from "node:fs"; +import { dirname, join, relative, resolve } from "node:path"; +import { fileURLToPath } from "node:url"; +import { describe, expect, it } from "vitest"; + +const here = dirname(fileURLToPath(import.meta.url)); +const srcRoot = resolve(here, "../../.."); + +/** + * The main shell, split down the middle. + * + * The same boundary `auth-boundaries.test.ts` draws around the login screens, + * for the same reason and with the same machinery: `ThemeLayoutContent` is + * rendered by a TanStack Start route as well as by Next.js, and a single import + * that only resolves inside a Next.js app turns that route into a build error + * nobody sees until they try it. + * + * The shared half is the *structure* - the slot order and the `<main>` landmark. + * Everything that fills a slot is the framework's, and `layout.tsx` is the proof + * that the Next.js half really does reach the things the shared half must not. + */ +const SHARED_ENTRY = join(here, "layout-content.tsx"); +const NEXT_WRAPPER = join(here, "layout.tsx"); + +const resolveSpecifier = (specifier: string, from: string): null | string => { + let base: string; + + if (specifier.startsWith("@/")) base = join(srcRoot, specifier.slice(2)); + else if (specifier.startsWith(".")) base = resolve(dirname(from), specifier); + else return null; + + for (const suffix of [".ts", ".tsx", "/index.ts", "/index.tsx"]) { + const candidate = base + suffix; + if (existsSync(candidate) && statSync(candidate).isFile()) return candidate; + } + + return existsSync(base) && statSync(base).isFile() ? base : null; +}; + +/** Every specifier a file imports at runtime; `import type` is erased first. */ +const runtimeImports = (path: string): string[] => { + const source = readFileSync(path, "utf8").replace( + /(^|[\n;])\s*import\s+type\s[\s\S]*?from\s*["'][^"']+["']/g, + "$1", + ); + + return [ + ...source.matchAll( + /(?:^|[^\w$.])from\s*["']([^"']+)["']|import\s*\(\s*["']([^"']+)["']|(?:^|[\n;}])\s*import\s*["']([^"']+)["']/g, + ), + ] + .map(match => match[1] ?? match[2] ?? match[3]) + .filter((specifier): specifier is string => Boolean(specifier)); +}; + +/** Every external specifier reachable from an entry, with the chain that got there. */ +const externalGraph = (entry: string): Map<string, string[]> => { + const found = new Map<string, string[]>(); + const parents = new Map<string, string>(); + const seen = new Set<string>(); + + const chain = (file: string): string => { + const parts: string[] = []; + for (let at: string | undefined = file; at; at = parents.get(at)) { + parts.unshift(relative(srcRoot, at)); + } + + return parts.join(" -> "); + }; + + const walk = (file: string) => { + if (seen.has(file)) return; + seen.add(file); + + for (const specifier of runtimeImports(file)) { + const target = resolveSpecifier(specifier, file); + + if (target) { + if (!parents.has(target)) parents.set(target, file); + walk(target); + continue; + } + + found.set(specifier, [...(found.get(specifier) ?? []), chain(file)]); + } + }; + + walk(entry); + + return found; +}; + +const matches = (specifier: string, forbidden: string): boolean => + specifier === forbidden || specifier.startsWith(`${forbidden}/`); + +const offenders = (entry: string, forbidden: string[]): string[] => + [...externalGraph(entry)] + .filter(([specifier]) => forbidden.some(one => matches(specifier, one))) + .flatMap(([specifier, chains]) => chains.map(at => `${specifier} in ${at}`)) + .sort(); + +/** Anything that only resolves inside a Next.js app. */ +const NEXT_ONLY = ["next", "server-only"]; + +/** + * `next-intl`'s Next-only halves. The root entry is absent on purpose: it + * re-exports `use-intl`, which is framework-free. + */ +const NEXT_INTL_RUNTIME = [ + "next-intl/middleware", + "next-intl/navigation", + "next-intl/plugin", + "next-intl/server", +]; + +describe("the import scan finds what it is looking for", () => { + // Every assertion below is a "found nothing" one, which a scanner that + // silently matches nothing also satisfies. The Next wrapper is the control. + it("finds the Next-only imports in the Next wrapper", () => { + expect( + offenders(NEXT_WRAPPER, [...NEXT_ONLY, ...NEXT_INTL_RUNTIME]), + ).not.toEqual([]); + }); + + it("walks past the entry file into its dependencies", () => { + // The session read is two hops from the wrapper, not one: `layout.tsx` -> + // `lib/api/get-session-api` -> `lib/fetcher`. + expect([...externalGraph(NEXT_WRAPPER).keys()]).toContain("next/headers"); + }); +}); + +describe("the shared main shell is framework-neutral", () => { + it("reaches nothing from next/*", () => { + expect(offenders(SHARED_ENTRY, NEXT_ONLY)).toEqual([]); + }); + + it("reaches none of next-intl's Next-only entrypoints", () => { + expect(offenders(SHARED_ENTRY, NEXT_INTL_RUNTIME)).toEqual([]); + }); + + it("never reaches the locale-aware navigation module", () => { + const reached = [...externalGraph(SHARED_ENTRY).keys()]; + + expect(reached.some(one => one.includes("navigation"))).toBe(false); + }); + + it("never reaches a server action", () => { + const reached = [...externalGraph(SHARED_ENTRY).keys()]; + + expect(reached.some(one => one.endsWith(".server"))).toBe(false); + expect( + runtimeImports(SHARED_ENTRY).some(one => one.includes(".server")), + ).toBe(false); + }); +}); + +describe("the shared main shell takes its framework parts as slots", () => { + const withoutComments = (path: string): string => + readFileSync(path, "utf8") + .replace(/\/\*[\s\S]*?\*\//g, "") + .replace(/\/\/.*$/gm, ""); + + const code = withoutComments(SHARED_ENTRY); + + it.each(["breadcrumb", "header", "listeners"])( + "asks for %s rather than rendering it", + slot => { + expect(code).toContain(slot); + }, + ); + + it("renders the header and the notification listeners itself in neither case", () => { + expect(code).not.toContain("HeaderLayout"); + expect(code).not.toContain("NotificationListener"); + expect(code).not.toContain("WebSocketAuthSync"); + }); + + /** + * One `<main>`, in the shell. A page under it renders its own container, not a + * second landmark - see the note on `ThemeLayoutContent`. + */ + it("owns the one main landmark", () => { + expect(code.match(/<main>/g)).toHaveLength(1); + }); +}); diff --git a/packages/vitnode/src/views/layouts/theme/web-socket-auth-sync.tsx b/packages/vitnode/src/views/layouts/theme/web-socket-auth-sync.tsx index c20c0d4be..9e2de9fdd 100644 --- a/packages/vitnode/src/views/layouts/theme/web-socket-auth-sync.tsx +++ b/packages/vitnode/src/views/layouts/theme/web-socket-auth-sync.tsx @@ -1,21 +1,48 @@ "use client"; -// The user id comes from the server render, so the reconnect must be driven by -// the prop changing (sign-in/sign-out) rather than a client event handler. +// The user id comes from whatever the app resolved the session to - a server +// render in Next.js, the canonical session query in TanStack Start - so the +// reconnect must be driven by the prop changing (sign-in/sign-out) rather than +// by a client event handler. /* eslint-disable react-you-might-not-need-an-effect/no-event-handler */ import React from "react"; +import type { VitNodeSocketUserId } from "@/ws/auth-sync"; + +import { shouldReconnectForUser } from "@/ws/auth-sync"; import { useVitNodeWebSocketContext } from "@/ws/provider"; -export const WebSocketAuthSync = ({ userId }: { userId: null | number }) => { +/** + * Keeps the shared WebSocket authenticated as the visitor the app currently + * believes in. + * + * Renders nothing and holds no state of its own: it is a client effect driven by + * one input. Which is what makes it framework-neutral - the app decides where + * `userId` comes from (`getSessionApi()` in Next.js, the canonical session query + * in TanStack Start) and this only reacts to it changing. + * + * `undefined` means the session is not known yet, and is the normal first value + * on a framework that reads it in the browser. It is deliberately not the same + * as `null`; see {@link shouldReconnectForUser}, which owns that distinction. + */ +export const WebSocketAuthSync = ({ + userId, +}: { + userId: undefined | VitNodeSocketUserId; +}) => { const { reconnect } = useVitNodeWebSocketContext(); const previousUserIdRef = React.useRef(userId); React.useEffect(() => { - if (previousUserIdRef.current === userId) return; + const shouldReconnect = shouldReconnectForUser( + previousUserIdRef.current, + userId, + ); - previousUserIdRef.current = userId; - reconnect(); + // Only a *known* identity advances the ref, so a session that becomes + // unknown again does not erase the one the socket is carrying. + if (userId !== undefined) previousUserIdRef.current = userId; + if (shouldReconnect) reconnect(); }, [userId, reconnect]); return null; diff --git a/packages/vitnode/src/ws/auth-sync.test.ts b/packages/vitnode/src/ws/auth-sync.test.ts new file mode 100644 index 000000000..baacf5f2d --- /dev/null +++ b/packages/vitnode/src/ws/auth-sync.test.ts @@ -0,0 +1,54 @@ +import { describe, expect, it } from "vitest"; + +import { shouldReconnectForUser } from "./auth-sync"; + +/** + * The transitions `WebSocketAuthSync` has to follow, and the ones it must leave + * alone. + * + * Worth stating as tests because both mistakes here are silent. Missing a + * transition means the server keeps delivering the previous visitor's + * notifications to a browser that has signed out of them - a leak, and one no + * error surfaces. Reconnecting when nothing changed means dropping the shared + * connection, which the manager relays to every tab of the origin, on a signal + * that arrives on every page load. + */ +describe("shouldReconnectForUser", () => { + it("reconnects when a guest signs in", () => { + expect(shouldReconnectForUser(null, 7)).toBe(true); + }); + + it("reconnects when a signed-in visitor signs out", () => { + expect(shouldReconnectForUser(7, null)).toBe(true); + }); + + it("reconnects when one visitor is replaced by another", () => { + expect(shouldReconnectForUser(7, 9)).toBe(true); + }); + + it("does nothing when the session answers the same visitor again", () => { + // What a re-render, a refetch or a route change produces. The socket is + // already authenticated as this user. + expect(shouldReconnectForUser(7, 7)).toBe(false); + }); + + it("does nothing when a guest is still a guest", () => { + expect(shouldReconnectForUser(null, null)).toBe(false); + }); + + it("does nothing on the first identity a client learns", () => { + // The client-side session read: the socket opened with the visitor's + // cookies, so the server already has the right user. Reconnecting here + // would re-open every tab's connection on every page load. + expect(shouldReconnectForUser(undefined, 7)).toBe(false); + expect(shouldReconnectForUser(undefined, null)).toBe(false); + }); + + it("does nothing when the session becomes unknown", () => { + // Nothing has been learned, so nothing is decided - and the caller keeps + // the last identity it did know rather than treating this as a sign-out. + expect(shouldReconnectForUser(7, undefined)).toBe(false); + expect(shouldReconnectForUser(null, undefined)).toBe(false); + expect(shouldReconnectForUser(undefined, undefined)).toBe(false); + }); +}); diff --git a/packages/vitnode/src/ws/auth-sync.ts b/packages/vitnode/src/ws/auth-sync.ts new file mode 100644 index 000000000..6d475ffd1 --- /dev/null +++ b/packages/vitnode/src/ws/auth-sync.ts @@ -0,0 +1,62 @@ +/** + * Whether a change of signed-in user requires the shared WebSocket to be + * re-opened - the one decision behind `WebSocketAuthSync`, as a pure function. + * + * It lives here, next to the manager it ultimately drives, rather than inside + * the component: the rule is about the socket's handshake, it is identical in + * every framework that mounts the component, and it is the part that can be + * wrong without anything failing loudly. + */ + +/** + * The user a WebSocket connection is authenticated as, as far as the client + * knows. + * + * `number` is a signed-in visitor, `null` a guest - the server derives the same + * value from the session cookie on the upgrade request (`handleVitNodeWebSocket` + * tags each connection with `c.get("user")?.id ?? null`) so it can deliver a + * per-user payload to the right connections. + */ +export type VitNodeSocketUserId = null | number; + +/** + * Whether the socket has to be dropped and re-opened, given the last identity + * it was known to carry and the one it should carry now. + * + * The connection authenticates once, during its HTTP upgrade, from the cookies + * the browser sent with it. Nothing afterwards can change who the server thinks + * it belongs to - so the only way to follow a sign-in or a sign-out is a fresh + * handshake, and {@link WebSocketManager.reconnect} is what performs one. + * + * ## `undefined` is "not known yet", and it must not reconnect + * + * A framework that resolves the session on the server before rendering - Next.js + * with `getSessionApi()` - always passes a known value, so this case never + * arises there. A client-side session read does: the component's first render + * happens before the query has answered, and the identity arrives one render + * later. + * + * Both directions of that are handled here, and both matter: + * + * - `next === undefined` - the session has become unknown again (a cleared cache + * entry). Nothing is learned, so nothing is done, and the caller keeps the + * last identity it *did* know rather than forgetting it. + * - `previous === undefined` - the first identity this client has learned. The + * socket already opened with the visitor's cookies attached, so the server has + * had the right user all along and there is nothing to correct. Reconnecting + * here would tear down the shared connection - and, because the manager + * relays a reconnect to the leader tab, every tab's connection with it - on + * every single page load. + * + * Once both sides are known it is a plain inequality: guest to user, user to + * guest, and one user to another are each a new handshake; the same id answered + * twice is not. + */ +export const shouldReconnectForUser = ( + previous: undefined | VitNodeSocketUserId, + next: undefined | VitNodeSocketUserId, +): boolean => { + if (next === undefined || previous === undefined) return false; + + return previous !== next; +}; diff --git a/plugins/example/src/routes/example-page.tsx b/plugins/example/src/routes/example-page.tsx index 6807bf51a..ca8f70c3c 100644 --- a/plugins/example/src/routes/example-page.tsx +++ b/plugins/example/src/routes/example-page.tsx @@ -12,9 +12,17 @@ * It exports a default component because that is how every VitNode plugin page * already exports itself, and because a default export is the one name a * generated registry can rely on without being told. + * + * No `<main>`, and that is part of the contract rather than a style choice. A + * plugin route declares `area: "main"`, which puts it inside the application + * shell - and the shell renders the document's one `<main>` landmark. A page + * that renders its own produces `<main><main>`: invalid HTML, and two "main" + * landmarks for a screen reader to choose between. A plugin page owns its + * container - its width, its padding, its vertical rhythm - and nothing above + * it. */ const ExamplePage = () => ( - <main className="container mx-auto flex max-w-2xl flex-col gap-4 p-4"> + <div className="container mx-auto flex max-w-2xl flex-col gap-4 p-4"> <h1 className="text-2xl font-semibold tracking-tight text-balance"> Example plugin route </h1> @@ -25,7 +33,7 @@ const ExamplePage = () => ( generated a literal import for it from the plugin's route manifest, and the bundler put it in its own chunk. </p> - </main> + </div> ); export default ExamplePage;