diff --git a/apps/web/.env.example b/apps/web/.env.example index 6a1dbbcf4..1f7beac89 100644 --- a/apps/web/.env.example +++ b/apps/web/.env.example @@ -15,6 +15,19 @@ REDIS_URL=redis://localhost:6379 NEXT_PUBLIC_WEB_URL=http://localhost:3001 NEXT_PUBLIC_API_URL=http://localhost:3001 +# The legacy Next.js application (`apps/docs`), while the migration is in +# progress. This app owns `/` and `/discover`; `/blog/*`, `/files/*`, `/search`, +# `/admin/*` and every plugin route are still served by Next.js on 3000, so a +# search result pointing at one has to leave this origin to reach it. Without +# this, `/blog/post-1` resolves against 3001 and 404s here instead. +# +# 3001 this app (vite dev) +# 3000 the legacy app (apps/docs, next dev) +# +# Temporary: delete it, and `src/lib/legacy-app.ts`, with the last legacy route. +# Leave it unset only if something in front of both apps routes by path. +NEXT_PUBLIC_LEGACY_WEB_URL=http://localhost:3000 + # === CRON Secret for Internal API Calls === CRON_SECRET=your-secure-cron-secret-key diff --git a/apps/web/package.json b/apps/web/package.json index 437a29c31..86755d359 100644 --- a/apps/web/package.json +++ b/apps/web/package.json @@ -7,7 +7,6 @@ }, "scripts": { "dev": "vite dev --port 3001", - "generate-routes": "tsr generate", "build": "vite build", "preview": "vite preview", "test": "vitest run", @@ -52,12 +51,15 @@ "@tanstack/devtools-vite": "^0.8.5", "@tanstack/eslint-config": "^0.4.0", "@tanstack/router-cli": "^1.132.0", + "@testing-library/dom": "^10.4.1", + "@testing-library/react": "^16.3.2", "@types/node": "^22.10.2", "@types/react": "^19.2.0", "@types/react-dom": "^19.2.0", "@vitejs/plugin-react": "^6.0.1", "@vitnode/config": "workspace:*", "eslint": "^10.7.0", + "jsdom": "^29.1.1", "tw-animate-css": "^1.4.0", "typescript": "^6.0.2", "vite": "^8.0.0", diff --git a/apps/web/src/components/migration-link.tsx b/apps/web/src/components/migration-link.tsx new file mode 100644 index 000000000..16850bb30 --- /dev/null +++ b/apps/web/src/components/migration-link.tsx @@ -0,0 +1,123 @@ +import type { AnyRouter } from '@tanstack/react-router' + +import { Link, useRouter } from '@tanstack/react-router' + +import { useLocale } from '#/lib/i18n/client' +import { localeRouting } from '#/lib/i18n/shared' +import { buildLegacyHref, legacyWebOrigin } from '#/lib/legacy-app' + +/** + * Linking to a VitNode page while half of VitNode still runs on Next.js. + * + * This app owns three routes today - `/`, `/discover` and the `/api/*` mount - + * and search results point at all of the ones it does not: `/blog/post-30`, + * `/files/...`, `/admin/...`, whatever a plugin indexed. Handing every + * internal-looking path to `` routes those into *this* router, which has + * nothing to match them with, so a perfectly good blog post becomes a TanStack + * not-found page. During a strangler migration a full document load to the + * running Next.js app is the correct answer, not a fallback. + * + * So: ask the router what it owns, and let it answer. + * + * owned -> , client-side navigation, locale prefix from the rewrite + * not owned -> , document navigation, locale prefix applied here + * + * This is deliberately not a cross-framework navigation system, and there is no + * hand-maintained table of migrated routes - the route tree *is* the table. When + * `/blog` is migrated it appears in the generated tree, `isTanStackOwnedPath` + * starts answering `true` for it, and nothing here changes. + */ + +/** + * The API mount is not a page. + * + * `/api/$` is a real route in the generated tree - it is how Hono is mounted - + * so it matches, and without this a search result pointing into `/api` would be + * handed to the router as a client-side navigation to a route that renders + * nothing. Matched by route id rather than by a hardcoded pathname, so it stays + * correct if the mount ever moves. + */ +const isApiRouteId = (routeId: string): boolean => + routeId === '/api' || routeId.startsWith('/api/') + +/** + * Whether this app's route tree can render `href` itself. + * + * Three things have to happen before the router is asked, and each one is a way + * this returned the wrong answer while it was being written: + * + * 1. **Strip the query and hash.** `matchRoutes` takes a *pathname*; + * `/discover?a=1` matches nothing. + * 2. **De-localize.** The route tree has no locale in it - that is the whole of + * Stage 3 - so `/pl/discover` matches nothing until the prefix comes off. + * 3. **Reject the API mount.** See {@link isApiRouteId}. + * + * An unmatched path resolves to the root route alone, so "something below the + * root matched" is the test. That also means a root-level catch-all route would + * make every path look owned; there is none today, and + * `migration-link.test.tsx` fails loudly if one appears. + */ +export const isTanStackOwnedPath = ( + router: AnyRouter, + href: string, +): boolean => { + // The same rule `rewrite.input` applies, from the same Stage 3 helper - the + // rewrite is `deLocalizeUrl` and nothing else, so this is one rule, not a copy. + const { pathname } = localeRouting.deLocalizeUrl( + new URL(href, 'https://vitnode.invalid'), + ) + + const matched = router + .matchRoutes(pathname, undefined) + .map((match: { routeId: string }) => match.routeId) + .filter((routeId: string) => routeId !== '__root__') + + return matched.length > 0 && !matched.some(isApiRouteId) +} + +/** + * A link to anywhere in VitNode, migrated or not. + * + * The two branches differ in origin as well as in mechanism, which is the whole + * point: a relative `/blog/post-1` from this app resolves against *this* app, + * so it turned a client-side not-found into a full-document not-found rather + * than reaching the application that owns the route. + * + * - **Owned.** `` takes the *internal* path and stays relative. The + * router's `rewrite.output` writes the locale prefix, so `/discover` renders + * as `/pl/discover` while reading Polish. Neither an origin nor a prefix is + * added here; either would be a duplicate. + * - **Not owned.** The router never sees the URL, so `buildLegacyHref` localizes + * it with the same Stage 3 rule and points it at the legacy origin. + * + * Search parameters and hashes survive both branches untouched. + */ +export const MigrationLink = ({ + children, + className, + href, +}: { + children: React.ReactNode + className?: string + href: string +}) => { + const router = useRouter() + const locale = useLocale() + + if (isTanStackOwnedPath(router, href)) { + return ( + + {children} + + ) + } + + return ( + + {children} + + ) +} diff --git a/apps/web/src/components/route-messages.tsx b/apps/web/src/components/route-messages.tsx new file mode 100644 index 000000000..5c0999580 --- /dev/null +++ b/apps/web/src/components/route-messages.tsx @@ -0,0 +1,61 @@ +import { useSuspenseQuery } from '@tanstack/react-query' +import { IntlProvider as CoreIntlProvider } from '@vitnode/core/lib/i18n/provider' +import { IntlProvider } from 'use-intl' + +import { i18n } from '#/i18n' +import { useLocale } from '#/lib/i18n/client' +import { intlQueryOptions } from '#/lib/i18n/query' + +/** + * The strings one route renders, scoped to that route. + * + * The root provides `core.global` and nothing else, deliberately: the merged + * message tree holds every plugin's AdminCP copy, and a page should ship only + * the branches it actually renders. This is the other half of that rule - the + * TanStack Start counterpart of ``, which is + * how the Next.js pages have always done it. + * + * ## It reads, it does not fetch + * + * `useSuspenseQuery` over the same `intlQueryOptions` the route's loader + * already warmed, so on the first render the entry is there and nothing + * suspends. A route that mounts this **must** ensure the identical options in + * its loader - same locale, same namespaces - or the first paint is a suspend + * and the strings arrive a round trip late. + * + * ## Why two providers + * + * One component, two module records. `@vitnode/core` is external to Vite's SSR + * pass and therefore loaded by Node, while this app's source runs through + * Vite's module runner - so `use-intl` imported here and `use-intl` imported + * inside a core component can be two records with two React contexts. The + * outer one covers this app's own code; the inner comes from core itself + * (`@vitnode/core/lib/i18n/provider`) and so is by construction the record + * every shared component reads. See the long note in `routes/__root.tsx`, which + * has the same shape for the same reason. + * + * Both get the same props from one object: two providers that disagreed would + * render half a page in the wrong language. + */ +export const RouteMessages = ({ + children, + namespaces, +}: { + children: React.ReactNode + namespaces: readonly string[] +}) => { + const locale = useLocale() + const { data } = useSuspenseQuery(intlQueryOptions({ locale, namespaces })) + + const intlProps = { + locale, + messages: data.messages, + timeZone: i18n.timeZone, + } + + return ( + + {children} + + ) +} diff --git a/apps/web/src/lib/i18n/client.ts b/apps/web/src/lib/i18n/client.ts index 09ba752a1..f53ded7ae 100644 --- a/apps/web/src/lib/i18n/client.ts +++ b/apps/web/src/lib/i18n/client.ts @@ -11,7 +11,7 @@ import { import type { Locale } from './shared' -import { intlQueryOptions } from './query' +import { intlQueryOptions, loadedIntlNamespaces } from './query' import { localeRouting } from './shared' /** @@ -95,6 +95,29 @@ export const publicPathnameOf = ({ publicHref: string }): string => new URL(publicHref, RELATIVE_BASE).pathname +/** + * An internal href, written in the public shape for one language. + * + * /blog/post-30 + pl -> /pl/blog/post-30 + * /blog/post-30 + en -> /blog/post-30 + * /admin/users + pl -> /admin/users (an ignored path takes no prefix) + * + * For links the router will never build. Anything it *does* build gets its + * prefix from `rewrite.output` instead, and applying both would produce + * `/pl/pl/...` - so this is only for the migration boundary in + * `components/migration-link.tsx`, where the destination belongs to the Next.js + * app and the router is deliberately not involved. + * + * `localizePathname` is the same Stage 3 rule the rewrite uses and is + * idempotent, so an href that already carries a prefix keeps exactly one. The + * query string and hash are preserved. + */ +export const localizeHref = (href: string, locale: Locale): string => { + const url = localeRouting.localizeUrl(new URL(href, RELATIVE_BASE), locale) + + return `${url.pathname}${url.search}${url.hash}` +} + /** * The router's half of locale routing: one route tree, two public URL shapes. * @@ -143,7 +166,15 @@ export const useLocale = (): Locale => /** * Puts a language's messages in the cache before anything renders in it. * - * Failure is deliberately not fatal: the switch still happens, and the + * Every set the page is currently showing, not just the global one. The root + * provides `core.global`; a route provides whatever it renders on top of that + * (`RouteMessages`), and both read through `useSuspenseQuery`. Warming only the + * first would leave the second suspending on a key nobody had fetched - which, + * because the suspend is caused by a store update, cannot be deferred: the page + * blanks for a round trip. `loadedIntlNamespaces` answers "which sets" by + * reading the cache, so this stays right as more routes declare their own. + * + * Failure is deliberately not fatal: the switch still happens, and each * provider's own query retries it. A language that cannot be fetched should * degrade to a moment of loading, not to a switcher that appears to do nothing. */ @@ -151,12 +182,21 @@ const warmMessages = async (router: AnyRouter, locale: Locale) => { const { queryClient } = router.options.context as { queryClient?: QueryClient } + if (!queryClient) return - try { - await queryClient?.ensureQueryData(intlQueryOptions({ locale })) - } catch { - /* empty */ - } + const current = resolveLocale(publicPathnameOf(router.latestLocation)) + + await Promise.all( + loadedIntlNamespaces(queryClient, current).map(async (namespaces) => { + try { + await queryClient.ensureQueryData( + intlQueryOptions({ locale, namespaces }), + ) + } catch { + /* empty */ + } + }), + ) } /** diff --git a/apps/web/src/lib/i18n/query.ts b/apps/web/src/lib/i18n/query.ts index 83bc8d983..a9cca5b84 100644 --- a/apps/web/src/lib/i18n/query.ts +++ b/apps/web/src/lib/i18n/query.ts @@ -1,3 +1,5 @@ +import type { QueryClient } from '@tanstack/react-query' + import { queryOptions } from '@tanstack/react-query' import { createServerFn } from '@tanstack/react-start' @@ -10,6 +12,19 @@ import { localeRouting } from './shared' /** The strings every page needs, whatever else it renders. */ export const GLOBAL_NAMESPACE = 'core.global' +/** Everything a message entry's key starts with, before the language. */ +const INTL_QUERY_SCOPE = ['vitnode', 'intl'] as const + +/** + * One language's slice of the message cache. + * + * Its own function because two things need it: the key each entry is stored + * under, and the prefix `loadedIntlNamespaces` searches by. Spelling the prefix + * out twice would let a search silently stop matching the keys it is looking + * for. + */ +const intlQueryPrefix = (locale: Locale) => [...INTL_QUERY_SCOPE, locale] + /** * Namespaces in a form two callers cannot spell differently. * @@ -172,7 +187,38 @@ export const intlQueryOptions = ({ return queryOptions({ queryFn: async () => await getIntlMessages({ data: { locale, namespaces: normalized } }), - queryKey: ['vitnode', 'intl', locale, ...normalized] as const, + queryKey: [...intlQueryPrefix(locale), ...normalized] as const, staleTime: Infinity, }) } + +/** + * Every namespace set a client currently holds messages for, in one language. + * + * Read off the cache rather than declared anywhere, and that is the point: the + * root asks for `core.global`, a route asks for whatever it renders, and by the + * time somebody switches language the cache is the only place that knows which + * sets are on screen. A language switch has to warm *those* - warming only the + * global set leaves the route's provider suspending on a key nobody fetched, + * which blanks the page for a round trip. + * + * Falls back to the global set, so a switch made before anything has loaded + * still warms the one set every page needs. + */ +export const loadedIntlNamespaces = ( + queryClient: QueryClient, + locale: Locale, +): string[][] => { + const prefix = intlQueryPrefix(locale) + const sets = queryClient + .getQueryCache() + .findAll({ queryKey: prefix }) + .map(({ queryKey }) => + queryKey + .slice(prefix.length) + .filter((part): part is string => typeof part === 'string'), + ) + .filter((namespaces) => namespaces.length > 0) + + return sets.length > 0 ? sets : [[GLOBAL_NAMESPACE]] +} diff --git a/apps/web/src/lib/legacy-app.ts b/apps/web/src/lib/legacy-app.ts new file mode 100644 index 000000000..228ab112d --- /dev/null +++ b/apps/web/src/lib/legacy-app.ts @@ -0,0 +1,71 @@ +import type { Locale } from '#/lib/i18n/shared' + +import { localizeHref } from '#/lib/i18n/client' + +/** + * Where the half of VitNode that has not moved yet is actually served. + * + * Temporary migration infrastructure, and deliberately this app's own rather + * than `@vitnode/core`'s `CONFIG`: "there is a second, older application" is + * true for the length of this migration and false before and after it, so it + * does not belong in the permanent configuration every VitNode install carries. + * It is expected to be deleted with the last legacy route. + * + * It is *not* `NEXT_PUBLIC_WEB_URL`, which already means "this application's own + * public origin" and is stamped into cookies, SSO callbacks, password-reset + * links and every email VitNode sends. Reusing it here would point all of those + * at the wrong app. + * + * Read through a getter rather than captured at module load, matching how + * `CONFIG` reads its own values: the browser gets this inlined at build time + * (see `CLIENT_ENV_KEYS` in `vitnode-env.ts`) and the server reads the live + * environment, so a built server can be repointed by its host. + */ +export const legacyWebOrigin = (): string | undefined => { + const configured = process.env.NEXT_PUBLIC_LEGACY_WEB_URL + + if (!configured) return undefined + + try { + return new URL(configured).origin + } catch { + // A typo in a public env var should not blank a page. Falling through to + // `undefined` degrades to a same-origin link, which is wrong in exactly the + // deployment that got the value wrong and harmless in the one that has no + // second origin at all. + return undefined + } +} + +/** + * A link to a route the legacy application still owns. + * + * Pure, so the two decisions it makes are testable without a router or a DOM: + * + * 1. **Localize.** `localizeHref` is Stage 3's own rule - idempotent, and a + * no-op for the default locale and for paths outside the localized URL space + * (`/admin`). Nothing here concatenates a prefix, so `/pl/pl/...` is not a + * shape this can produce. + * 2. **Re-origin.** With `legacyOrigin` set, the localized path is resolved + * against it, which is what turns a document navigation into one that + * actually leaves this server. The query string and hash ride along. + * + * With no `legacyOrigin` the result stays relative - the pre-existing behaviour, + * and the right one when a proxy in front of both apps routes by path. VitNode + * ships no such proxy today, which is why `.env.example` sets the variable. + */ +export const buildLegacyHref = ({ + href, + legacyOrigin, + locale, +}: { + href: string + legacyOrigin?: string + locale: Locale +}): string => { + const localized = localizeHref(href, locale) + + if (!legacyOrigin) return localized + + return new URL(localized, legacyOrigin).href +} diff --git a/apps/web/src/lib/search/discover-feed.ts b/apps/web/src/lib/search/discover-feed.ts new file mode 100644 index 000000000..32c2feb20 --- /dev/null +++ b/apps/web/src/lib/search/discover-feed.ts @@ -0,0 +1,93 @@ +import type { + SearchFeedPageArgs, + SearchFeedPageFetcher, +} from '@vitnode/core/views/search/search-feed-query' + +import { createIsomorphicFn } from '@tanstack/react-start' +import { + fetchSearchFeedPageInBrowser, + searchFeedQueryKey, + searchFeedQueryOptions, +} from '@vitnode/core/views/search/search-feed-query' + +import type { Locale } from '#/lib/i18n/shared' + +import { DISCOVER_FEED_PARAMS } from '#/lib/search/discover-request' +import { fetchDiscoverFeedPageOnServer } from '#/server/discover-feed.server' + +/** + * The Discover feed, as this app's one query definition. + * + * Everything about *what* a feed page is - the request, the page size, the + * cursor rule, what counts as a failure - comes from + * `@vitnode/core/views/search/search-feed-query`, which is also what the mounted + * `SearchFeedContent` runs. This module supplies only the two things core cannot + * know: which parameters Discover browses with, and how to reach the API from a + * server that is rendering a request. + */ + +/** + * The transport boundary, and the reason one query definition works in a loader + * and in a component. + * + * Both branches call the Hono API directly - the server one from inside the + * request being rendered, the browser one over the network to the same origin. + * There is deliberately no `createServerFn` in between: a server function is a + * `POST` back to this app that then calls Hono, so every scroll of the feed + * would cost two round trips to fetch a public, anonymous read that the API is + * already the boundary for. + * + * `createIsomorphicFn` is what makes that safe rather than merely tidy. The + * Start compiler keeps only the branch belonging to the bundle it is building + * and drops the other's import with it, so `discover-feed.server.ts` - and the + * `server-only` marker at the top of it - never reaches the browser. The client + * branch is core's own browser fetcher, so a hydrated page and a Next.js page + * fetch through exactly the same code. + * + * Un-compiled (tests, plain Node) the stub falls back to the server branch, + * which is the right default off a browser. + */ +const fetchDiscoverFeedPage: SearchFeedPageFetcher = createIsomorphicFn() + .server(fetchDiscoverFeedPageOnServer) + .client(fetchSearchFeedPageInBrowser) + +/** + * The cache entry one language's feed lives in. + * + * Core's key, not one of this app's devising. `SearchFeedContent` runs the + * mounted `useInfiniteQuery` and stores its pages here; a key invented locally + * would be a *second* entry holding the same feed, so the loader would fill one, + * the component would miss the other, and every visit would render a skeleton + * and fetch page one again from the browser. + * + * The locale is in it, which is the whole contract: `/discover` and + * `/pl/discover` are two feeds over two sets of documents, so they get two + * entries. A language switch changes the key rather than the value under it. + */ +export const discoverFeedQueryKey = (locale: Locale) => + searchFeedQueryKey({ locale, params: DISCOVER_FEED_PARAMS }) + +/** + * The Discover feed, as the one query definition every caller shares. + * + * loader: context.queryClient.ensureInfiniteQueryData(options) + * component: + * load more: fetchNextPage() // the same queryFn, cursor rule and checks + * + * No `initialData`. The loader has already put page one in the entry this key + * names and the SSR pass dehydrates it, so passing it again would be a second + * copy of the same bytes that can disagree with the first. + * + * No `staleTime` either. Freshness is whatever the API's own caching gives, plus + * VitNode's client defaults (`refetchOnMount` and `refetchOnWindowFocus` both + * off), so a hydrated feed is not refetched behind the reader. Deciding a cache + * lifetime belongs to the caching stage, with the API and Redis in the same view. + */ +export const discoverFeedQueryOptions = ({ locale }: { locale: Locale }) => + searchFeedQueryOptions({ + fetchPage: fetchDiscoverFeedPage, + locale, + params: DISCOVER_FEED_PARAMS, + }) + +export type { SearchFeedPageArgs } diff --git a/apps/web/src/lib/search/discover-request.ts b/apps/web/src/lib/search/discover-request.ts new file mode 100644 index 000000000..a6a1a1d14 --- /dev/null +++ b/apps/web/src/lib/search/discover-request.ts @@ -0,0 +1,32 @@ +import type { SearchFeedParams } from '@vitnode/core/views/search/search-feed-query' + +/** + * What the Discover feed *is*, with nothing about how it travels. + * + * Two constants and no logic. Building the request, checking the response, + * deciding the next cursor and naming the cache entry all live in + * `@vitnode/core/views/search/search-feed-query`, because the mounted feed + * component does the same things and there must not be two answers - a loader + * and a component that agree only on the cache key is exactly the bug this + * module used to contain half of. + */ + +/** + * Discover is the *browse* feed: no term, newest first. A term search is a + * different request with a different sort, and it is not this module's. + */ +export const DISCOVER_FEED_SORT = 'newest' as const + +/** + * Discover, as the shared feed's own parameters. + * + * One frozen module-level object, read by both halves of the route: the loader + * builds its cache key from it and `` is handed a query built + * from the same reference. Query hashes keys structurally, so an equal object + * would do - but one object makes it impossible for the two to drift. + * + * `search` is absent rather than empty: Discover browses, it does not query. + */ +export const DISCOVER_FEED_PARAMS: SearchFeedParams = Object.freeze({ + sort: DISCOVER_FEED_SORT, +}) diff --git a/apps/web/src/locales/@vitnode/core/pl.json b/apps/web/src/locales/@vitnode/core/pl.json index 513fa2c67..b42564635 100644 --- a/apps/web/src/locales/@vitnode/core/pl.json +++ b/apps/web/src/locales/@vitnode/core/pl.json @@ -6,6 +6,17 @@ "language_switcher": "Zmień język", "save": "Zapisz", "theme_switcher": "Zmień motyw" + }, + "search": { + "discoverTitle": "Odkrywaj", + "discoverDesc": "Zobacz najnowszą aktywność w społeczności.", + "empty": "Nic tu jeszcze nie ma.", + "loadMore": "Wczytaj więcej", + "loading": "Wczytywanie…", + "types": { + "blog_post": "Wpis", + "unknown": "Treść" + } } } } diff --git a/apps/web/src/routeTree.gen.ts b/apps/web/src/routeTree.gen.ts index 7d36ee748..343372be2 100644 --- a/apps/web/src/routeTree.gen.ts +++ b/apps/web/src/routeTree.gen.ts @@ -10,6 +10,7 @@ import { Route as rootRouteImport } from './routes/__root' import { Route as IndexRouteImport } from './routes/index' +import { Route as DiscoverRouteImport } from './routes/discover' import { Route as ApiSplatRouteImport } from './routes/api/$' const IndexRoute = IndexRouteImport.update({ @@ -17,6 +18,11 @@ const IndexRoute = IndexRouteImport.update({ path: '/', getParentRoute: () => rootRouteImport, } as any) +const DiscoverRoute = DiscoverRouteImport.update({ + id: '/discover', + path: '/discover', + getParentRoute: () => rootRouteImport, +} as any) const ApiSplatRoute = ApiSplatRouteImport.update({ id: '/api/$', path: '/api/$', @@ -25,27 +31,31 @@ const ApiSplatRoute = ApiSplatRouteImport.update({ export interface FileRoutesByFullPath { '/': typeof IndexRoute + '/discover': typeof DiscoverRoute '/api/$': typeof ApiSplatRoute } export interface FileRoutesByTo { '/': typeof IndexRoute + '/discover': typeof DiscoverRoute '/api/$': typeof ApiSplatRoute } export interface FileRoutesById { __root__: typeof rootRouteImport '/': typeof IndexRoute + '/discover': typeof DiscoverRoute '/api/$': typeof ApiSplatRoute } export interface FileRouteTypes { fileRoutesByFullPath: FileRoutesByFullPath - fullPaths: '/' | '/api/$' + fullPaths: '/' | '/discover' | '/api/$' fileRoutesByTo: FileRoutesByTo - to: '/' | '/api/$' - id: '__root__' | '/' | '/api/$' + to: '/' | '/discover' | '/api/$' + id: '__root__' | '/' | '/discover' | '/api/$' fileRoutesById: FileRoutesById } export interface RootRouteChildren { IndexRoute: typeof IndexRoute + DiscoverRoute: typeof DiscoverRoute ApiSplatRoute: typeof ApiSplatRoute } @@ -58,6 +68,13 @@ declare module '@tanstack/react-router' { preLoaderRoute: typeof IndexRouteImport parentRoute: typeof rootRouteImport } + '/discover': { + id: '/discover' + path: '/discover' + fullPath: '/discover' + preLoaderRoute: typeof DiscoverRouteImport + parentRoute: typeof rootRouteImport + } '/api/$': { id: '/api/$' path: '/api/$' @@ -70,6 +87,7 @@ declare module '@tanstack/react-router' { const rootRouteChildren: RootRouteChildren = { IndexRoute: IndexRoute, + DiscoverRoute: DiscoverRoute, ApiSplatRoute: ApiSplatRoute, } export const routeTree = rootRouteImport diff --git a/apps/web/src/routes/discover.tsx b/apps/web/src/routes/discover.tsx new file mode 100644 index 000000000..ca8102247 --- /dev/null +++ b/apps/web/src/routes/discover.tsx @@ -0,0 +1,177 @@ +import type { SearchFeedLinkProps } from '@vitnode/core/views/search/search-feed-content' + +import { createFileRoute } from '@tanstack/react-router' +import { HeaderContent } from '@vitnode/core/components/ui/header-content' +import { formatPageTitle } from '@vitnode/core/lib/metadata' +import { SearchFeedContent } from '@vitnode/core/views/search/search-feed-content' +import { createTranslator } from 'use-intl' + +import { MigrationLink } from '#/components/migration-link' +import { RouteMessages } from '#/components/route-messages' +import { useLocale } from '#/lib/i18n/client' +import { intlQueryOptions } from '#/lib/i18n/query' +import { discoverFeedQueryOptions } from '#/lib/search/discover-feed' +import { vitNodeShellConfig } from '#/vitnode.shell.config' + +/** + * Discover, the first VitNode route to render outside Next.js. + * + * One route file serving two public URLs. `/discover` and `/pl/discover` match + * *this* route: the locale is stripped before matching and written back into + * every link the router builds (`rewrite` in `src/router.tsx`), so nothing here + * mentions a language and there is no `/pl/discover.tsx` to keep in step. The + * Next.js route at `packages/vitnode/src/routes/main/discover/page.tsx` is still + * live and unchanged - this is a parallel slice until the cutover. + * + * Everything visible is shared: `HeaderContent` and `SearchFeedContent` are the + * same modules the Next.js app renders, with the two things a shared component + * cannot resolve for itself passed in - the locale, and a `Link`. + */ + +/** + * What this page renders strings from. + * + * `core.global` is the shell's, `core.search` is the feed's - its empty state, + * its "load more", the label on every result type. One list, read by both the + * loader that fetches them and the provider that mounts them, because they have + * to be the same set or the provider suspends on a key nobody warmed. + */ +const DISCOVER_NAMESPACES = ['core.global', 'core.search'] as const + +/** + * The feed's link. + * + * `MigrationLink` rather than the router's `Link` directly, because a search + * result points wherever the indexed content lives and most of VitNode has not + * moved yet. It asks the route tree whether this app can render the + * destination: `/discover` is a client-side navigation, `/blog/post-30` is a + * document load into the Next.js app that still serves it. Either way the + * locale prefix is applied exactly once - by the router's rewrite on one branch + * and by `localizeHref` on the other. + * + * Declared at module scope rather than inline, so it is the same component type + * on every render and React reconciles the feed rather than remounting every + * result. External and unsafe URLs never reach this: `SearchFeedContent` + * classifies those and renders them itself. + */ +const DiscoverFeedLink = ({ + children, + className, + href, +}: SearchFeedLinkProps) => ( + + {children} + +) + +export const Route = createFileRoute('/discover')({ + component: DiscoverRoute, + /** + * Both things this page needs, fetched in parallel before it renders. + * + * `context.locale` comes from the root route's `beforeLoad`, which resolved it + * from the public URL - so `/pl/discover` fetches Polish messages and a Polish + * feed, and the first byte of HTML is already in that language. + * + * Neither call is repeated by the component. The messages are read back by + * `RouteMessages` through the identical `intlQueryOptions`, and the feed by + * `SearchFeedContent` through the key `discoverFeedQueryOptions` warms - the + * key core itself exports for exactly this. A mismatch on either would show up + * as a render that starts empty and fills in a round trip later, which is the + * thing SSR is for. + * + * The strings the metadata needs are returned rather than looked up again: + * `createTranslator` is `use-intl`'s framework-free translator, over the + * messages just fetched. + */ + loader: async ({ context }) => { + const [intl] = await Promise.all([ + context.queryClient.ensureQueryData( + intlQueryOptions({ + locale: context.locale, + namespaces: DISCOVER_NAMESPACES, + }), + ), + context.queryClient.ensureInfiniteQueryData( + discoverFeedQueryOptions({ locale: context.locale }), + ), + ]) + + const t = createTranslator({ + locale: context.locale, + messages: intl.messages, + namespace: 'core.search', + }) + + return { description: t('discoverDesc'), title: t('discoverTitle') } + }, + /** + * The page's metadata, in the language the request resolved to. + * + * **`head` must be written after `loader`.** `loaderData`'s type is inferred + * from `loader` in the same object literal, and TypeScript reads a literal's + * members in order - put `head` first and `loaderData` is `never`, while + * `Route.useLoaderData()` collapses to `undefined`. Neither error names the + * cause. It costs nothing to get right and half an hour to diagnose. + * + * `head` is synchronous here and reads the two strings out of `loaderData`, + * which is the smallest thing that works and the reason it is worth spelling + * out: `head` receives no router context, so it cannot resolve a locale, and + * translating inside it would mean a second lookup that could disagree with + * the `

`. The loader translates once; the tab title and the heading are + * then the same string by construction, which is exactly what the Next.js + * route gets from calling `getTranslations` once per request. + * + * `formatPageTitle` applies the same `" - "` rule Next.js applies + * through `title.template`, so both frameworks produce the same title. + * + * This is deliberately route-local. A general answer - metadata declared once + * and translated for every route - is a pattern this stage does not yet have + * enough migrated routes to design. + */ + head: ({ loaderData }) => ({ + meta: [ + // Indexable, and stated rather than assumed: TanStack Start emits no + // robots directive of its own, and the Next.js route sets + // `robots: { index: true, follow: true }` explicitly. + { content: 'index, follow', name: 'robots' }, + ...(loaderData + ? [ + { + title: formatPageTitle( + vitNodeShellConfig.metadata, + loaderData.title, + ), + }, + { content: loaderData.description, name: 'description' }, + ] + : []), + ], + }), +}) + +function DiscoverRoute() { + const locale = useLocale() + const { description, title } = Route.useLoaderData() + + return ( + +
+ + + {/* + The same options object the loader ensured, so this is a cache read + rather than a fetch: no `initialData` and no Suspense boundary, both of + which would be admissions that the data is not here yet. `fetchNextPage` + then continues from the loader's cursor through the loader's own + request and status checking. + */} + +
+ + ) +} diff --git a/apps/web/src/server/discover-feed.server.ts b/apps/web/src/server/discover-feed.server.ts new file mode 100644 index 000000000..b41b11caf --- /dev/null +++ b/apps/web/src/server/discover-feed.server.ts @@ -0,0 +1,43 @@ +import '@tanstack/react-start/server-only' +import type { + SearchFeedPageArgs, + SearchFeedPageFetcher, +} from '@vitnode/core/views/search/search-feed-query' + +import { + assertSearchFeedResponse, + searchFeedRequest, + searchModuleRef, +} from '@vitnode/core/views/search/search-feed-query' + +import { fetcherServer } from '#/server/fetcher.server' + +/** + * One page of the Discover feed, fetched during SSR. + * + * The request and the response check are core's - the same two the browser + * fetcher uses, so a page fetched here and a page fetched by `fetchNextPage()` + * after hydration are the same request with the same failure semantics. Only + * the *transport* is this module's, and it is the only part that genuinely + * cannot be shared. + * + * `fetcherServer` rather than a bare `fetch`: it resolves the API origin from + * the request being rendered - so a preview deployment calls its own hostname + * rather than a configured one - and forwards the visitor's cookie, user agent + * and `x-forwarded-for`. The feed itself is the same for everyone, but the API + * reads those for the rate-limit bucket and the audit IP, and a render that + * sends none of them puts every visitor in one bucket. + * + * Only ever reached through the isomorphic transport in + * `#/lib/search/discover-feed`, which is what keeps this module - and the + * `server-only` import above - out of the browser bundle. + */ +export const fetchDiscoverFeedPageOnServer: SearchFeedPageFetcher = async ( + args: SearchFeedPageArgs, +) => { + const response = await fetcherServer(searchModuleRef, searchFeedRequest(args)) + + assertSearchFeedResponse(response, args) + + return await response.json() +} diff --git a/apps/web/src/server/messages.server.ts b/apps/web/src/server/messages.server.ts index cabb19dfc..802fc106a 100644 --- a/apps/web/src/server/messages.server.ts +++ b/apps/web/src/server/messages.server.ts @@ -1,5 +1,6 @@ import '@tanstack/react-start/server-only' import type { MessagesSource } from '@vitnode/core/lib/i18n/types' +import type { AbstractIntlMessages } from 'use-intl' import { CONFIG_PLUGIN as CORE } from '@vitnode/core/config' import { loadMessages } from '@vitnode/core/lib/i18n/load-messages' @@ -35,7 +36,18 @@ const sources: MessagesSource[] = [ export interface IntlMessages { locale: string - messages: object + /** + * The picked message tree, as `use-intl`'s own shape rather than a bare + * `object`. + * + * It matters at both ends. `createTranslator` constrains its messages to an + * indexable type, so an `object` there collapses every key it could translate + * to `never` - which is how a route resolves its own metadata strings. And a + * server function's return type has to prove itself serializable, which + * `Record` cannot: `unknown` might be a function. A tree of + * strings can. + */ + messages: AbstractIntlMessages } /** @@ -72,5 +84,11 @@ export const loadIntlMessages = async ({ const merged = await loadMessages({ defaultLocale, locale, sources }) - return { locale, messages: pickMessages(merged, namespaces) } + // `pickMessages` walks an unknown tree and cannot know what it found; what it + // returns is a message tree by construction, every leaf a string from a JSON + // file. Asserted here, once, rather than by every caller. + return { + locale, + messages: pickMessages(merged, namespaces) as AbstractIntlMessages, + } } diff --git a/apps/web/src/tests/discover-route.test.ts b/apps/web/src/tests/discover-route.test.ts new file mode 100644 index 000000000..b78633fc5 --- /dev/null +++ b/apps/web/src/tests/discover-route.test.ts @@ -0,0 +1,630 @@ +import type { AnyRouter } from '@tanstack/react-router' +import type { SearchFeedPage } from '@vitnode/core/views/search/types' + +import { QueryClient } from '@tanstack/react-query' +import { createMemoryHistory } from '@tanstack/react-router' +import { requestHandler } from '@tanstack/react-start/server' +import { searchFeedRequest } from '@vitnode/core/views/search/search-feed-query' +import { Hono } from 'hono' +import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest' + +import { switchLocaleOn } from '#/lib/i18n/client' +import { intlQueryOptions } from '#/lib/i18n/query' +import { + discoverFeedQueryKey, + discoverFeedQueryOptions, +} from '#/lib/search/discover-feed' +import { DISCOVER_FEED_PARAMS } from '#/lib/search/discover-request' +import { getRouter } from '#/router' + +import { PLUGIN_ID } from './api-bridge-contract' +import { renderPage } from './start-runtime/ssr-handler' + +/** + * `/discover`, over a real request, in both languages. + * + * This is Stage 4's whole claim in one file. `renderPage` drives TanStack + * Start's own request handler across the app's real route tree - the middleware + * chain in `src/start.ts`, the locale rewrite, `beforeLoad`, the route's loader, + * the server function that loads the messages, and React rendering to HTML - so + * what is asserted below is what a browser would receive. The one thing stood + * in for is the search API itself, which otherwise needs a database and a + * populated index. + * + * The assertions are grouped by the promise they keep: one route for two URLs, + * a feed already in the first byte of HTML, metadata in the request's language, + * a cache the browser can pick up without re-fetching, and a cursor to continue + * from. + */ + +vi.setConfig({ hookTimeout: 60_000, testTimeout: 60_000 }) + +const ORIGIN = 'https://vitnode.test' +const at = (path: string) => new URL(path, ORIGIN).href +const FEED_PATH = `/api/${PLUGIN_ID}/search` + +interface RecordedRequest { + path: string + query: Record +} + +/** Every search call the stand-in API has been handed, oldest first. */ +const recorded: RecordedRequest[] = [] + +/** Thirty documents: one full page of twenty, then a short second one. */ +const INDEXED_IDS = Array.from({ length: 30 }, (_, index) => 30 - index) + +/** The id the first page ends on, and therefore the cursor page two starts from. */ +const FIRST_PAGE_END_CURSOR = 11 + +/** + * The newest-first walk the search index actually performs, over a synthetic + * index of descending row ids - and the language it was asked for, written into + * every title. + * + * The language in the *content* is the point: a route that fetched the default + * locale's feed and rendered it under a Polish heading would otherwise look + * identical to one that got it right. + */ +const answerFeed = (query: URLSearchParams): SearchFeedPage => { + const first = Number(query.get('first') ?? '10') + const cursor = query.get('cursor') + const lang = query.get('lang') ?? 'en' + const remaining = + cursor === null + ? INDEXED_IDS + : INDEXED_IDS.filter((id) => id < Number(cursor)) + const page = remaining.slice(0, first) + + return { + edges: page.map((id) => ({ + author: null, + authorId: null, + containerId: null, + containerType: null, + content: `Body of ${lang} post ${id}`, + createdAt: new Date(Date.UTC(2026, 0, 1)).toISOString(), + id, + itemId: id, + itemType: 'blog_post', + languageCode: lang, + metadata: {}, + pluginId: PLUGIN_ID, + score: null, + title: `Indexed ${lang} post ${id}`, + url: `/blog/post-${id}`, + })), + pageInfo: { + count: page.length, + endCursor: page.at(-1) ?? null, + hasNextPage: remaining.length > first, + hasPreviousPage: cursor !== null, + startCursor: page.at(0) ?? null, + totalCount: INDEXED_IDS.length, + }, + } +} + +/** The mounted API, stood in for at the path the real search route lives on. */ +const createSearchApi = () => { + const plugin = new Hono() + + plugin.get('/search', (c) => { + const url = new URL(c.req.url) + recorded.push({ + path: url.pathname, + query: Object.fromEntries(url.searchParams.entries()), + }) + + return c.json(answerFeed(url.searchParams)) + }) + + const app = new Hono().basePath('/api') + app.route(`/${PLUGIN_ID}`, plugin) + + return app +} + +const realFetch = globalThis.fetch + +/** One render, with the search calls it made. */ +const renderDiscover = async (path: string) => { + recorded.length = 0 + const page = await renderPage(at(path)) + + return { ...page, requests: [...recorded] } +} + +const langOf = (html: string): string | undefined => + /]*\blang="([^"]*)"/.exec(html)?.[1] + +const h1Of = (html: string): string | undefined => + /]*>([^<]*) + /]*>([^<]*) + new RegExp(` + [...html.matchAll(/href="(\/[^"]*)"/g)].map(([, href]) => href) + +/** + * Runs `handler` inside a request the way the server runtime does, so the + * `getRequest*` helpers `fetcherServer` reads have something to read. + * + * A rejection is carried back out rather than swallowed: `requestHandler` turns + * anything thrown into a 500, so a handler that failed would otherwise look to + * the caller like one that returned nothing. + */ +const withRequest = async (handler: () => Promise): Promise => { + let result!: T + let failure: undefined | { error: unknown } + + await requestHandler(async () => { + try { + result = await handler() + } catch (error) { + failure = { error } + } + + return new Response(null, { status: 204 }) + })(new Request(at('/discover')), {}) + + if (failure) throw failure.error + + return result +} + +/** + * These tests boot the whole application - React, `@vitnode/core`, the plugin + * registry, every message file - on the first render, which under `turbo test` + * can outlast Vitest's default timeout and say nothing about the code. The + * warm-up pays that once. + */ +beforeAll(async () => { + const api = createSearchApi() + globalThis.fetch = async (input: RequestInfo | URL, init?: RequestInit) => + api.fetch(new Request(input, init)) + + await renderPage(at('/discover')) +}) + +afterAll(() => { + globalThis.fetch = realFetch +}) + +describe('one route serves both public URLs', () => { + it('has exactly one discover route in the tree', () => { + // Not `/pl/discover.tsx` beside it: the rewrite strips the prefix before + // matching, so a second file would be a second copy of this page to keep in + // step. If this ever finds two, the locale has leaked into the route tree. + const ids = Object.keys(getRouter().routesById).filter((id) => + id.includes('discover'), + ) + + expect(ids).toEqual(['/discover']) + }) + + it('answers both URLs with the page rather than a 404', async () => { + const [en, pl] = await Promise.all([ + renderPage(at('/discover')), + renderPage(at('/pl/discover')), + ]) + + expect([en.status, pl.status]).toEqual([200, 200]) + }) +}) + +describe('GET /discover renders the English feed on the server', () => { + it('is a 200 in the default language', async () => { + const { html, status } = await renderDiscover('/discover') + + expect(status).toBe(200) + expect(langOf(html)).toBe('en') + }) + + it('renders the English heading and description', async () => { + const { html } = await renderDiscover('/discover') + + expect(h1Of(html)).toBe('Discover') + expect(html).toContain('See the latest activity across the community.') + }) + + it('renders the feed itself, not a placeholder for one', async () => { + const { html } = await renderDiscover('/discover') + + // The whole first page, top and bottom - so a render that streamed in a + // partial page would fail here rather than pass on the first item. + expect(html).toContain('Indexed en post 30') + expect(html).toContain(`Indexed en post ${FIRST_PAGE_END_CURSOR}`) + expect(html).toContain('Body of en post 30') + // The timeline list itself, and neither of the two things "not rendered + // yet" looks like. The empty state is matched as *rendered text* rather + // than as a substring: `core.search.empty` is also one of the messages + // dehydrated into this document, so a bare `toContain` would always fail. + expect(html).toContain('
    ') + expect(html).not.toContain('data-slot="skeleton"') + expect(html).not.toMatch(/>Nothing found yet\. { + const { requests } = await renderDiscover('/discover') + + expect(requests).toHaveLength(1) + expect(requests.at(0)?.path).toBe(FEED_PATH) + expect(requests.at(0)?.query).toStrictEqual({ + first: '20', + lang: 'en', + sort: 'newest', + }) + }) + + it('renders result links unprefixed on the unprefixed URL', async () => { + const { html } = await renderDiscover('/discover') + + expect(hrefsOf(html)).toContain('/blog/post-30') + }) +}) + +describe('GET /pl/discover renders the same route in Polish', () => { + it('is a 200 with a Polish document', async () => { + const { html, status } = await renderDiscover('/pl/discover') + + expect(status).toBe(200) + expect(langOf(html)).toBe('pl') + }) + + it('renders the Polish heading and description', async () => { + const { html } = await renderDiscover('/pl/discover') + + expect(h1Of(html)).toBe('Odkrywaj') + expect(html).toContain('Zobacz najnowszą aktywność w społeczności.') + }) + + it('asks the search API for the Polish feed', async () => { + const { requests } = await renderDiscover('/pl/discover') + + expect(requests).toHaveLength(1) + expect(requests.at(0)?.query.lang).toBe('pl') + }) + + it('renders the Polish feed the API answered', async () => { + const { html } = await renderDiscover('/pl/discover') + + expect(html).toContain('Indexed pl post 30') + expect(html).not.toContain('Indexed en post 30') + }) + + it('translates the feed’s own strings, not just the heading', async () => { + const { html } = await renderDiscover('/pl/discover') + + // `SearchFeedContent` reads `core.search` from the provider this route + // mounts. Getting the heading right while this stayed English would mean the + // namespace never reached the tree. + expect(html).toContain('Wczytaj więcej') + expect(html).not.toContain('Load more') + }) + + it('writes the locale prefix into every result link, exactly once', async () => { + const { html } = await renderDiscover('/pl/discover') + const hrefs = hrefsOf(html) + + expect(hrefs).toContain('/pl/blog/post-30') + // The router owns the prefix. A link that also added one by hand would show + // up here, and nowhere else until somebody clicked it. + expect(hrefs.filter((href) => href.startsWith('/pl/pl/'))).toEqual([]) + }) +}) + +describe('the metadata is the request’s language', () => { + it('titles the English page through the site’s own template', async () => { + const { html } = await renderDiscover('/discover') + + expect(titleOf(html)).toBe('Discover - VitNode') + expect(metaOf(html, 'description')).toBe( + 'See the latest activity across the community.', + ) + }) + + it('titles the Polish page in Polish', async () => { + const { html } = await renderDiscover('/pl/discover') + + expect(titleOf(html)).toBe('Odkrywaj - VitNode') + expect(metaOf(html, 'description')).toBe( + 'Zobacz najnowszą aktywność w społeczności.', + ) + }) + + it('leaves one title in the document, not the shell’s as well', async () => { + const { html } = await renderDiscover('/discover') + + expect(html.match(/ { + const [en, pl] = await Promise.all([ + renderDiscover('/discover'), + renderDiscover('/pl/discover'), + ]) + + expect(metaOf(en.html, 'robots')).toBe('index, follow') + expect(metaOf(pl.html, 'robots')).toBe('index, follow') + }) + + it('says the same thing in the tab title and in the heading', async () => { + // One `createTranslator` call in the loader feeds both, so they cannot drift. + const { html } = await renderDiscover('/pl/discover') + + expect(titleOf(html)).toContain(h1Of(html)) + }) +}) + +/** + * What the browser picks up. + * + * The dehydrated cache is inlined into the document by + * `setupRouterSsrQueryIntegration`, so these read the payload the client will + * hydrate from. The query *hash* is the load-bearing part: it is the entry + * `SearchFeedContent` looks in when it mounts, and if the loader had warmed + * anything else the feed would refetch page one on hydration - an SSR page that + * flickers back to a skeleton in the browser. + */ +describe('the feed crosses to the browser in the cache, not as a refetch', () => { + const feedHash = (locale: string) => + JSON.stringify(['search', { sort: 'newest' }, locale]) + + it('dehydrates the feed under the key the shared component reads', async () => { + const { html } = await renderDiscover('/discover') + + expect(html).toContain(JSON.stringify(feedHash('en')).slice(1, -1)) + }) + + it('hands it over settled, with nothing in flight', async () => { + const { html } = await renderDiscover('/discover') + + // `fetchStatus:"idle"` plus VitNode's `refetchOnMount: false` is the whole + // of "the browser does not fetch this again". + expect(html).toContain('status:"success"') + expect(html).toContain('fetchStatus:"idle"') + }) + + it('dehydrates this route’s messages, not only the shell’s', async () => { + const { html } = await renderDiscover('/pl/discover') + + expect(html).toContain( + JSON.stringify( + JSON.stringify(['vitnode', 'intl', 'pl', 'core.global', 'core.search']), + ).slice(1, -1), + ) + }) + + it('keeps the two languages in separate entries', async () => { + const [en, pl] = await Promise.all([ + renderDiscover('/discover'), + renderDiscover('/pl/discover'), + ]) + const hashOf = (html: string, locale: string) => + html.includes(JSON.stringify(feedHash(locale)).slice(1, -1)) + + expect([hashOf(en.html, 'en'), hashOf(en.html, 'pl')]).toEqual([ + true, + false, + ]) + expect([hashOf(pl.html, 'en'), hashOf(pl.html, 'pl')]).toEqual([ + false, + true, + ]) + }) + + it('renders the whole page from one request, however many components read it', async () => { + // `RouteMessages` and `SearchFeedContent` both read what the loader fetched. + // A second call here would mean one of them missed and fetched for itself. + const { requests } = await renderDiscover('/pl/discover') + + expect(requests).toHaveLength(1) + }) +}) + +describe('the feed can be continued from where SSR left off', () => { + it('offers a next page in the rendered document', async () => { + const { html } = await renderDiscover('/discover') + + // The button only exists while `getNextPageParam` returns something, so this + // is the server agreeing that a cursor survived into the page. + expect(html).toContain('Load more') + expect(html).toContain(`endCursor:${FIRST_PAGE_END_CURSOR}`) + expect(html).toContain('hasNextPage:!0') + }) + + it('builds the second request from that cursor', () => { + const { args } = searchFeedRequest({ + cursor: String(FIRST_PAGE_END_CURSOR), + locale: 'en', + params: DISCOVER_FEED_PARAMS, + }) + + expect(args.query.cursor).toBe(String(FIRST_PAGE_END_CURSOR)) + }) + + it('fetches the second page and then stops', async () => { + recorded.length = 0 + const queryClient = new QueryClient() + + const data = await withRequest(async () => + queryClient.fetchInfiniteQuery({ + ...discoverFeedQueryOptions({ locale: 'en' }), + pages: 3, + }), + ) + + expect(recorded.at(1)?.query.cursor).toBe(String(FIRST_PAGE_END_CURSOR)) + expect(data.pages.flatMap((page) => page.edges)).toHaveLength(30) + // Thirty documents is two pages, so the third fetch never happens - which + // is `getNextPageParam` reading the API's `hasNextPage`, not a page count + // guessed from the total. + expect(recorded).toHaveLength(2) + }) +}) + +/** + * Switching language on the page itself, with no document reload. + * + * Driven at the router level rather than through a browser: `switchLocaleOn` is + * written as a plain function over a router precisely so this is testable, and + * `routerAt` builds the router the way `createStartHandler` does - the real route + * tree, the real rewrite, a memory history seeded with a public URL. + * + * A request scope wraps it because the loaders reach the API through + * `fetcherServer`, which reads the request being handled. In a browser the same + * loaders take the client branch of the isomorphic transport instead. + */ +describe('switching language stays on the page', () => { + const routerAt = (publicHref: string) => { + const router = getRouter() + router.update({ + ...router.options, + history: createMemoryHistory({ initialEntries: [publicHref] }), + }) + + return router + } + + const localeOf = (router: AnyRouter): string => + (router.state.matches.at(0)?.context as { locale?: string }).locale ?? '' + + const clientOf = (router: AnyRouter): QueryClient => + (router.options.context as { queryClient: QueryClient }).queryClient + + /** + * `/discover` in English, switched to Polish, without leaving the page. + * + * `added` is the order cache entries appeared in, which is the only way from + * outside to see *when* the switch warmed what. The message entries error in + * this harness - `createServerFn` needs the Start server context that only + * `createStartHandler` installs, and these tests drive the router directly - + * but an entry is created either way, and the question here is ordering rather + * than content. The SSR suite above is where the messages themselves are read. + */ + const switchedToPolish = async () => { + recorded.length = 0 + + return withRequest(async () => { + const router = routerAt('/discover') + const added: string[] = [] + clientOf(router) + .getQueryCache() + .subscribe((event) => { + if (event.type === 'added') added.push(event.query.queryHash) + }) + + await router.load() + const before = [...recorded] + + await switchLocaleOn(router, 'pl') + + return { added, after: [...recorded], before, router } + }) + } + + it('moves the public URL and leaves the matched route alone', async () => { + const { router } = await switchedToPolish() + + expect(router.state.location.publicHref).toBe('/pl/discover') + // 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') + }) + + it('brings the loader context in step with the new URL', async () => { + const { router } = await switchedToPolish() + + expect(localeOf(router)).toBe('pl') + }) + + it('fetches the feed again, in the new language', async () => { + const { after, before } = await switchedToPolish() + + expect(before.map((request) => request.query.lang)).toEqual(['en']) + // A different language is a different feed over different documents, so a + // switch that reused the English pages would be the bug, not the fetch. + expect(after.map((request) => request.query.lang)).toEqual(['en', 'pl']) + }) + + it('keeps both languages\u2019 feeds in the one client', async () => { + const { router } = await switchedToPolish() + + expect( + clientOf(router).getQueryData(discoverFeedQueryKey('en')), + ).toBeDefined() + expect( + clientOf(router).getQueryData(discoverFeedQueryKey('pl')), + ).toBeDefined() + }) + + it('asks for every set of the new language\u2019s messages', async () => { + const { added } = await switchedToPolish() + + for (const namespaces of [ + ['core.global'], + ['core.global', 'core.search'], + ]) { + expect(added, namespaces.join()).toContain( + JSON.stringify(intlQueryOptions({ locale: 'pl', namespaces }).queryKey), + ) + } + }) + + it('asks for them before the URL moves', async () => { + // The one ordering that matters. The location store updates the moment + // history does, so both providers re-render under the new locale while the + // invalidated loaders are still running - and a `useSuspenseQuery` that + // misses there blanks the page for a round trip. So `switchLocaleOn` warms + // every set the page is holding *first*, this route's included. + // + // The Polish feed is the marker for "after": only the invalidated loader + // fetches it, and that runs once the URL has moved. + const { added } = await switchedToPolish() + const messagesAt = added.indexOf( + JSON.stringify( + intlQueryOptions({ + locale: 'pl', + namespaces: ['core.global', 'core.search'], + }).queryKey, + ), + ) + const feedAt = added.indexOf(JSON.stringify(discoverFeedQueryKey('pl'))) + + expect(messagesAt).toBeGreaterThanOrEqual(0) + expect(feedAt).toBeGreaterThan(messagesAt) + }) + + it('writes the locale prefix into the links it now builds', async () => { + const { router } = await switchedToPolish() + + expect(router.buildLocation({ to: '/discover' }).publicHref).toBe( + '/pl/discover', + ) + }) + + it('switches back to the unprefixed URL', async () => { + recorded.length = 0 + + const router = await withRequest(async () => { + const started = routerAt('/pl/discover') + await started.load() + await switchLocaleOn(started, 'en') + + return started + }) + + expect(router.state.location.publicHref).toBe('/discover') + expect(localeOf(router)).toBe('en') + expect(router.buildLocation({ to: '/discover' }).publicHref).toBe( + '/discover', + ) + }) +}) diff --git a/apps/web/src/tests/env-plugin.test.ts b/apps/web/src/tests/env-plugin.test.ts index 4a6f1a935..1b4479d31 100644 --- a/apps/web/src/tests/env-plugin.test.ts +++ b/apps/web/src/tests/env-plugin.test.ts @@ -86,6 +86,9 @@ describe('vitNodeEnv', () => { it('inlines the API and web URLs into the client bundle', async () => { expect(clientDefine(await runConfig(root))).toStrictEqual({ 'process.env.NEXT_PUBLIC_API_URL': '"https://api.example.test"', + // Temporary migration infrastructure, unset in this fixture. See + // `src/lib/legacy-app.ts`; it goes away with the last legacy route. + 'process.env.NEXT_PUBLIC_LEGACY_WEB_URL': 'undefined', 'process.env.NEXT_PUBLIC_WEB_URL': '"https://web.example.test"', }) }) @@ -120,6 +123,7 @@ describe('vitNodeEnv', () => { // instead of falling through to the default the core config has for it. expect(clientDefine(await runConfig(root))).toStrictEqual({ 'process.env.NEXT_PUBLIC_API_URL': 'undefined', + 'process.env.NEXT_PUBLIC_LEGACY_WEB_URL': 'undefined', 'process.env.NEXT_PUBLIC_WEB_URL': 'undefined', }) }) diff --git a/apps/web/src/tests/intl-query.test.ts b/apps/web/src/tests/intl-query.test.ts index 3eca311af..40420a283 100644 --- a/apps/web/src/tests/intl-query.test.ts +++ b/apps/web/src/tests/intl-query.test.ts @@ -3,7 +3,11 @@ import { describe, expect, it } from 'vitest' import type { Locale } from '#/lib/i18n/shared' -import { GLOBAL_NAMESPACE, intlQueryOptions } from '#/lib/i18n/query' +import { + GLOBAL_NAMESPACE, + intlQueryOptions, + loadedIntlNamespaces, +} from '#/lib/i18n/query' import { loadIntlMessages } from '#/server/messages.server' /** @@ -111,3 +115,68 @@ describe('two languages live in one QueryClient at once', () => { ).toBeUndefined() }) }) + +/** + * Which namespace sets a page is showing, answered by the cache. + * + * The root asks for `core.global`; a route asks for whatever it renders on top + * (`RouteMessages`). Nothing declares the union anywhere, so a language switch - + * which has to warm every set *before* the URL moves, or the second provider + * suspends and the page blanks - reads it back off the entries that exist. + */ +describe('the sets a client is holding', () => { + const clientHolding = ( + entries: { locale: Locale; namespaces?: readonly string[] }[], + ) => { + const queryClient = new QueryClient() + + for (const entry of entries) { + queryClient.setQueryData(intlQueryOptions(entry).queryKey, { + locale: entry.locale, + messages: {}, + }) + } + + return queryClient + } + + it('finds every set one language holds', () => { + const queryClient = clientHolding([ + { locale: 'en' }, + { locale: 'en', namespaces: [GLOBAL_NAMESPACE, 'core.search'] }, + ]) + + expect(loadedIntlNamespaces(queryClient, 'en')).toEqual([ + [GLOBAL_NAMESPACE], + [GLOBAL_NAMESPACE, 'core.search'], + ]) + }) + + it('ignores the other languages’ entries', () => { + const queryClient = clientHolding([ + { locale: 'en', namespaces: [GLOBAL_NAMESPACE, 'core.search'] }, + { locale: 'pl' }, + ]) + + expect(loadedIntlNamespaces(queryClient, 'pl')).toEqual([ + [GLOBAL_NAMESPACE], + ]) + }) + + it('ignores everything that is not a message entry', () => { + const queryClient = clientHolding([{ locale: 'en' }]) + queryClient.setQueryData(['search', { sort: 'newest' }, 'en'], {}) + + expect(loadedIntlNamespaces(queryClient, 'en')).toEqual([ + [GLOBAL_NAMESPACE], + ]) + }) + + it('falls back to the global set on an empty cache', () => { + // A switch made before anything has loaded still has to warm the one set + // every page needs. + expect(loadedIntlNamespaces(new QueryClient(), 'en')).toEqual([ + [GLOBAL_NAMESPACE], + ]) + }) +}) diff --git a/apps/web/src/tests/isolation.test.ts b/apps/web/src/tests/isolation.test.ts index d3d4c6912..0913d4050 100644 --- a/apps/web/src/tests/isolation.test.ts +++ b/apps/web/src/tests/isolation.test.ts @@ -67,7 +67,14 @@ const offendersIn = (files: string[], forbidden: string[]): string[] => /** Anything that only exists inside a TanStack Start app. */ const TANSTACK_ONLY = ['@tanstack/react-start', '@tanstack/react-router'] -/** Anything that only exists inside a Next.js app. */ +/** + * Anything that only exists inside a Next.js app. + * + * `next` covers every subpath by the prefix rule in `matches` - `next/cache`, + * `next/server`, `next/headers`, `next/dynamic`, `next/image`. They are not + * listed one by one on purpose: a list of subpaths is a list somebody has to + * remember to extend, and the package itself is the boundary. + */ const NEXT_ONLY = ['next', 'server-only'] /** @@ -343,12 +350,17 @@ describe('the whole graph this app imports stays Next-free', () => { /** Everything the app reaches, from every entry point it has. */ const ENTRIES = [ 'apps/web/src/components/language-switcher.tsx', + 'apps/web/src/components/route-messages.tsx', 'apps/web/src/lib/i18n/client.ts', 'apps/web/src/lib/i18n/query.ts', 'apps/web/src/lib/i18n/shared.ts', + 'apps/web/src/lib/search/discover-feed.ts', + 'apps/web/src/lib/search/discover-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/server/discover-feed.server.ts', 'apps/web/src/server/locale.server.ts', 'apps/web/src/server/messages.server.ts', 'apps/web/src/start.ts', @@ -389,6 +401,71 @@ describe('the whole graph this app imports stays Next-free', () => { it("reaches none of next-intl's Next-only entries", () => { expect(offenders(ENTRIES, NEXT_INTL_RUNTIME)).toEqual([]) }) + + /** + * `/discover`, on its own. + * + * The first VitNode route to render outside Next.js, and the one whose graph + * is worth stating separately from the app's: it is the route that renders + * *shared* components, so it is the one that would find out - at runtime, in + * production - that a piece of the design system still reaches for Next's + * router or its request scope. It already did once: `HeaderContent` imported + * `@/lib/navigation`, which is `next-intl/navigation` and `next-intl/server`, + * and the back link it needed them for is now a prop. + * + * Every forbidden entry is asserted one at a time rather than as a set, so a + * 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'] + + it('walks into the shared components the route renders', () => { + // Without this the assertions below would pass on a graph that stopped at + // the route file - which is exactly the graph that cannot break. + const reached = [...reachableExternals(DISCOVER).visited] + + expect(reached.some((path) => path.includes('search-feed-content'))).toBe( + true, + ) + expect(reached.some((path) => path.includes('header-content'))).toBe(true) + }) + + it.each([ + 'next', + 'next/cache', + 'next/server', + 'next-intl/navigation', + 'next-intl/server', + 'server-only', + ])('never reaches %s', (forbidden) => { + expect(offenders(DISCOVER, [forbidden])).toEqual([]) + }) + + it('takes its translations from use-intl', () => { + const reached = [...reachableExternals(DISCOVER).externals.keys()] + + expect(reached).toContain('use-intl') + }) + + it("only ever reaches next-intl's framework-free root entry", () => { + // The root entry is `use-intl` re-exported and resolves fine outside + // Next.js - `Button`'s client half still imports it for the loading + // label, and that is allowed by the same rule the app-wide scan uses. + // What must never appear is a subpath: those reach Next's request scope, + // its middleware or its build plugin, and none of them resolves here. + const reached = [...reachableExternals(DISCOVER).externals.keys()] + + expect(reached.filter((one) => one.startsWith('next-intl/'))).toEqual([]) + }) + + it('never reaches a locale-aware navigation module', () => { + // The one that made `HeaderContent` Next-only. `Link` now arrives as a + // prop, from whichever router the app happens to have. + const reached = [...reachableExternals(DISCOVER).externals.keys()] + + expect(reached.filter((one) => one.includes('navigation'))).toEqual([]) + }) + }) }) /** diff --git a/apps/web/src/tests/router-query.test.ts b/apps/web/src/tests/router-query.test.ts index f69a63169..821f61d7d 100644 --- a/apps/web/src/tests/router-query.test.ts +++ b/apps/web/src/tests/router-query.test.ts @@ -109,10 +109,14 @@ describe('the Query SSR integration is installed', () => { describe('nothing but the router creates a query client', () => { const appFiles = [ 'routes/__root.tsx', + 'routes/discover.tsx', 'routes/index.tsx', + 'components/route-messages.tsx', 'lib/i18n/client.ts', 'lib/i18n/query.ts', 'lib/i18n/shared.ts', + 'lib/search/discover-feed.ts', + 'lib/search/discover-request.ts', ] it.each(appFiles)('%s mounts no QueryClientProvider', (file) => { diff --git a/apps/web/vite.config.ts b/apps/web/vite.config.ts index f0dd840a6..e4e3a5631 100644 --- a/apps/web/vite.config.ts +++ b/apps/web/vite.config.ts @@ -9,6 +9,19 @@ import { vitNodeEnv } from './vitnode-env' const config = defineConfig({ resolve: { tsconfigPaths: true }, + /** + * A second dev server has to fail rather than quietly move. + * + * `tanstackStart()` runs the route generator and *writes* + * `src/routeTree.gen.ts`. Two servers means two generators owning one file: if + * their route lists ever differ - which is precisely what happens when one was + * started before a route file existed - they overwrite each other forever, and + * every write is a full page reload. Without `strictPort` the second `pnpm dev` + * says "Port 3001 is in use, trying another one" and succeeds, so the fight + * starts silently and looks like an inexplicable refresh loop on the first + * server. + */ + server: { strictPort: true }, ssr: { /** * The VitNode API packages mounted at `/api/*`, kept out of the SSR pass. diff --git a/apps/web/vitnode-env.ts b/apps/web/vitnode-env.ts index 3b6ffa978..cefc0802f 100644 --- a/apps/web/vitnode-env.ts +++ b/apps/web/vitnode-env.ts @@ -18,7 +18,14 @@ import { loadEnv } from 'vite' * consequence of what somebody happened to call a variable. Add a key to publish * one more. */ -const CLIENT_ENV_KEYS = ['NEXT_PUBLIC_API_URL', 'NEXT_PUBLIC_WEB_URL'] as const +const CLIENT_ENV_KEYS = [ + 'NEXT_PUBLIC_API_URL', + // Temporary, for the length of the migration: the origin serving the routes + // this app does not own yet. `components/migration-link.tsx` reads it in the + // browser, so it has to be inlined like the others. See `src/lib/legacy-app.ts`. + 'NEXT_PUBLIC_LEGACY_WEB_URL', + 'NEXT_PUBLIC_WEB_URL', +] as const /** * Environment handling for a TanStack Start app that serves a VitNode API. diff --git a/packages/vitnode/package.json b/packages/vitnode/package.json index 35c378087..cabf416d5 100644 --- a/packages/vitnode/package.json +++ b/packages/vitnode/package.json @@ -171,6 +171,7 @@ "sharp": "^0.35.3", "tailwind-merge": "^3.6.0", "use-debounce": "^10.1.1", + "use-intl": "^4.13.7", "vaul": "^1.1.2" } } diff --git a/packages/vitnode/src/components/avatar.tsx b/packages/vitnode/src/components/avatar.tsx index 4d0e3dceb..d1aea3db8 100644 --- a/packages/vitnode/src/components/avatar.tsx +++ b/packages/vitnode/src/components/avatar.tsx @@ -1,5 +1,3 @@ -import Image from "next/image"; - import { cn } from "@/lib/utils"; const generateLetterPhoto = (letter: string, color: string) => @@ -7,23 +5,34 @@ const generateLetterPhoto = (letter: string, color: string) => `<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 1024 1024" style="background:#${color}"><g><text text-anchor="middle" dy=".35em" x="512" y="512" fill="#ffffff" font-size="700" font-family="-apple-system, BlinkMacSystemFont, Roboto, Helvetica, Arial, sans-serif">${letter.toLocaleUpperCase()}</text></g></svg>`, )}`; +/** + * A plain `<img>` rather than `next/image`, on purpose. + * + * The `src` is always a `data:` URI built in this file, and Next refuses to + * optimize those - it marks them unoptimized and emits the same tag this does. + * So the import bought nothing, and it cost the whole component tree that + * renders an avatar its portability: the search feed, the user bar and the + * AdminCP tables all become Next-only the moment one of them shows a face. + * + * `loading` and `decoding` are spelled out because `next/image` set them, and a + * data URI is inline anyway - the browser has the bytes before it can defer. + */ export const Avatar = ({ user: { avatarColor, name }, className, size, ...props -}: Omit< - React.ComponentProps<typeof Image>, - "alt" | "height" | "src" | "width" -> & { +}: Omit<React.ComponentProps<"img">, "alt" | "height" | "src" | "width"> & { size: number; user: { avatarColor: string; name: string; nameCode: string }; }) => { return ( - <Image + <img alt={name} className={cn("rounded-full object-cover", className)} + decoding="async" height={size} + loading="lazy" src={generateLetterPhoto(name.slice(0, 1), avatarColor)} width={size} {...props} diff --git a/packages/vitnode/src/components/date-format.tsx b/packages/vitnode/src/components/date-format.tsx index 8e27ad4f0..95675b8e7 100644 --- a/packages/vitnode/src/components/date-format.tsx +++ b/packages/vitnode/src/components/date-format.tsx @@ -1,6 +1,6 @@ "use client"; -import { useFormatter, useNow } from "next-intl"; +import { useFormatter, useNow } from "use-intl"; import { TooltipWithContent } from "./ui/tooltip"; diff --git a/packages/vitnode/src/components/ui/header-content.tsx b/packages/vitnode/src/components/ui/header-content.tsx index 40effe454..b61d821b5 100644 --- a/packages/vitnode/src/components/ui/header-content.tsx +++ b/packages/vitnode/src/components/ui/header-content.tsx @@ -1,16 +1,15 @@ import { ArrowLeftIcon } from "lucide-react"; -import { Link } from "@/lib/navigation"; import { cn } from "@/lib/utils"; import { Button } from "./button"; -interface HeaderContentH1Props extends HeaderContentProps { +interface HeaderContentH1Props { h1: React.ReactNode | string; h2?: never; } -interface HeaderContentH2Props extends HeaderContentProps { +interface HeaderContentH2Props { h1?: never; h2: React.ReactNode | string; } @@ -20,15 +19,63 @@ export interface HeaderContentBack { label: React.ReactNode; } -interface HeaderContentProps { - back?: HeaderContentBack; +/** + * The anchor a back link ends up rendering. + * + * Every prop of one, not just `href`: the button around it is a Base UI + * `render`, which clones the element with the children, the class name and the + * ref it needs to stay a button. A wrapper that accepted only `href` would drop + * all three, so the type says so. + */ +export interface HeaderContentBackLinkProps extends Omit< + React.ComponentProps<"a">, + "href" +> { + href: string; +} + +/** + * The one thing this header cannot decide for itself. + * + * Turning `/admin/blog` 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. Both are a component taking {@link HeaderContentBackLinkProps}, so the + * header takes one and stops caring - and importing neither is what lets a + * TanStack Start route render this component at all. The same boundary + * `SearchFeedContent` draws for a search hit, for the same reason. + * + * It is required alongside `back` rather than defaulting to `<a>`: a missing + * wrapper would otherwise degrade silently into a full document reload. + */ +export type HeaderContentBackLinkComponent = ( + props: HeaderContentBackLinkProps, +) => React.ReactNode; + +/** + * A back link, or neither half of one. + * + * Written as a union so `back` without `BackLink` is a type error at the call + * site. The alternative - two independent optional props - compiles, renders + * nothing, and looks like a header whose back button was never designed. + */ +type HeaderContentBackProps = + | { back: HeaderContentBack; BackLink: HeaderContentBackLinkComponent } + | { back?: never; BackLink?: never }; + +interface HeaderContentBaseProps { children?: React.ReactNode; className?: string; desc?: React.ReactNode; ref?: React.RefCallback<HTMLDivElement>; } +export type HeaderContentProps = HeaderContentBackProps & + HeaderContentBaseProps & + (HeaderContentH1Props | HeaderContentH2Props); + export const HeaderContent = ({ + BackLink, back, children, className, @@ -36,7 +83,7 @@ export const HeaderContent = ({ h1, h2, ref, -}: HeaderContentH1Props | HeaderContentH2Props) => { +}: HeaderContentProps) => { return ( <div className={cn( @@ -60,10 +107,10 @@ export const HeaderContent = ({ {!!back || !!children ? ( <div className="flex w-full flex-col flex-wrap items-center justify-center gap-2 sm:w-auto sm:flex-row [&>*]:w-full [&>*]:sm:w-auto"> - {back ? ( + {back && BackLink ? ( <Button nativeButton={false} - render={<Link href={back.href} />} + render={<BackLink href={back.href} />} variant="outline" > <ArrowLeftIcon /> diff --git a/packages/vitnode/src/lib/i18n/provider.tsx b/packages/vitnode/src/lib/i18n/provider.tsx new file mode 100644 index 000000000..447e40638 --- /dev/null +++ b/packages/vitnode/src/lib/i18n/provider.tsx @@ -0,0 +1,28 @@ +"use client"; + +/** + * `use-intl`'s provider, handed out from inside `@vitnode/core`. + * + * Two lines, and the reason for them is module identity rather than behaviour. + * Every shared component in this package reads its strings through + * `useTranslations` from `use-intl`, which is a React context - and a React + * context belongs to the *module record* it was created in, not to the package + * name. An app that mounts its own `use-intl` provider is only providing into + * core's context while both sides happen to have loaded the same record. + * + * They do not always. In `apps/web`, `@vitnode/core` is external to Vite's SSR + * pass and so is loaded by Node, while the app's own source goes through Vite's + * module runner - two records, two contexts, and every core component that + * translates throws "No intl context found" under `vite dev` while a production + * build (which merges them into one chunk) stays green. + * + * Importing the provider from here removes the coincidence: it is loaded by + * whatever loaded this package, which is by definition the record core's own + * components read. An app that wants to scope messages to a route mounts this + * one - alongside its own, if its own code translates too. + * + * Framework-free on purpose: no `next-intl`, so a TanStack Start route can use + * it. Next.js apps have `I18nProvider` (`@/components/i18n-provider`), which + * reads the request scope and is Next-only by design. + */ +export { IntlProvider } from "use-intl"; diff --git a/packages/vitnode/src/views/admin/views/content/form/primitives.tsx b/packages/vitnode/src/views/admin/views/content/form/primitives.tsx index 26be3b5eb..f8dde22a6 100644 --- a/packages/vitnode/src/views/admin/views/content/form/primitives.tsx +++ b/packages/vitnode/src/views/admin/views/content/form/primitives.tsx @@ -30,6 +30,7 @@ export const ContentFormHeader = ({ return ( <HeaderContent back={header.back} + BackLink={Link} className={className} desc={header.desc} h1={header.title} diff --git a/packages/vitnode/src/views/search/feed-boundaries.test.ts b/packages/vitnode/src/views/search/feed-boundaries.test.ts new file mode 100644 index 000000000..e4a43c160 --- /dev/null +++ b/packages/vitnode/src/views/search/feed-boundaries.test.ts @@ -0,0 +1,222 @@ +// @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, "../.."); + +const SHARED_ENTRY = join(here, "search-feed-content.tsx"); +const NEXT_WRAPPER = join(here, "search-feed.tsx"); + +/** + * The other half of what a migrated feed page renders. + * + * Scanned here rather than in a file of its own because it is the same boundary + * for the same reason: `/discover` is a heading and a feed, and either one + * reaching `next-intl/navigation` makes the whole page Next-only. This one did, + * until the back link became a prop. + */ +const HEADER_CONTENT = join(here, "../../components/ui/header-content.tsx"); + +/** + * The specifier a `from "..."` resolves to, or `null` when it leaves the + * package. + * + * Only `@/` and relative paths are followed. Anything else is a bare package + * name, which is exactly what the assertions below are looking at. + */ +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 feed imports the search + * module's *type* to keep `fetcherClient` typed, and that module is a Hono + * server module. It is erased at compile time and never reaches a bundle, so + * counting it would make this suite fail on something that cannot break. + */ +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 deliberately absent, for the same reason `apps/web`'s + * `isolation.test.ts` leaves it out: it re-exports `use-intl`, which is + * framework-free, and `apps/web` already renders core components that import it. + * These four are the ones that reach for Next's request scope, its middleware or + * its build plugin - and `lib/navigation` is built on two of them. + */ +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 provably imports the things the shared feed must not. + it("finds the Next-only imports in the Next wrapper", () => { + expect(offenders(NEXT_WRAPPER, NEXT_INTL_RUNTIME)).not.toEqual([]); + }); + + it("walks past the entry file into its dependencies", () => { + // `lib/navigation` is two hops from the wrapper, not one. + expect(offenders(NEXT_WRAPPER, ["next-intl/navigation"]).join()).toContain( + "lib/navigation", + ); + }); +}); + +describe("the shared search feed is framework-neutral", () => { + it("reaches nothing from next/* or server-only", () => { + 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("takes its translations from use-intl, not from next-intl", () => { + const imports = runtimeImports(SHARED_ENTRY); + + expect(imports).toContain("use-intl"); + expect(imports).not.toContain("next-intl"); + }); + + it("takes its query as a prop rather than building one", () => { + // Comments stripped first - the file explains at length *why* it no longer + // resolves a locale or builds a request, and prose is not a call site. + const code = readFileSync(SHARED_ENTRY, "utf8") + .replace(/\/\*[\s\S]*?\*\//g, "") + .replace(/\/\/.*$/gm, ""); + + expect(code).not.toContain("useLocale"); + expect(code).toContain("queryOptions: SearchFeedQueryOptions;"); + // One `useInfiniteQuery`, and it is handed its definition. A second + // implementation here is the bug this boundary exists to prevent. + expect(code.match(/useInfiniteQuery\(/g)).toHaveLength(1); + expect(code).toContain("useInfiniteQuery(queryOptions)"); + }); +}); + +describe("the shared header is framework-neutral", () => { + it("reaches nothing from next/* or server-only", () => { + expect(offenders(HEADER_CONTENT, NEXT_ONLY)).toEqual([]); + }); + + it("reaches none of next-intl's Next-only entrypoints", () => { + expect(offenders(HEADER_CONTENT, NEXT_INTL_RUNTIME)).toEqual([]); + }); + + it("never reaches the locale-aware navigation module", () => { + // It used to import it directly, for one back button on one admin screen. + const reached = [...externalGraph(HEADER_CONTENT).keys()]; + + expect(reached.some(one => one.includes("navigation"))).toBe(false); + }); + + it("takes the back link as a prop instead of importing one", () => { + const code = readFileSync(HEADER_CONTENT, "utf8") + .replace(/\/\*[\s\S]*?\*\//g, "") + .replace(/\/\/.*$/gm, ""); + + expect(code).toContain("BackLink"); + expect(code).not.toContain("@/lib/navigation"); + }); +}); + +describe("the Next wrapper keeps the Next-only pieces", () => { + it("is the only one of the two that knows about next-intl navigation", () => { + expect(offenders(NEXT_WRAPPER, ["next-intl/navigation"])).not.toEqual([]); + expect(offenders(SHARED_ENTRY, ["next-intl/navigation"])).toEqual([]); + }); + + it("resolves the locale itself", () => { + expect(readFileSync(NEXT_WRAPPER, "utf8")).toContain("useLocale()"); + }); +}); diff --git a/packages/vitnode/src/views/search/search-feed-content.test.tsx b/packages/vitnode/src/views/search/search-feed-content.test.tsx new file mode 100644 index 000000000..c2b2f3901 --- /dev/null +++ b/packages/vitnode/src/views/search/search-feed-content.test.tsx @@ -0,0 +1,539 @@ +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { fireEvent, render, screen, waitFor } from "@testing-library/react"; +import React from "react"; +import { IntlProvider } from "use-intl"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +import type { + SearchFeedLinkProps, + SearchFeedParams, +} from "./search-feed-content"; +import type { SearchFeedPage, SearchResultItem } from "./types"; + +/** + * The one boundary a render test cannot cross: the feed talks to the search API. + * + * `clientModule` is mocked alongside it because the real one is imported at + * module scope, before any test has run. + */ +const fetcherClient = vi.fn(); + +vi.mock("@/lib/fetcher-client", () => ({ + clientModule: (pluginId: string) => ({ pluginId }), + fetcherClient: (...args: unknown[]) => fetcherClient(...args), +})); + +const { classifySearchFeedHref, SearchFeedContent } = await import( + "./search-feed-content" +); +const { searchFeedQueryKey, searchFeedQueryOptions } = await import( + "./search-feed-query" +); + +const messages = { + core: { + search: { + empty: "Nothing found yet.", + loadMore: "Load more", + loading: "Loading…", + types: { blog_post: "Post", unknown: "Content" }, + }, + }, +}; + +const plMessages = { + core: { + search: { + empty: "Nic nie znaleziono.", + loadMore: "Załaduj więcej", + loading: "Ładowanie…", + types: { blog_post: "Wpis", unknown: "Treść" }, + }, + }, +}; + +/** + * The link the shared feed is handed, standing in for a framework's own. + * + * It records what it was asked to render, which is how the tests below tell an + * internal href (delegated here, and so client-side navigable) from an external + * one (a bare `<a>`). + */ +const routedHrefs: string[] = []; + +const TestLink = ({ children, className, href }: SearchFeedLinkProps) => { + routedHrefs.push(href); + + return ( + <a className={className} data-routed="true" href={href}> + {children} + </a> + ); +}; + +const item = (overrides: Partial<SearchResultItem> = {}): SearchResultItem => ({ + author: { + avatarColor: "ff0000", + id: 1, + name: "Ada", + nameCode: "ada", + }, + authorId: 1, + containerId: null, + containerType: null, + content: "A short body.", + createdAt: "2026-08-01T10:00:00.000Z", + id: 1, + itemId: 1, + itemType: "blog_post", + languageCode: "en", + metadata: {}, + pluginId: "@vitnode/core", + score: null, + title: "First post", + url: "/blog/first-post", + ...overrides, +}); + +const page = ( + edges: SearchResultItem[], + pageInfo: Partial<SearchFeedPage["pageInfo"]> = {}, +): SearchFeedPage => ({ + edges, + pageInfo: { + count: edges.length, + endCursor: null, + hasNextPage: false, + hasPreviousPage: false, + startCursor: null, + totalCount: edges.length, + ...pageInfo, + }, +}); + +let lastQueryClient: QueryClient; + +const renderFeed = ({ + initialData, + locale = "en", + params = { sort: "newest" }, + variant, +}: { + initialData?: SearchFeedPage; + locale?: string; + params?: SearchFeedParams; + variant?: "list" | "timeline"; +} = {}) => { + const queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false } }, + }); + + lastQueryClient = queryClient; + + return render( + <QueryClientProvider client={queryClient}> + <IntlProvider + locale={locale} + messages={locale === "pl" ? plMessages : messages} + timeZone="UTC" + > + <SearchFeedContent + LinkComponent={TestLink} + queryOptions={searchFeedQueryOptions({ initialData, locale, params })} + variant={variant} + /> + </IntlProvider> + </QueryClientProvider>, + ); +}; + +/** The `args.query` the feed sent on the nth call to the API. */ +const queryOfCall = (index: number): Record<string, string> => + ( + fetcherClient.mock.calls[index]?.[1] as { + args: { query: Record<string, string> }; + } + ).args.query; + +/** Every `args.query` the feed has sent, oldest first. */ +const queriesSent = (): Record<string, string>[] => + fetcherClient.mock.calls.map((_, index) => queryOfCall(index)); + +beforeEach(() => { + fetcherClient.mockReset(); + routedHrefs.length = 0; + // jsdom has no IntersectionObserver, and the feed builds one to drive its + // infinite scroll the moment a next page exists. + vi.stubGlobal( + "IntersectionObserver", + class { + disconnect = vi.fn(); + observe = vi.fn(); + unobserve = vi.fn(); + }, + ); +}); + +describe("translations", () => { + it("renders strings from a bare use-intl provider, with no next-intl in the tree", () => { + renderFeed({ initialData: page([]) }); + + expect(screen.getByText("Nothing found yet.")).toBeDefined(); + }); + + it("follows the provider's locale", () => { + renderFeed({ initialData: page([]), locale: "pl" }); + + expect(screen.getByText("Nic nie znaleziono.")).toBeDefined(); + }); +}); + +describe("the locale is explicit", () => { + it("sends the locale it was given as the query's language", async () => { + fetcherClient.mockResolvedValue({ + ok: true, + json: async () => Promise.resolve(page([item()])), + }); + + renderFeed({ locale: "pl" }); + + await screen.findByText("First post"); + expect(queryOfCall(0).lang).toBe("pl"); + }); + + it("refetches under a different locale rather than reusing the cache", async () => { + fetcherClient.mockResolvedValue({ + ok: true, + json: async () => Promise.resolve(page([item()])), + }); + + const { rerender } = renderFeed({ locale: "en" }); + await screen.findByText("First post"); + + const queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false } }, + }); + + rerender( + <QueryClientProvider client={queryClient}> + <IntlProvider locale="pl" messages={plMessages} timeZone="UTC"> + <SearchFeedContent + LinkComponent={TestLink} + queryOptions={searchFeedQueryOptions({ + locale: "pl", + params: { sort: "newest" }, + })} + /> + </IntlProvider> + </QueryClientProvider>, + ); + + await waitFor(() => { + expect(fetcherClient.mock.calls.length).toBe(2); + }); + expect(queryOfCall(1).lang).toBe("pl"); + }); +}); + +describe("the list variant", () => { + it("renders one card per hit, with its type label and author", () => { + renderFeed({ + initialData: page([ + item(), + item({ id: 2, itemId: 2, title: "Second post" }), + ]), + variant: "list", + }); + + expect(screen.getByText("First post")).toBeDefined(); + expect(screen.getByText("Second post")).toBeDefined(); + expect(screen.getAllByText("Post")).toHaveLength(2); + expect(screen.getAllByText("Ada")).toHaveLength(2); + }); + + it("falls back to the generic renderer for an unknown type", () => { + renderFeed({ + initialData: page([item({ itemType: "something_new" })]), + variant: "list", + }); + + expect(screen.getByText("Content")).toBeDefined(); + }); + + it("renders a hit with no url as plain text", () => { + renderFeed({ initialData: page([item({ url: null })]), variant: "list" }); + + expect(screen.getByText("First post").closest("a")).toBeNull(); + }); +}); + +describe("the timeline variant", () => { + it("renders an ordered list, one entry per hit", () => { + const { container } = renderFeed({ + initialData: page([ + item(), + item({ id: 2, itemId: 2, title: "Second post" }), + ]), + variant: "timeline", + }); + + expect(container.querySelectorAll("ol")).toHaveLength(1); + expect(container.querySelectorAll("li")).toHaveLength(2); + }); + + it("wraps the whole entry in a link when the hit has a url", () => { + renderFeed({ initialData: page([item()]), variant: "timeline" }); + + const link = screen.getByText("First post").closest("a"); + + expect(link?.getAttribute("href")).toBe("/blog/first-post"); + expect(link?.textContent).toContain("A short body."); + }); +}); + +describe("links", () => { + it("hands an internal href to the injected component, unchanged", () => { + renderFeed({ + initialData: page([item({ url: "/blog/first-post?page=2" })]), + variant: "list", + }); + + expect(routedHrefs).toEqual(["/blog/first-post?page=2"]); + expect( + screen.getByText("First post").closest("a")?.getAttribute("href"), + ).toBe("/blog/first-post?page=2"); + }); + + it("renders an external href as a plain anchor, never through the router", () => { + renderFeed({ + initialData: page([item({ url: "https://example.com/post" })]), + variant: "list", + }); + + const link = screen.getByText("First post").closest("a"); + + expect(link?.getAttribute("href")).toBe("https://example.com/post"); + expect(link?.dataset.routed).toBeUndefined(); + expect(routedHrefs).toEqual([]); + }); + + it.each(["mailto:hi@example.com", "//cdn.example.com/x"])( + "keeps %s away from the router", + url => { + renderFeed({ initialData: page([item({ url })]), variant: "list" }); + + expect(routedHrefs).toEqual([]); + expect( + screen.getByText("First post").closest("a")?.getAttribute("href"), + ).toBe(url); + }, + ); +}); + +describe("the empty state", () => { + it("replaces the feed entirely when nothing came back", () => { + const { container } = renderFeed({ initialData: page([]) }); + + expect(screen.getByText("Nothing found yet.")).toBeDefined(); + expect(container.querySelectorAll("ol")).toHaveLength(0); + }); + + it("shows no load-more button", () => { + renderFeed({ initialData: page([]) }); + + expect(screen.queryByRole("button")).toBeNull(); + }); +}); + +describe("loading more", () => { + it("shows no button when the API says there is nothing after this page", () => { + renderFeed({ initialData: page([item()]) }); + + expect(screen.queryByText("Load more")).toBeNull(); + }); + + it("offers the button when another page exists", () => { + renderFeed({ + initialData: page([item()], { endCursor: 1, hasNextPage: true }), + }); + + expect(screen.getByText("Load more")).toBeDefined(); + }); + + it("fetches the next page from the cursor and appends it", async () => { + fetcherClient.mockResolvedValue({ + ok: true, + json: async () => + Promise.resolve( + page([item({ id: 2, itemId: 2, title: "Second post" })]), + ), + }); + + renderFeed({ + initialData: page([item()], { endCursor: 7, hasNextPage: true }), + }); + + fireEvent.click(screen.getByText("Load more")); + + expect(await screen.findByText("Second post")).toBeDefined(); + expect(screen.getByText("First post")).toBeDefined(); + // Not `calls[0]`: React Query treats `initialData` as already stale and + // revalidates page one on mount, so the cursor request is not the first. + expect(queriesSent().map(query => query.cursor)).toContain("7"); + // The button goes away with the page that offered it. + expect(screen.queryByText("Load more")).toBeNull(); + }); + + it("observes a sentinel so scrolling loads the next page too", () => { + const observe = vi.fn(); + + vi.stubGlobal( + "IntersectionObserver", + class { + disconnect = vi.fn(); + observe = observe; + unobserve = vi.fn(); + }, + ); + + renderFeed({ + initialData: page([item()], { endCursor: 1, hasNextPage: true }), + }); + + expect(observe).toHaveBeenCalledTimes(1); + }); +}); + +describe("the loading state", () => { + it("renders skeletons until the first page arrives", async () => { + let resolvePage: (value: { + json: () => Promise<SearchFeedPage>; + }) => void = () => undefined; + + fetcherClient.mockReturnValue( + new Promise(resolve => { + resolvePage = resolve; + }), + ); + + const { container } = renderFeed(); + + expect( + container.querySelectorAll('[data-slot="skeleton"]').length, + ).toBeGreaterThan(0); + + resolvePage({ ok: true, json: async () => Promise.resolve(page([item()])) }); + expect(await screen.findByText("First post")).toBeDefined(); + }); +}); + +/** + * The handle a prefetching framework needs. + * + * A TanStack Start route loader warms the cache before this component exists, so + * the key it writes and the key the component reads have to be the same one - + * otherwise the feed mounts, misses, and fetches page one all over again. + */ +describe("the exported query key", () => { + it("is the entry the feed actually stores its pages under", () => { + const params: SearchFeedParams = { sort: "newest" }; + + renderFeed({ initialData: page([item()]), locale: "pl", params }); + + const cached = lastQueryClient.getQueryData( + searchFeedQueryKey({ locale: "pl", params }), + ); + + expect(cached).toBeDefined(); + }); + + it("separates two locales and two parameter sets", () => { + const key = (locale: string, params: SearchFeedParams) => + JSON.stringify(searchFeedQueryKey({ locale, params })); + + expect(key("en", { sort: "newest" })).not.toBe( + key("pl", { sort: "newest" }), + ); + expect(key("en", { sort: "newest" })).not.toBe( + key("en", { sort: "oldest" }), + ); + expect(key("en", { sort: "newest" })).toBe(key("en", { sort: "newest" })); + }); +}); + +/** + * What a search result is allowed to link to. + * + * A document's `url` is written by whichever plugin indexed it, so it is data + * arriving from a database rather than a literal in this repository. The rule + * this replaced treated *any* scheme as "external, render it in an `<a href>`", + * which passed `javascript:` and `data:` through untouched. + */ +describe("the URL scheme allowlist", () => { + const ALLOWED = [ + "http://example.com/post", + "https://example.com/post", + "mailto:test@example.com", + "tel:+123456789", + "//cdn.example.com/x", + ]; + + const REFUSED = [ + "javascript:alert(1)", + "JavaScript:alert(1)", + "data:text/html,<script>alert(1)</script>", + "vbscript:msgbox(1)", + "custom-unknown-scheme:whatever", + // Control characters split the scheme; browsers strip them and follow it. + "java\nscript:alert(1)", + "\u0000javascript:alert(1)", + ]; + + it.each(ALLOWED)("classifies %s as external", url => { + expect(classifySearchFeedHref(url)).toBe("external"); + }); + + it.each(REFUSED)("classifies %s as unsafe", url => { + expect(classifySearchFeedHref(url)).toBe("unsafe"); + }); + + it.each(["/blog/post-1", "/blog/post-1?tab=comments#top", "relative/path"])( + "classifies %s as internal", + url => { + expect(classifySearchFeedHref(url)).toBe("internal"); + }, + ); + + it.each(ALLOWED)("renders %s as a plain anchor", url => { + renderFeed({ initialData: page([item({ url })]), variant: "list" }); + + const link = screen.getByText("First post").closest("a"); + + expect(link?.getAttribute("href")).toBe(url); + // Never through the router: no framework `Link` would accept it anyway. + expect(routedHrefs).toEqual([]); + }); + + it.each(REFUSED)("renders %s with no href at all", url => { + renderFeed({ initialData: page([item({ url })]), variant: "list" }); + + // The result still appears - it is a real document - but there is nothing + // to click, and nothing was handed to the router either. + expect(screen.getByText("First post")).toBeDefined(); + expect(screen.getByText("First post").closest("a")).toBeNull(); + expect(routedHrefs).toEqual([]); + }); + + it("refuses an unsafe href in the timeline variant too", () => { + // The timeline wraps the whole card in the link rather than the title, so + // it is a second call site with its own chance to get this wrong. + const { container } = renderFeed({ + initialData: page([item({ url: "javascript:alert(1)" })]), + variant: "timeline", + }); + + expect(screen.getByText("First post")).toBeDefined(); + expect(container.querySelector("a")).toBeNull(); + expect(routedHrefs).toEqual([]); + }); +}); diff --git a/packages/vitnode/src/views/search/search-feed-content.tsx b/packages/vitnode/src/views/search/search-feed-content.tsx new file mode 100644 index 000000000..ae455ebd2 --- /dev/null +++ b/packages/vitnode/src/views/search/search-feed-content.tsx @@ -0,0 +1,415 @@ +"use client"; + +import { useInfiniteQuery } from "@tanstack/react-query"; +import React from "react"; +import { useTranslations } from "use-intl"; + +import { Avatar } from "@/components/avatar"; +import { DateFormat } from "@/components/date-format"; +import { Button } from "@/components/ui/button"; +import { Card, CardContent } from "@/components/ui/card"; +import { Skeleton } from "@/components/ui/skeleton"; +import { TooltipWithContent } from "@/components/ui/tooltip"; +import { cn } from "@/lib/utils"; + +import type { SearchFeedQueryOptions } from "./search-feed-query"; +import type { SearchResultItem } from "./types"; + +import { getSearchTypeRenderer } from "./registry"; + +const SNIPPET_LENGTH = 240; + +export type SearchFeedVariant = "list" | "timeline"; + +/** + * Re-exported so `search-feed.tsx` and `search-controls.tsx` keep importing + * their parameter type from where they always have. The definition now lives + * with the query it parameterises. + */ +export type { + SearchFeedParams, + SearchFeedQueryOptions, +} from "./search-feed-query"; +export { searchFeedQueryKey } from "./search-feed-query"; + +/** + * Everything the feed ever asks a link to be. + * + * Deliberately three props and no more. A search hit is a title and a + * destination - it never needs prefetch hints, scroll behaviour or an active + * state - so widening this later is a decision somebody has to make on purpose + * rather than one that leaks in. + */ +export interface SearchFeedLinkProps { + children: React.ReactNode; + className?: string; + href: string; +} + +/** + * The one thing this feed cannot decide for itself. + * + * A search result carries an app-internal path, and turning a path 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`, TanStack + * Start wants the router's own. Both are a component taking + * {@link SearchFeedLinkProps}, so the feed takes one and stops caring. + * + * It is a required prop rather than one defaulting to `<a>`: a missing wrapper + * would otherwise degrade silently into a full document reload, which is the + * kind of regression nobody notices until someone measures it. + */ +export type SearchFeedLinkComponent = ( + props: SearchFeedLinkProps, +) => React.ReactNode; + +const getSnippet = (content: string): string => + content.length > SNIPPET_LENGTH + ? `${content.slice(0, SNIPPET_LENGTH).trimEnd()}…` + : content; + +/** + * The schemes a search result is allowed to link to. + * + * An allowlist rather than a denylist, and that direction is the whole point. A + * search document's `url` is written by whichever plugin indexed it, so this is + * data, not code - and the previous rule ("anything with a scheme is external, + * render it in an `<a href>`") happily passed `javascript:` and + * `data:text/html,...` straight through. React 19 blocks `javascript:` at + * render, but that is React's backstop, not this component's policy, and it + * covers neither `data:` nor whatever the next scheme turns out to be. + * + * `mailto:` and `tel:` are here because a plugin indexing a contact record has + * a real reason to emit them. Anything outside this set is refused. + */ +const SAFE_EXTERNAL_SCHEMES: ReadonlySet<string> = new Set([ + "http:", + "https:", + "mailto:", + "tel:", +]); + +/** + * Control characters and spaces removed, because they hide a scheme: + * `java\nscript:` and `\u0000javascript:` are both followed by a browser. + * + * Written as a code-point filter rather than a regular expression: a character + * class containing a literal NUL is what `no-control-regex` exists to catch, + * and the intent - "drop anything at or below a space" - reads better this way. + */ +const withoutControlCharacters = (value: string): string => + [...value].filter(char => (char.codePointAt(0) ?? 0) > 0x20).join(""); + +export type SearchFeedHrefKind = "external" | "internal" | "unsafe"; + +/** + * What kind of destination an indexed `url` is. + * + * - `internal` - a path this app routes. The framework's `LinkComponent` gets it. + * - `external` - an allowlisted scheme, or a protocol-relative `//host/path` + * (kept because it is existing behaviour). A bare `<a>` gets it; no router + * would accept it anyway. + * - `unsafe` - everything else. Nothing gets it: {@link ResultLink} renders the + * title as text. A result that cannot be linked to safely is still a result. + * + * Exported for the tests, which is the only way to state the policy directly + * rather than through rendered markup. + */ +export const classifySearchFeedHref = (href: string): SearchFeedHrefKind => { + const cleaned = withoutControlCharacters(href); + + // Protocol-relative. Checked before the scheme test, which would not match it. + if (cleaned.startsWith("//")) return "external"; + // A path, absolute or relative - no scheme to vet. + if (!/^[a-z][a-z\d+\-.]*:/i.test(cleaned)) return "internal"; + + const scheme = cleaned.slice(0, cleaned.indexOf(":") + 1).toLowerCase(); + + return SAFE_EXTERNAL_SCHEMES.has(scheme) ? "external" : "unsafe"; +}; + +/** + * A result's destination, rendered by whatever is allowed to render it. + * + * An `unsafe` href falls back to the children with no anchor at all, so a + * hostile document degrades to plain text instead of to a link nobody should + * click - and, just as importantly, is never handed to a router either. + */ +const ResultLink = ({ + LinkComponent, + children, + className, + href, +}: SearchFeedLinkProps & { LinkComponent: SearchFeedLinkComponent }) => { + const kind = classifySearchFeedHref(href); + + if (kind === "unsafe") return <span className={className}>{children}</span>; + + if (kind === "external") { + return ( + <a className={className} href={href}> + {children} + </a> + ); + } + + return ( + <LinkComponent className={className} href={href}> + {children} + </LinkComponent> + ); +}; + +const ItemTitle = ({ + LinkComponent, + item, +}: { + item: SearchResultItem; + LinkComponent: SearchFeedLinkComponent; +}) => + item.url ? ( + <ResultLink + className="hover:underline" + href={item.url} + LinkComponent={LinkComponent} + > + {item.title} + </ResultLink> + ) : ( + <>{item.title}</> + ); + +const TimelineItem = ({ + LinkComponent, + item, + isLast, +}: { + isLast: boolean; + item: SearchResultItem; + LinkComponent: SearchFeedLinkComponent; +}) => { + const t = useTranslations("core.search"); + const renderer = getSearchTypeRenderer(item.itemType); + const Icon = renderer.icon; + const snippet = getSnippet(item.content); + + const content = ( + <> + <div className="text-muted-foreground flex items-center gap-2 text-sm"> + {item.author && ( + <> + <Avatar size={20} user={item.author} /> + <span className="text-foreground font-medium"> + {item.author.name} + </span> + <span aria-hidden>·</span> + </> + )} + <DateFormat date={item.createdAt} /> + </div> + <h3 className="text-xl leading-tight font-bold">{item.title}</h3> + {snippet && <p className="text-muted-foreground">{snippet}</p>} + </> + ); + + return ( + <li className="flex gap-4"> + <div className="flex flex-col items-center gap-2"> + <TooltipWithContent text={t(renderer.labelKey)}> + <span className="bg-muted/50 text-foreground flex size-10 shrink-0 items-center justify-center rounded-lg border shadow-sm"> + <Icon className="size-4" /> + </span> + </TooltipWithContent> + {!isLast && <span className="bg-border w-px grow" />} + </div> + + <div className={cn("min-w-0 flex-1", !isLast && "pb-8")}> + {item.url ? ( + <ResultLink + className="hover:bg-muted/50 flex flex-col gap-1 rounded-lg border p-4 transition-colors" + href={item.url} + LinkComponent={LinkComponent} + > + {content} + </ResultLink> + ) : ( + <div className="flex flex-col gap-1 rounded-lg border p-4"> + {content} + </div> + )} + </div> + </li> + ); +}; + +const SearchResultCard = ({ + LinkComponent, + item, +}: { + item: SearchResultItem; + LinkComponent: SearchFeedLinkComponent; +}) => { + const t = useTranslations("core.search"); + const renderer = getSearchTypeRenderer(item.itemType); + const Icon = renderer.icon; + const snippet = getSnippet(item.content); + + return ( + <Card> + <CardContent className="flex gap-3"> + {item.author ? ( + <Avatar size={40} user={item.author} /> + ) : ( + <span className="bg-muted text-muted-foreground flex size-10 items-center justify-center rounded-full"> + <Icon className="size-5" /> + </span> + )} + + <div className="flex min-w-0 flex-col gap-1"> + <div className="text-muted-foreground flex flex-wrap items-center gap-x-2 text-sm"> + <span className="bg-muted inline-flex items-center gap-1 rounded px-1.5 py-0.5"> + <Icon className="size-3.5" /> + {t(renderer.labelKey)} + </span> + {item.author && ( + <span className="text-foreground font-medium"> + {item.author.name} + </span> + )} + <DateFormat date={item.createdAt} /> + </div> + + <h3 className="text-foreground truncate text-lg font-semibold"> + <ItemTitle item={item} LinkComponent={LinkComponent} /> + </h3> + + {snippet && ( + <p className="text-muted-foreground line-clamp-2 text-sm"> + {snippet} + </p> + )} + </div> + </CardContent> + </Card> + ); +}; + +/** + * The search feed, with nothing framework-shaped left in it. + * + * This is the whole of the rendering and paging behaviour - infinite scroll, the + * load-more fallback, both variants, the empty and loading states - and it runs + * unchanged under Next.js and under TanStack Start. Exactly two things are + * pulled out, and they are the only two that ever needed to be: + * + * - **`queryOptions`**, built by `searchFeedQueryOptions`. There is one + * `useInfiniteQuery` in this file and it is handed its definition, so the page + * a route loader prefetched and the page `fetchNextPage()` asks for come from + * the same request, the same cursor rule and the same status checking. This + * component used to build its own, which agreed with a loader on the cache key + * and on nothing else - a 400 on page two was parsed as a page and the feed + * quietly emptied itself. The locale and the search parameters left with it, + * because both are things the *query* needs rather than the markup. + * - **`LinkComponent`**. See {@link SearchFeedLinkComponent}. + * + * Translations come from `use-intl` directly - the framework-free half of + * `next-intl`, and the same instance `NextIntlClientProvider` provides into, so + * the Next.js app needs no extra provider for this to work. + */ +export const SearchFeedContent = ({ + LinkComponent, + queryOptions, + variant = "list", +}: { + LinkComponent: SearchFeedLinkComponent; + queryOptions: SearchFeedQueryOptions; + variant?: SearchFeedVariant; +}) => { + const t = useTranslations("core.search"); + const sentinelRef = React.useRef<HTMLDivElement>(null); + + const { data, fetchNextPage, hasNextPage, isFetchingNextPage, isLoading } = + useInfiniteQuery(queryOptions); + + React.useEffect(() => { + const el = sentinelRef.current; + if (!el || !hasNextPage) return; + + const observer = new IntersectionObserver(entries => { + if (entries[0]?.isIntersecting && !isFetchingNextPage) { + void fetchNextPage(); + } + }); + observer.observe(el); + + return () => observer.disconnect(); + }, [hasNextPage, isFetchingNextPage, fetchNextPage]); + + const items = data?.pages.flatMap(page => page.edges) ?? []; + + if (isLoading) { + return ( + <div className="flex flex-col gap-3"> + {["a", "b", "c"].map(id => ( + <Skeleton className="h-24 w-full rounded-xl" key={id} /> + ))} + </div> + ); + } + + if (items.length === 0) { + return ( + <p className="text-muted-foreground py-10 text-center">{t("empty")}</p> + ); + } + + const loadMore = ( + <> + <div ref={sentinelRef} /> + + {hasNextPage && ( + <Button + className="mx-auto" + disabled={isFetchingNextPage} + onClick={() => void fetchNextPage()} + variant="outline" + > + {isFetchingNextPage ? t("loading") : t("loadMore")} + </Button> + )} + </> + ); + + if (variant === "timeline") { + return ( + <div className="flex flex-col gap-4"> + <ol className="flex flex-col"> + {items.map((item, index) => ( + <TimelineItem + isLast={index === items.length - 1} + item={item} + key={`${item.itemType}-${item.itemId}`} + LinkComponent={LinkComponent} + /> + ))} + </ol> + + {loadMore} + </div> + ); + } + + return ( + <div className="flex flex-col gap-3"> + {items.map(item => ( + <SearchResultCard + item={item} + key={`${item.itemType}-${item.itemId}`} + LinkComponent={LinkComponent} + /> + ))} + + {loadMore} + </div> + ); +}; diff --git a/packages/vitnode/src/views/search/search-feed-query.ts b/packages/vitnode/src/views/search/search-feed-query.ts new file mode 100644 index 000000000..322b8f0db --- /dev/null +++ b/packages/vitnode/src/views/search/search-feed-query.ts @@ -0,0 +1,264 @@ +import { infiniteQueryOptions } from "@tanstack/react-query"; + +import type { searchModule } from "@/api/modules/search/search.module"; + +import { CONFIG_PLUGIN } from "@/config"; +import { clientModule, fetcherClient } from "@/lib/fetcher-client"; + +import type { SearchFeedPage } from "./types"; + +/** + * The search feed, as one query definition. + * + * Everything about *fetching* a feed lives here and nowhere else: the request, + * the page size, the cursor rule, what counts as a failure, and the cache entry + * it all lands in. `SearchFeedContent` renders whatever this produces and owns + * none of it. + * + * That split exists because the alternative was tried and does not hold. When + * the component built its own `useInfiniteQuery` and a TanStack Start loader + * built another, the two agreed on the cache *key* and on nothing else - so the + * server-rendered first page came from one contract and every `fetchNextPage()` + * after hydration came from a second one with a different cursor rule and no + * status checking. A 400 arrived as `{ message }`, was read as a page, and the + * feed silently rendered as empty. Sharing a key is not sharing a contract. + * + * The one thing deliberately *not* fixed here is the transport: a loader running + * on a server and a component running in a browser cannot reach the API the same + * way. So {@link searchFeedQueryOptions} takes a `fetchPage` and defaults it to + * the browser's, which is the only one a shared component can assume. + */ + +/** How many hits one page holds, wherever that page is fetched from. */ +export const SEARCH_FEED_PAGE_SIZE = 20; + +/** + * Where a page starts. `null` is the first one, spelled as the *absence* of a + * cursor rather than an empty one: the route's schema rejects `cursor=` + * outright (`.min(1)`), so sending it empty would 400 the first page of every + * visit. + */ +export type SearchFeedCursor = null | string; + +/** The first page carries no cursor. Named, because a test has to say so too. */ +export const SEARCH_FEED_FIRST_PAGE: SearchFeedCursor = null; + +export interface SearchFeedParams { + authorId?: string; + from?: string; + search?: string; + sort?: "newest" | "oldest" | "relevance"; + to?: string; + types?: string; +} + +/** + * The search module, as a value the fetchers can carry without pulling the API + * into either bundle. The module is imported as a *type* only, so route + * literals, methods and response schemas all still infer; `clientModule` + * supplies the one field the fetcher reads at runtime. + */ +export const searchModuleRef = clientModule<typeof searchModule>( + CONFIG_PLUGIN.pluginId, +); + +export interface SearchFeedPageArgs { + cursor: SearchFeedCursor; + /** + * The language the page is rendered in. Required rather than defaulted: a + * feed that quietly falls back to the default locale is a Polish page full of + * English posts, and nothing about the response says so. + */ + locale: string; + params: SearchFeedParams; +} + +/** + * One page of a feed, as arguments to whichever fetcher is carrying it. + * + * `first` is a string because the query schema reads it off a query string, and + * every optional key is omitted rather than set to `undefined` so it never + * reaches the URL at all. + */ +export const searchFeedRequest = ({ + cursor, + locale, + params, +}: SearchFeedPageArgs) => { + const query: Record<string, string> = { + first: String(SEARCH_FEED_PAGE_SIZE), + lang: locale, + }; + + if (params.search) query.search = params.search; + if (params.types) query.types = params.types; + if (params.authorId) query.authorId = params.authorId; + if (params.sort) query.sort = params.sort; + if (params.from) query.from = params.from; + if (params.to) query.to = params.to; + if (cursor !== null) query.cursor = cursor; + + return { + args: { query }, + method: "get" as const, + module: "search" as const, + path: "/" as const, + }; +}; + +/** + * Refuses a response that is not a search page. + * + * The fetchers hand non-2xx responses back rather than throwing on them - a + * rejected cursor is a 400, a rate-limited visitor a 429 - and `json()` would + * happily parse either one's `{ message }` body. Read as a page it has no + * `edges`, so the feed renders as empty: a failure that looks exactly like a + * community with nothing in it. Query can only retry, report, or keep the last + * good page if the promise actually rejects. + * + * A 500 never reaches here; `rawApiFetch` throws on those with the body + * attached. A 429 does, *after* `fetcherClient` has already raised the + * global rate-limit notice - so the visitor is told, and the query still fails + * rather than appending an error object as a page. + * + * Takes a plain `Response` so the caller keeps its typed one: passing the typed + * response in widens `ok` to a boolean and leaves `json()` alone. + */ +export const assertSearchFeedResponse = ( + response: Response, + { cursor, locale }: SearchFeedPageArgs, +): void => { + if (response.ok) return; + + throw new Error( + `The search API answered ${response.status} for the feed (locale "${locale}", cursor ${cursor ?? "none"}).`, + ); +}; + +/** + * Where the next page starts, or nothing when this was the last one. + * + * Two conditions rather than one. `hasNextPage` is the API's answer and is + * authoritative, but the newest-first walk cursors by row id, so an `endCursor` + * of `null` means there is no row to continue from - asking anyway would send + * `cursor=null` as a literal string and replay page one forever. Returning + * `undefined` is what tells Query the feed has ended, which is what turns the + * "load more" button off. + */ +export const nextSearchFeedCursor = ( + page: SearchFeedPage, +): SearchFeedCursor | undefined => { + const { endCursor, hasNextPage } = page.pageInfo; + + if (!hasNextPage || endCursor === null) return undefined; + + return String(endCursor); +}; + +/** + * The cache entry one feed reads and writes. + * + * `params` before `locale` is the order it has always been in; changing it + * would silently orphan every entry a running client already holds. The locale + * is *in* the key, and that is the whole contract: `/discover` and + * `/pl/discover` are two feeds over two sets of documents, so they get two + * entries. + * + * An object in a key is safe - Query hashes keys structurally rather than by + * identity - but only while it holds the same values, so a caller with fixed + * parameters should keep one module-level object rather than a literal per + * render. + */ +export const searchFeedQueryKey = ({ + locale, + params, +}: { + locale: string; + params: SearchFeedParams; +}) => ["search", params, locale] as const; + +/** How a page is actually fetched. See {@link searchFeedQueryOptions}. */ +export type SearchFeedPageFetcher = ( + args: SearchFeedPageArgs, +) => Promise<SearchFeedPage>; + +/** + * One page, fetched from the browser. + * + * `fetcherClient` builds the same `/api/@vitnode/core/search` URL every other + * VitNode client call uses - same-origin, cookies attached by the browser + * itself, and a 429 routed to the global rate-limit notice. + */ +export const fetchSearchFeedPageInBrowser: SearchFeedPageFetcher = + async args => { + const response = await fetcherClient( + searchModuleRef, + searchFeedRequest(args), + ); + + assertSearchFeedResponse(response, args); + + return await response.json(); + }; + +/** + * The feed, as the one query definition every caller shares. + * + * A route loader warms it before the component renders: + * + * context.queryClient.ensureInfiniteQueryData(searchFeedQueryOptions({...})) + * + * and the component reads the very same options back: + * + * <SearchFeedContent queryOptions={searchFeedQueryOptions({...})} /> + * + * Same key, same page function, same cursor rule - so the loader's page is the + * page the component renders, `fetchNextPage` continues from it under the same + * status checking, and no route implements paging a second time. + * + * `fetchPage` is the seam. It defaults to the browser's fetcher, which is what + * a Next.js client component wants and what a hydrated TanStack page wants too; + * an app that also fetches during SSR passes one that can do both. It is a + * plain async function rather than anything framework-shaped, so nothing about + * this module knows which framework is rendering it. + * + * `initialData` is for a server that already has page one in hand and no cache + * to put it in - Next.js renders the feed from a Server Component and passes it + * down. A framework that hydrates a real Query cache must **not** use it: the + * page is already in the entry this key names, and passing it again is a second + * copy of the same bytes that can disagree with the first. + * + * No `staleTime`. Freshness is whatever the API's own caching gives, plus + * VitNode's client defaults (`refetchOnMount` and `refetchOnWindowFocus` both + * off), so a hydrated feed is not refetched behind the reader. + */ +export const searchFeedQueryOptions = ({ + fetchPage = fetchSearchFeedPageInBrowser, + initialData, + locale, + params, +}: { + fetchPage?: SearchFeedPageFetcher; + initialData?: SearchFeedPage; + locale: string; + params: SearchFeedParams; +}) => + infiniteQueryOptions({ + getNextPageParam: nextSearchFeedCursor, + initialData: initialData + ? { pageParams: [SEARCH_FEED_FIRST_PAGE], pages: [initialData] } + : undefined, + initialPageParam: SEARCH_FEED_FIRST_PAGE, + queryFn: async ({ pageParam }) => + await fetchPage({ cursor: pageParam, locale, params }), + queryKey: searchFeedQueryKey({ locale, params }), + }); + +/** + * What {@link SearchFeedContent} accepts, and the reason it accepts only this. + * + * Typed as the factory's own return type on purpose: a caller cannot hand the + * feed a hand-rolled options object that happens to type-check, so "one query + * definition" is enforced by the compiler rather than by review. + */ +export type SearchFeedQueryOptions = ReturnType<typeof searchFeedQueryOptions>; diff --git a/packages/vitnode/src/views/search/search-feed.test.tsx b/packages/vitnode/src/views/search/search-feed.test.tsx new file mode 100644 index 000000000..b65cd5adf --- /dev/null +++ b/packages/vitnode/src/views/search/search-feed.test.tsx @@ -0,0 +1,183 @@ +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { render, screen } from "@testing-library/react"; +import { + NextIntlClientProvider, + IntlProvider as NextIntlIntlProvider, +} from "next-intl"; +import React from "react"; +import { IntlProvider } from "use-intl"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +import type { SearchFeedPage, SearchResultItem } from "./types"; + +const fetcherClient = vi.fn(); + +vi.mock("@/lib/fetcher-client", () => ({ + clientModule: (pluginId: string) => ({ pluginId }), + fetcherClient: (...args: unknown[]) => fetcherClient(...args), +})); + +/** + * `next-intl`'s `Link` stands in for itself here. + * + * `lib/navigation` is built on `next-intl/navigation`, which imports + * `next/navigation` - a bare CJS file that Next only ever resolves through its + * own bundler, so loading it under Vite fails outright. Mocking the module keeps + * this suite about the wrapper's job (resolve the locale, hand the shared feed a + * link) rather than about Next's router. + * + * The stand-in honours the same contract the real one does for an internal + * href: render an anchor at that path. That `lib/navigation` is what the wrapper + * actually reaches for is asserted in `feed-boundaries.test.ts`, and the real + * locale prefixing is covered by the docs app's end-to-end suite. + */ +vi.mock("@/lib/navigation", () => ({ + Link: ({ + children, + className, + href, + }: { + children: React.ReactNode; + className?: string; + href: string; + }) => ( + <a className={className} data-next-intl-link="true" href={href}> + {children} + </a> + ), +})); + +const { SearchFeed } = await import("./search-feed"); + +const messages = { + core: { + search: { + empty: "Nothing found yet.", + loadMore: "Load more", + loading: "Loading…", + types: { blog_post: "Post", unknown: "Content" }, + }, + }, +}; + +const hit: SearchResultItem = { + author: null, + authorId: null, + containerId: null, + containerType: null, + content: "A short body.", + createdAt: "2026-08-01T10:00:00.000Z", + id: 1, + itemId: 1, + itemType: "blog_post", + languageCode: "en", + metadata: {}, + pluginId: "@vitnode/core", + score: null, + title: "First post", + url: "/blog/first-post", +}; + +const page = (edges: SearchResultItem[]): SearchFeedPage => ({ + edges, + pageInfo: { + count: edges.length, + endCursor: null, + hasNextPage: false, + hasPreviousPage: false, + startCursor: null, + totalCount: edges.length, + }, +}); + +const renderWrapper = (locale: string, initialData?: SearchFeedPage) => + render( + <QueryClientProvider + client={ + new QueryClient({ defaultOptions: { queries: { retry: false } } }) + } + > + {/* Deliberately the *Next* provider, and only it - see the suite above. */} + <NextIntlClientProvider + locale={locale} + messages={messages} + timeZone="UTC" + > + <SearchFeed + initialData={initialData} + params={{ sort: "newest" }} + variant="timeline" + /> + </NextIntlClientProvider> + </QueryClientProvider>, + ); + +beforeEach(() => { + fetcherClient.mockReset(); + vi.stubGlobal( + "IntersectionObserver", + class { + disconnect = vi.fn(); + observe = vi.fn(); + unobserve = vi.fn(); + }, + ); +}); + +/** + * The assumption the whole split rests on. + * + * `@vitnode/core` now imports `use-intl` directly while the Next.js app still + * provides through `next-intl`. That only works because the two are the same + * module - `next-intl` re-exports `use-intl/react`'s provider verbatim - so they + * share one React context. Should a version bump ever give `@vitnode/core` its + * own copy of `use-intl`, every core component that translates would throw + * "No intl context found" in the Next.js app, at runtime rather than at build + * time. This is the cheap early warning. + */ +describe("core's use-intl is the same instance next-intl provides into", () => { + it("resolves to one provider component, not two", () => { + expect(IntlProvider).toBe(NextIntlIntlProvider); + }); +}); + +describe("the Next wrapper", () => { + it("renders the shared feed under next-intl's provider alone", () => { + renderWrapper("en", page([hit])); + + expect(screen.getByText("First post")).toBeDefined(); + expect(screen.getByText("A short body.")).toBeDefined(); + }); + + it("translates through the Next provider", () => { + renderWrapper("en", page([])); + + expect(screen.getByText("Nothing found yet.")).toBeDefined(); + }); + + it("passes the locale next-intl resolved into the search query", async () => { + fetcherClient.mockResolvedValue({ + ok: true, + json: async () => Promise.resolve(page([hit])), + }); + + renderWrapper("pl"); + + await screen.findByText("First post"); + + const { args } = fetcherClient.mock.calls[0]?.[1] as { + args: { query: Record<string, string> }; + }; + + expect(args.query.lang).toBe("pl"); + }); + + it("renders result links through next-intl's locale-aware Link", () => { + renderWrapper("en", page([hit])); + + const link = screen.getByText("First post").closest("a"); + + expect(link?.dataset.nextIntlLink).toBe("true"); + expect(link?.getAttribute("href")).toBe("/blog/first-post"); + }); +}); diff --git a/packages/vitnode/src/views/search/search-feed.tsx b/packages/vitnode/src/views/search/search-feed.tsx index 03a87f5c3..5756dfb42 100644 --- a/packages/vitnode/src/views/search/search-feed.tsx +++ b/packages/vitnode/src/views/search/search-feed.tsx @@ -1,284 +1,78 @@ "use client"; -import { useInfiniteQuery } from "@tanstack/react-query"; -import { useLocale, useTranslations } from "next-intl"; -import React from "react"; +import { useLocale } from "next-intl"; -import type { searchModule } from "@/api/modules/search/search.module"; - -import { Avatar } from "@/components/avatar"; -import { DateFormat } from "@/components/date-format"; -import { Button } from "@/components/ui/button"; -import { Card, CardContent } from "@/components/ui/card"; -import { Skeleton } from "@/components/ui/skeleton"; -import { TooltipWithContent } from "@/components/ui/tooltip"; -import { CONFIG_PLUGIN } from "@/config"; -import { clientModule, fetcherClient } from "@/lib/fetcher-client"; import { Link } from "@/lib/navigation"; -import { cn } from "@/lib/utils"; - -import type { SearchFeedPage, SearchResultItem } from "./types"; - -import { getSearchTypeRenderer } from "./registry"; - -const searchRef = clientModule<typeof searchModule>(CONFIG_PLUGIN.pluginId); -const SNIPPET_LENGTH = 240; - -export type SearchFeedVariant = "list" | "timeline"; - -export interface SearchFeedParams { - authorId?: string; - from?: string; - search?: string; - sort?: "newest" | "oldest" | "relevance"; - to?: string; - types?: string; -} - -const getSnippet = (content: string): string => - content.length > SNIPPET_LENGTH - ? `${content.slice(0, SNIPPET_LENGTH).trimEnd()}…` - : content; - -const ItemTitle = ({ item }: { item: SearchResultItem }) => - item.url ? ( - <Link className="hover:underline" href={item.url}> - {item.title} - </Link> - ) : ( - <>{item.title}</> - ); - -const TimelineItem = ({ - item, - isLast, -}: { - isLast: boolean; - item: SearchResultItem; -}) => { - const t = useTranslations("core.search"); - const renderer = getSearchTypeRenderer(item.itemType); - const Icon = renderer.icon; - const snippet = getSnippet(item.content); - - const content = ( - <> - <div className="text-muted-foreground flex items-center gap-2 text-sm"> - {item.author && ( - <> - <Avatar size={20} user={item.author} /> - <span className="text-foreground font-medium"> - {item.author.name} - </span> - <span aria-hidden>·</span> - </> - )} - <DateFormat date={item.createdAt} /> - </div> - <h3 className="text-xl leading-tight font-bold">{item.title}</h3> - {snippet && <p className="text-muted-foreground">{snippet}</p>} - </> - ); - - return ( - <li className="flex gap-4"> - <div className="flex flex-col items-center gap-2"> - <TooltipWithContent text={t(renderer.labelKey)}> - <span className="bg-muted/50 text-foreground flex size-10 shrink-0 items-center justify-center rounded-lg border shadow-sm"> - <Icon className="size-4" /> - </span> - </TooltipWithContent> - {!isLast && <span className="bg-border w-px grow" />} - </div> - - <div className={cn("min-w-0 flex-1", !isLast && "pb-8")}> - {item.url ? ( - <Link - className="hover:bg-muted/50 flex flex-col gap-1 rounded-lg border p-4 transition-colors" - href={item.url} - > - {content} - </Link> - ) : ( - <div className="flex flex-col gap-1 rounded-lg border p-4"> - {content} - </div> - )} - </div> - </li> - ); -}; - -const SearchResultCard = ({ item }: { item: SearchResultItem }) => { - const t = useTranslations("core.search"); - const renderer = getSearchTypeRenderer(item.itemType); - const Icon = renderer.icon; - const snippet = getSnippet(item.content); - - return ( - <Card> - <CardContent className="flex gap-3"> - {item.author ? ( - <Avatar size={40} user={item.author} /> - ) : ( - <span className="bg-muted text-muted-foreground flex size-10 items-center justify-center rounded-full"> - <Icon className="size-5" /> - </span> - )} - - <div className="flex min-w-0 flex-col gap-1"> - <div className="text-muted-foreground flex flex-wrap items-center gap-x-2 text-sm"> - <span className="bg-muted inline-flex items-center gap-1 rounded px-1.5 py-0.5"> - <Icon className="size-3.5" /> - {t(renderer.labelKey)} - </span> - {item.author && ( - <span className="text-foreground font-medium"> - {item.author.name} - </span> - )} - <DateFormat date={item.createdAt} /> - </div> - - <h3 className="text-foreground truncate text-lg font-semibold"> - <ItemTitle item={item} /> - </h3> - - {snippet && ( - <p className="text-muted-foreground line-clamp-2 text-sm"> - {snippet} - </p> - )} - </div> - </CardContent> - </Card> - ); -}; - -const buildQuery = ( - params: SearchFeedParams, - cursor: string, -): Record<string, string> => { - const query: Record<string, string> = { first: "20" }; - if (params.search) query.search = params.search; - if (params.types) query.types = params.types; - if (params.authorId) query.authorId = params.authorId; - if (params.sort) query.sort = params.sort; - if (params.from) query.from = params.from; - if (params.to) query.to = params.to; - if (cursor) query.cursor = cursor; - - return query; -}; +import type { + SearchFeedLinkProps, + SearchFeedVariant, +} from "./search-feed-content"; +import type { SearchFeedParams } from "./search-feed-query"; +import type { SearchFeedPage } from "./types"; + +import { SearchFeedContent } from "./search-feed-content"; +import { searchFeedQueryOptions } from "./search-feed-query"; + +export type { SearchFeedParams, SearchFeedVariant }; + +/** + * The feed's link, the Next.js way: `next-intl`'s locale-aware `Link`. + * + * Declared at module scope rather than inline, so it is the same component type + * on every render and React reconciles rather than remounting each result. + */ +const NextSearchFeedLink = ({ + children, + className, + href, +}: SearchFeedLinkProps) => ( + <Link className={className} href={href}> + {children} + </Link> +); + +/** + * {@link SearchFeedContent}, wired to Next.js. + * + * Everything the feed does lives in the shared component; this supplies the + * three things that cannot be shared, and the props are unchanged, so + * `SearchControls`, `DiscoverView` and the AdminCP user timeline see exactly the + * component they always did. + * + * - **The locale**, which `next-intl` reads from Next's request scope. + * - **A `Link`** that knows how to write a locale prefix into an internal href. + * - **The query**, built here from `searchFeedQueryOptions` - the same factory + * a TanStack Start route loader uses, with the same request, cursor rule and + * status checking. Only the transport differs, and this app takes the default: + * the browser's fetcher, which is the right one for a client component. + * + * `initialData` stays supported because Next.js has nowhere else to put a page + * it already fetched: `DiscoverView` and `SearchView` render the first page in a + * Server Component and hand it down, with no Query cache to hydrate from. An app + * that *does* hydrate one must not use it - see `searchFeedQueryOptions`. + * + * The options object is rebuilt on every render, deliberately. `SearchControls` + * derives `params` from component state as the visitor types, so memoising on it + * would be memoising on a value that changes anyway - and Query hashes the key + * structurally, so an equal object is the same cache entry. + */ export const SearchFeed = ({ - params, initialData, + params, variant = "list", }: { initialData?: SearchFeedPage; params: SearchFeedParams; variant?: SearchFeedVariant; }) => { - const t = useTranslations("core.search"); const locale = useLocale(); - const sentinelRef = React.useRef<HTMLDivElement>(null); - - const { data, fetchNextPage, hasNextPage, isFetchingNextPage, isLoading } = - useInfiniteQuery({ - queryKey: ["search", params, locale], - initialPageParam: "", - queryFn: async ({ pageParam }) => { - const res = await fetcherClient(searchRef, { - module: "search", - path: "/", - method: "get", - args: { query: { ...buildQuery(params, pageParam), lang: locale } }, - }); - - return await res.json(); - }, - getNextPageParam: last => - last.pageInfo.hasNextPage ? String(last.pageInfo.endCursor) : undefined, - initialData: initialData - ? { pages: [initialData], pageParams: [""] } - : undefined, - }); - - React.useEffect(() => { - const el = sentinelRef.current; - if (!el || !hasNextPage) return; - - const observer = new IntersectionObserver(entries => { - if (entries[0]?.isIntersecting && !isFetchingNextPage) { - void fetchNextPage(); - } - }); - observer.observe(el); - - return () => observer.disconnect(); - }, [hasNextPage, isFetchingNextPage, fetchNextPage]); - - const items = data?.pages.flatMap(page => page.edges) ?? []; - - if (isLoading) { - return ( - <div className="flex flex-col gap-3"> - {["a", "b", "c"].map(id => ( - <Skeleton className="h-24 w-full rounded-xl" key={id} /> - ))} - </div> - ); - } - - if (items.length === 0) { - return ( - <p className="text-muted-foreground py-10 text-center">{t("empty")}</p> - ); - } - - const loadMore = ( - <> - <div ref={sentinelRef} /> - - {hasNextPage && ( - <Button - className="mx-auto" - disabled={isFetchingNextPage} - onClick={() => void fetchNextPage()} - variant="outline" - > - {isFetchingNextPage ? t("loading") : t("loadMore")} - </Button> - )} - </> - ); - - if (variant === "timeline") { - return ( - <div className="flex flex-col gap-4"> - <ol className="flex flex-col"> - {items.map((item, index) => ( - <TimelineItem - isLast={index === items.length - 1} - item={item} - key={`${item.itemType}-${item.itemId}`} - /> - ))} - </ol> - - {loadMore} - </div> - ); - } return ( - <div className="flex flex-col gap-3"> - {items.map(item => ( - <SearchResultCard item={item} key={`${item.itemType}-${item.itemId}`} /> - ))} - - {loadMore} - </div> + <SearchFeedContent + LinkComponent={NextSearchFeedLink} + queryOptions={searchFeedQueryOptions({ initialData, locale, params })} + variant={variant} + /> ); }; diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index d7d16bcf2..fd6ff12ab 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -359,6 +359,12 @@ importers: '@tanstack/router-cli': specifier: ^1.132.0 version: 1.167.33 + '@testing-library/dom': + specifier: ^10.4.1 + version: 10.4.1 + '@testing-library/react': + specifier: ^16.3.2 + version: 16.3.2(@testing-library/dom@10.4.1)(@types/react-dom@19.2.3(@types/react@19.2.17))(@types/react@19.2.17)(react-dom@19.2.8(react@19.2.8))(react@19.2.8) '@types/node': specifier: ^22.10.2 version: 22.20.1 @@ -377,6 +383,9 @@ importers: eslint: specifier: ^10.7.0 version: 10.7.0(jiti@2.7.0) + jsdom: + specifier: ^29.1.1 + version: 29.1.1 tw-animate-css: specifier: ^1.4.0 version: 1.4.0 @@ -754,6 +763,9 @@ importers: use-debounce: specifier: ^10.1.1 version: 10.1.1(react@19.2.8) + use-intl: + specifier: ^4.13.7 + version: 4.13.7(react@19.2.8) vaul: specifier: ^1.1.2 version: 1.1.2(@types/react-dom@19.2.3(@types/react@19.2.17))(@types/react@19.2.17)(react-dom@19.2.8(react@19.2.8))(react@19.2.8)