diff --git a/apps/docs/content/docs/dev/search.mdx b/apps/docs/content/docs/dev/search.mdx index e75da632f..3e48f9df4 100644 --- a/apps/docs/content/docs/dev/search.mdx +++ b/apps/docs/content/docs/dev/search.mdx @@ -29,10 +29,11 @@ config change followed by a rebuild. ## Indexing content - A content type only needs a - [`search` block](/docs/dev/content-engine/public-api-and-caching#2-full-text-search-indexing) - publishing, editing, - unpublishing and deleting a record then keep its document in step - automatically, and it joins the rebuild without any of the wiring below. + A content type only needs a [`search` + block](/docs/dev/content-engine/public-api-and-caching#2-full-text-search-indexing) + - publishing, editing, unpublishing and deleting a record then keep its + document in step automatically, and it joins the rebuild without any of the + wiring below. Any API handler can (re)index or remove an item through `c.get("search")`. It is @@ -93,8 +94,8 @@ agnostic** and match every locale, so single-language plugins need no changes. Postgres full-text ranking picks a text-search configuration per locale - (`polish` for `pl`, `german` for `de`, and so on), falling back to `simple` for - a locale with no bundled dictionary - and for a document with no + (`polish` for `pl`, `german` for `de`, and so on), falling back to `simple` + for a locale with no bundled dictionary - and for a document with no `languageCode`, which matches every locale. Matching works across languages either way; only stemming and stop-words differ. @@ -146,8 +147,8 @@ documents (one per language) or none at all (a row that cannot be projected). Returning `documents.length` as `itemsRead`, or ending the loop on an empty `documents` array, silently truncates the index: a page whose rows all fail to - project would stop the rebuild before the valid rows behind it. Report the rows - you read. + project would stop the rebuild before the valid rows behind it. Report the + rows you read. ### The older array result @@ -230,18 +231,18 @@ A collection is **unmanaged by the rebuild system** when documents for its `itemType` are in the index but no `SearchIndexer` is registered for it. - It does **not** prove the plugin is uninstalled or inactive. A live-only plugin - looks exactly the same from the index's point of view, and may be keeping the - collection completely up to date. All VitNode can tell is that it has no way to - rebuild it. + It does **not** prove the plugin is uninstalled or inactive. A live-only + plugin looks exactly the same from the index's point of view, and may be + keeping the collection completely up to date. All VitNode can tell is that it + has no way to rebuild it. -**AdminCP → Advanced → Search** labels those rows *Unmanaged*, shows the plugin +**AdminCP → Advanced → Search** labels those rows _Unmanaged_, shows the plugin stored on their documents, and says no rebuild indexer is registered. Coverage is left blank rather than calculated: with no indexer there is no source count, and `11 / 11` would claim a collection nothing can rebuild is fully covered. -*Reindex* is replaced by **Remove documents**, behind a confirmation. It deletes +_Reindex_ is replaced by **Remove documents**, behind a confirmation. It deletes what is currently indexed and rebuilds nothing - but it does not stop anything either, so a live-writing plugin may recreate those documents on its next write. It is a way to clear a stale indexed state, not a way to uninstall a collection. @@ -255,6 +256,44 @@ Result cards look up an icon and label by `itemType`. Add an entry to the render registry (`@vitnode/core/views/search/registry`); unknown types fall back to a generic renderer, so nothing breaks if an entry is missing. +## Rendering search in your own app + +The search UI ships in three framework-neutral pieces, so a Next.js page and a +TanStack Start route render the same components: + +| Module | What it is | +| ---------------------------------------------------- | ------------------------------------------------------------------------------------------------ | +| `@vitnode/core/views/search/search-params` | Pure functions: normalise a term from a URL, pick the default sort, build the feed's parameters. | +| `@vitnode/core/views/search/search-feed-query` | The feed as one query definition - request, page size, cursor rule, response check, cache key. | +| `@vitnode/core/views/search/search-controls-content` | The search box, type filters, sort and results, ready to mount. | + +Two things are injected, because they are the only two a shared component cannot +answer for itself: how a page is fetched, and how an internal link becomes a +navigation. + +```tsx title="A search page, in any framework" +import { SearchControlsContent } from "@vitnode/core/views/search/search-controls-content"; +import { searchFeedQueryOptions } from "@vitnode/core/views/search/search-feed-query"; +import { searchFeedParamsFor } from "@vitnode/core/views/search/search-params"; + + searchFeedQueryOptions({ locale, params })} + LinkComponent={MyLink} + variant="timeline" +/>; +``` + + + Warm the *same* `searchFeedQueryOptions` in your loader + (`ensureInfiniteQueryData`) and the first page is already in the cache when + the component mounts - no `initialData`, no second copy of the same bytes. + + +Only the term belongs in the URL. The sort and the type filters are controls the +visitor drives after the page loads, so they stay component state - and a +malformed `?search=` normalises to the browse feed rather than breaking the page. + ## Choosing the engine The engine is set in `vitnode.api.config.ts`, exactly like the storage and email diff --git a/apps/docs/content/docs/ui/data-table.mdx b/apps/docs/content/docs/ui/data-table.mdx index effd19ed9..8f4231a48 100644 --- a/apps/docs/content/docs/ui/data-table.mdx +++ b/apps/docs/content/docs/ui/data-table.mdx @@ -500,6 +500,76 @@ That pruning is also what makes a partly-successful action readable: revalidate so a run that partly succeeded can say so. +## URL State + +Every control on the table is really a URL editor. Sorting writes `?orderBy=` and `?order=`, paging writes `?first=`/`?last=` and `?cursor=`, the search box writes `?search=`, and each filter writes its own parameter. Nothing is kept in React state, which is why a table link can be bookmarked, shared, or opened in a new tab and show the same rows. + +The rules those controls follow live in one framework-free module, so you can reuse them - or test them - without a router: + +```ts +import { + readTableOrder, + readTablePageSize, + readTableSearch, + toggleTableOrder, + withTableFilter, + withTableOrder, + withTablePage, + withTablePageSize, + withTableSearch, +} from "@vitnode/core/components/table/url-state"; + +withTableSearch("page=3&tab=media", "vitnode"); +// → "page=3&tab=media&search=vitnode" +``` + +Each `with*` helper takes the current query string (or a `URLSearchParams`) and returns a new one. They never mutate what you hand them, they always keep parameters they don't own - your own `?tab=` survives a sort click - and they remove a parameter rather than leaving it empty. Filtering and changing the page size drop the pagination cursor, because the rows underneath it changed; sorting and searching leave it alone. + +### Using the table outside Next.js + +`DataTable` is the Next.js binding: it supplies the current search parameters and a locale-aware, scroll-free push, and every page in this documentation uses it. Under it sits the same table with that one decision taken as an argument, which is all another router needs to render it. + +Give `DataTableNavigationProvider` where you are and how to move, and render `ContentDataTable` inside it: + +```tsx +import { ContentDataTable } from "@vitnode/core/components/table/content"; +import { DataTableNavigationProvider } from "@vitnode/core/components/table/navigation"; + + router.navigate({ search: nextSearch }), + searchParams: new URLSearchParams(location.searchStr), + }} +> + +; +``` + + Promise | void", + }, + searchParams: { + description: + "The query string the table is currently rendering. Never mutated.", + required: true, + type: "URLSearchParams", + }, + }} +/> + +The types, and the `DataTableSkeleton` you render as a loading fallback, come from `@vitnode/core/components/table/data-table-content` - importing them from `data-table` would pull Next.js in behind them. + ## Complete Example Here's a complete example showing how to use the `DataTable` component in a page: diff --git a/apps/web/src/lib/files/my-files-route.ts b/apps/web/src/lib/files/my-files-route.ts new file mode 100644 index 000000000..95c497b0a --- /dev/null +++ b/apps/web/src/lib/files/my-files-route.ts @@ -0,0 +1,229 @@ +import type { + MyFilesOrder, + MyFilesOrderBy, + MyFilesParams, + RawMyFilesParams, +} from '@vitnode/core/views/files/my-files-query' + +import { DEFAULT_TABLE_PAGE_SIZE } from '@vitnode/core/components/table/url-state' +import { normalizeMyFilesParams } from '@vitnode/core/views/files/my-files-query' + +/** + * What `/files` reads out of its URL, and the three things it turns that into. + * + * Four pure functions, no transport and no React, so the route's contract can be + * stated and tested without a router - `src/tests/my-files-route.test.ts` is the + * whole of it. The same split `/search` already uses + * (`lib/search/search-request.ts`), applied to a table instead of a feed. + * + * Every one of them delegates the *meaning* of a parameter to + * `@vitnode/core/views/files/my-files-query`, which is the module the Next.js + * `MyFilesTableView` reads its `searchParams` through. So `/files?orderBy=name` + * is the same request in both applications rather than two hand-written + * approximations of it, and nothing here re-states which columns are sortable or + * how large a page may be. + * + * ## Three shapes, and why they are not one + * + * the URL ?orderBy=name&first=20 what a visitor sees and shares + * the search { orderBy: 'name', first: 20 } the route's validated state + * the request { first: '20', orderBy: 'name' } what the API is asked for + * + * The middle one is the URL, validated. The last one is core's `MyFilesParams`, + * which additionally *always* names a page size, because a request must - and a + * URL need not. Keeping them apart is what stops `?first=10` being written into + * every link to a page whose canonical address is `/files`. + * + * ## All four are total and idempotent + * + * None of them can throw and none of them reject: a URL typed by hand renders + * the table it would have rendered anyway. That is not politeness, it is a + * requirement of where they run - `validateSearch` throwing turns a hand-edited + * query string into a router error screen, and this page's query string is edited + * by hand every time somebody shares a sorted link. + * + * Idempotent because they are applied twice on every navigation: once when a + * table control's new query string is turned back into route search, and once + * more by the router when it validates the location that produces. A rule that + * moved the value on the second pass would make the table drift a step per click. + */ + +/** + * The page size the URL does not need to mention. + * + * `DEFAULT_TABLE_PAGE_SIZE` is what every `DataTable` falls back to when the URL + * asks for no size, so `?first=10` and no `first` at all are the same request + * spelled two ways - and the shorter spelling is the one this route settles on. + */ +const DEFAULT_PAGE_SIZE = String(DEFAULT_TABLE_PAGE_SIZE) + +/** + * The route's validated search - the URL contract, and nothing else. + * + * Exactly the six parameters `DataTable`'s controls write: the sort header emits + * `orderBy`/`order`, the search box `search`, and the pager `first`/`last` with + * a `cursor`. There is no seventh, because this table declares no filters. + * + * `first` and `last` are numbers rather than strings, and that is about the + * address bar rather than about types. TanStack Router's default search + * serializer JSON-encodes a *string* that would parse as JSON, so the string + * `'20'` is written to the URL as `first=%2220%22`; the number `20` is written + * as `first=20`, which is what the Next.js page produces and what the API reads. + */ +export interface MyFilesRouteSearch { + cursor?: string + first?: number + last?: number + order?: MyFilesOrder + orderBy?: MyFilesOrderBy + search?: string +} + +/** + * A search as it arrives, before anything has checked it. + * + * Two shapes, because there are two callers and they are genuinely different. + * The router hands over its *parsed* search - an arbitrary bag of whatever was + * in the query string - and this route hands its own validated search straight + * back in, on every navigation and in the idempotence assertions. An `interface` + * has no implicit index signature, so the second is not assignable to the first + * and the union has to say so. + */ +export type UncheckedMyFilesSearch = + MyFilesRouteSearch | Record + +/** + * One search parameter as the string it was in the query string. + * + * The router hands `validateSearch` its *parsed* search, and the default parser + * JSON-parses every value - so `?first=20` arrives as the number `20`, `?x=true` + * as a boolean, and a repeated key as an array. Core's normaliser is written + * against a query string, where everything is a string, and one of its rules + * (`search.trim()`) throws on anything else. + * + * So this is the seam between the two, and it is deliberately narrow: scalars + * become their string spelling, the first entry of an array wins because only one + * value can reach the API, and everything else - an object, a nested array, a + * `null` - is *absent* rather than coerced. `String({})` is `"[object Object]"`, + * which is a value no rule below would recognise but every rule would have to + * consider. + */ +const readParam = (value: unknown): string | undefined => { + const one = Array.isArray(value) ? (value[0] as unknown) : value + + if (typeof one === 'string') return one + if (typeof one === 'number') + return Number.isFinite(one) ? String(one) : undefined + if (typeof one === 'boolean') return String(one) + + return undefined +} + +/** + * The six parameters this route has, in the shape core's normaliser reads. + * + * Named one by one rather than passed through, which is the whole of rule 3: + * nothing a visitor puts in the query string reaches the request builder unless + * this route asked for it. A stray `?tab=2` is not carried, not validated, and + * not sent. + */ +const rawParamsOf = (input: UncheckedMyFilesSearch): RawMyFilesParams => ({ + cursor: readParam(input.cursor), + first: readParam(input.first), + last: readParam(input.last), + order: readParam(input.order), + orderBy: readParam(input.orderBy), + search: readParam(input.search), +}) + +/** + * The request this URL is asking for - core's `MyFilesParams`, and therefore also + * the object the query key is built from. + * + * Every defaulting and clamping rule is `normalizeMyFilesParams`': an unusable + * page size falls back rather than 400ing, `first` beats `last`, a sort column + * the list cannot sort by is dropped so the API applies its own `createdAt desc`, + * a blank search is no search, and a cursor that cannot be one is not sent. + * + * Takes the loose object rather than {@link MyFilesRouteSearch} on purpose. The + * router merges a route's validated search over the *raw* parsed one, so + * `Route.useSearch()` still carries whatever else was in the query string; going + * back through the same normalisation is what makes this answer depend only on + * the six parameters above, whoever is calling it. + */ +export const myFilesRouteParams = ( + input: UncheckedMyFilesSearch, +): MyFilesParams => normalizeMyFilesParams(rawParamsOf(input)) + +/** + * The route's search schema - written as a function rather than a schema object + * because its job is to *normalise*, not to reject. + * + * `/files` is a page whose query string is edited by hand and pasted between + * people: `?orderBy=password`, `?first=5000`, `?first=abc`, `?cursor=💥`. Every + * one of them should render the visitor's files sorted the way the table + * defaults to, not a router error - so an unusable value becomes an absent one, + * and the API's own `createdAt desc` is what an unrecognised `orderBy` falls back + * to. + * + * The one thing it does *not* keep is a page size equal to the default. `/files` + * and `/files?first=10` are the same page, and a schema that answered `first: 10` + * for the first of them would write `?first=10` into every link the router builds + * to this route - including the one `MigrationLink` renders and the one a guest's + * `?returnTo=` comes back through. + */ +export const normalizeMyFilesRouteSearch = ( + input: UncheckedMyFilesSearch, +): MyFilesRouteSearch => { + const { cursor, first, last, order, orderBy, search } = + myFilesRouteParams(input) + + return { + ...(cursor === undefined ? {} : { cursor }), + // See above: the default page size is the URL saying nothing. + ...(first === undefined || first === DEFAULT_PAGE_SIZE + ? {} + : { first: Number(first) }), + // `last` is never dropped: paging *backwards* at the default size is a + // different request from not paging at all, and the parameter is what says so. + ...(last === undefined ? {} : { last: Number(last) }), + ...(order === undefined ? {} : { order }), + ...(orderBy === undefined ? {} : { orderBy }), + ...(search === undefined ? {} : { search }), + } +} + +/** + * The query string the table's controls read themselves out of. + * + * `DataTable`'s sort headers, pager and search box are handed a + * `URLSearchParams` and produce a new query string from it + * (`components/table/url-state.ts`); this is the other end of that, and it is + * built from the validated search rather than from the address bar so a control + * can only ever edit a parameter this route recognises. + */ +export const myFilesSearchParams = ( + input: UncheckedMyFilesSearch, +): URLSearchParams => { + const params = new URLSearchParams() + + for (const [key, value] of Object.entries( + normalizeMyFilesRouteSearch(input), + )) { + params.set(key, String(value)) + } + + return params +} + +/** + * A query string one of those controls produced, back as route search. + * + * The return leg, and the point at which the table's own URL arithmetic is + * re-validated: a control cannot write a sort column this route does not have, + * because what it wrote goes through the same schema the address bar does. + */ +export const myFilesSearchFrom = (nextSearch: string): MyFilesRouteSearch => + normalizeMyFilesRouteSearch( + Object.fromEntries(new URLSearchParams(nextSearch)), + ) diff --git a/apps/web/src/lib/files/my-files.ts b/apps/web/src/lib/files/my-files.ts new file mode 100644 index 000000000..3be846ea3 --- /dev/null +++ b/apps/web/src/lib/files/my-files.ts @@ -0,0 +1,172 @@ +import type { QueryClient } from '@tanstack/react-query' +import type { + BulkDeleteFilesResult, + DeleteFileResult, + DeleteMyFile, + DeleteMyFileArgs, + DeleteMyFiles, + DeleteMyFilesArgs, +} from '@vitnode/core/views/files/my-files-delete' +import type { + MyFilesPageFetcher, + MyFilesParams, +} from '@vitnode/core/views/files/my-files-query' + +import { useQueryClient } from '@tanstack/react-query' +import { createIsomorphicFn } from '@tanstack/react-start' +import { + deleteMyFileInBrowser, + deleteMyFilesInBrowser, + shouldRefreshAfterBulkDelete, +} from '@vitnode/core/views/files/my-files-delete' +import { + fetchMyFilesPageInBrowser, + MY_FILES_QUERY_ROOT, + myFilesQueryOptions, +} from '@vitnode/core/views/files/my-files-query' +import React from 'react' + +import { fetchMyFilesPageOnServer } from '#/server/my-files.server' + +/** + * The visitor's own files, as this app's one query definition and two deletes. + * + * Everything about *what* the list is - the request, the defaults, the cache + * key, what counts as a refusal - comes from + * `@vitnode/core/views/files/my-files-query`, which is also what the mounted + * `MyFilesTableContent` is rendered from. This module supplies only the two + * things core cannot know: how to reach the API from a server that is rendering + * a request, and what "refresh the table" means in a router that has a query + * cache instead of `revalidatePath`. + */ + +/** + * 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 sort, page and search + * of the table would cost two round trips for a read the API is already the + * boundary for. The session read next door *is* a server function, and the + * difference is real rather than stylistic: nothing here needs a `Set-Cookie` + * copied onto this app's own response. + * + * The cookie still travels on both branches. On the server `fetcherServer` + * forwards the one the page request arrived with; in the browser the call is + * same-origin, so the browser attaches it without being asked. That is what + * makes a `401` here mean "the session ended", never "we forgot to say who was + * asking". + * + * `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 `my-files.server.ts` - and the + * `server-only` marker at the top of it - never reaches the browser. + */ +const fetchMyFilesPage: MyFilesPageFetcher = createIsomorphicFn() + .server(fetchMyFilesPageOnServer) + .client(fetchMyFilesPageInBrowser) + +/** + * The files table, as the one query definition every caller shares. + * + * loader: context.queryClient.ensureQueryData(myFilesQuery({ params })) + * component: useQuery(myFilesQuery({ params })) + * after a delete: invalidate, and the component above refetches + * + * `params` must be the *normalised* ones - `normalizeMyFilesParams` from core, + * over the route's validated search - because the cache key is built from them. + * Passing raw URL values would make `?first=10` and no `first` two entries + * holding identical rows, and the loader would fill one while the component read + * the other. + * + * No `initialData`: the loader has already put the page 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. + */ +export const myFilesQuery = ({ params }: { params: MyFilesParams }) => + myFilesQueryOptions({ fetchPage: fetchMyFilesPage, params }) + +/** + * Marks every cached page of the visitor's files stale. + * + * The whole family, by prefix - not the one page on screen. A delete changes + * which rows exist, so every other page, sort and search of the same list is now + * wrong too, and the visitor reaches those by pressing a button that reads from + * the cache. It is emphatically *not* `queryClient.invalidateQueries()` with no + * key: the session, the messages and every other list this app holds have not + * changed, and refetching them because a file was deleted is the blunt version + * of the `revalidatePath` this replaces. + * + * Invalidating rather than removing keeps the current rows on screen while the + * fresh ones are fetched, instead of blanking the table under the dialog that is + * still open. + */ +export const invalidateMyFiles = async ( + queryClient: QueryClient, +): Promise => + await queryClient.invalidateQueries({ queryKey: MY_FILES_QUERY_ROOT }) + +/** + * Deletes one file, then refreshes the table if it actually went. + * + * Only on success. A `409` left the file exactly where it was and the dialog is + * still open offering to force past the revisions holding it; refetching + * underneath that would replace the rows the person is being asked about. + */ +export const deleteMyFile = async ( + queryClient: QueryClient, + args: DeleteMyFileArgs, +): Promise => { + const result = await deleteMyFileInBrowser(args) + + if (!result.error) await invalidateMyFiles(queryClient) + + return result +} + +/** + * Deletes a selection, then refreshes the table if anything went. + * + * `shouldRefreshAfterBulkDelete` is core's rule, and the same one the Next.js + * server action applies before it calls `revalidatePath`: a run that deleted + * nothing leaves the page as it was, and refetching would drop the selection + * that is showing which rows were kept - which is the only thing telling the + * person what to do next. + */ +export const deleteMyFiles = async ( + queryClient: QueryClient, + args: DeleteMyFilesArgs, +): Promise => { + const result = await deleteMyFilesInBrowser(args) + + if (shouldRefreshAfterBulkDelete(result)) await invalidateMyFiles(queryClient) + + return result +} + +/** + * The two callbacks `MyFilesTableContent` takes, bound to this router's cache. + * + * Memoised on the client, which is the only reason this is a hook rather than + * two calls at the point of use: the callbacks are props on a table that + * re-renders on every navigation, and new function identities would remount the + * confirm dialogs mid-delete. + */ +export const useMyFilesDeleteCallbacks = (): { + onDeleteFile: DeleteMyFile + onDeleteFiles: DeleteMyFiles +} => { + const queryClient = useQueryClient() + + return React.useMemo( + () => ({ + onDeleteFile: async (args: DeleteMyFileArgs) => + await deleteMyFile(queryClient, args), + onDeleteFiles: async (args: DeleteMyFilesArgs) => + await deleteMyFiles(queryClient, args), + }), + [queryClient], + ) +} diff --git a/apps/web/src/lib/search/discover-feed.ts b/apps/web/src/lib/search/discover-feed.ts index 32c2feb20..5e240e127 100644 --- a/apps/web/src/lib/search/discover-feed.ts +++ b/apps/web/src/lib/search/discover-feed.ts @@ -1,93 +1,36 @@ -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 { SearchFeedPageArgs } 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' +import { feedQueryKey, feedQueryOptions } from '#/lib/search/feed' /** - * 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. + * Discover, as the shared feed with Discover's parameters. + * + * Two bindings and no logic. 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`; how it travels comes from + * `#/lib/search/feed`, which every feed in this app shares. All that is left + * here is which parameters this route browses with, and those live in + * `discover-request.ts`. + * + * The named exports stay, because a loader, a component and a test all read + * "the Discover feed" and none of them should have to know it is + * `{ sort: 'newest' }`. */ -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 cache entry one language's Discover feed lives in. * - * 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. + * Core's key, through this app's one binding of it - see `feedQueryKey`. A key + * invented here would be a second entry holding the same feed. */ export const discoverFeedQueryKey = (locale: Locale) => - searchFeedQueryKey({ locale, params: DISCOVER_FEED_PARAMS }) + feedQueryKey({ 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. - */ +/** The Discover feed, as the one query definition every caller shares. */ export const discoverFeedQueryOptions = ({ locale }: { locale: Locale }) => - searchFeedQueryOptions({ - fetchPage: fetchDiscoverFeedPage, - locale, - params: DISCOVER_FEED_PARAMS, - }) + feedQueryOptions({ locale, params: DISCOVER_FEED_PARAMS }) export type { SearchFeedPageArgs } diff --git a/apps/web/src/lib/search/feed.ts b/apps/web/src/lib/search/feed.ts new file mode 100644 index 000000000..49dbf5a8a --- /dev/null +++ b/apps/web/src/lib/search/feed.ts @@ -0,0 +1,109 @@ +import type { + SearchFeedPageArgs, + SearchFeedPageFetcher, + SearchFeedParams, +} 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 { fetchSearchFeedPageOnServer } from '#/server/search-feed.server' + +/** + * The search 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, the cache entry it lands in - comes + * from `@vitnode/core/views/search/search-feed-query`, which is also what the + * mounted `SearchFeedContent` runs. This module supplies only the one thing core + * cannot know: how to reach the API from a server that is rendering a request. + * + * Every feed in the app is built from here - `/discover` browsing newest-first, + * `/search` with a term and filters, and whatever comes next - because they are + * the same query with different parameters. A route that bound its own transport + * would be a second definition of a feed that agreed with this one only until it + * didn't. + */ + +/** + * 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 and + * every keystroke in the search box 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 `search-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. + */ +export const fetchSearchFeedPage: SearchFeedPageFetcher = createIsomorphicFn() + .server(fetchSearchFeedPageOnServer) + .client(fetchSearchFeedPageInBrowser) + +/** + * The cache entry one 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 feedQueryKey = ({ + locale, + params, +}: { + locale: Locale + params: SearchFeedParams +}) => searchFeedQueryKey({ locale, params }) + +/** + * One 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`. A route 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 feedQueryOptions = ({ + locale, + params, +}: { + locale: Locale + params: SearchFeedParams +}) => + searchFeedQueryOptions({ + fetchPage: fetchSearchFeedPage, + locale, + params, + }) + +export type { SearchFeedPageArgs, SearchFeedParams } diff --git a/apps/web/src/lib/search/search-request.ts b/apps/web/src/lib/search/search-request.ts new file mode 100644 index 000000000..ea5a2c23c --- /dev/null +++ b/apps/web/src/lib/search/search-request.ts @@ -0,0 +1,74 @@ +import type { SearchFeedParams } from '@vitnode/core/views/search/search-feed-query' + +import { + normalizeSearchTerm, + searchFeedParamsFor, +} from '@vitnode/core/views/search/search-params' + +/** + * What `/search` reads out of its URL, and what it turns that into. + * + * Two pure functions, no transport and no React, so the route's contract can be + * stated and tested without a router: `src/tests/search-request.test.ts` is the + * whole of it. + * + * Both delegate to `@vitnode/core/views/search/search-params`, which is where + * the meaning of a search request lives - the same module the Next.js + * `SearchView` reads its `searchParams` through. `/search?search=hello` is + * therefore the same request in both applications rather than two hand-written + * approximations of it. + */ + +/** + * The one search parameter this route has. + * + * `search` is in the URL because a search has to be shareable and because a + * crawler landing on `/search?search=hello` should be served those results. The + * sort and the type filters are **not**: they are controls the visitor drives + * after the page has loaded, they have never been in the URL, and putting them + * there means deciding what a canonical search URL is and whether every + * keystroke is a history entry. That is a product question; Stage 7 is a move. + */ +export interface SearchRouteSearch { + search?: string +} + +/** + * The route's search schema - written as a function rather than a schema object + * because its job is to *normalise*, not to reject. + * + * A search page is the one page whose query string is typed by strangers. + * `?search=` arrives empty, `?search=a&search=b` arrives as an array, + * `?search=<40KB>` arrives as a denial-of-service attempt on the full-text + * index - and every one of them should render the search page, not an error + * boundary. `normalizeSearchTerm` answers all three: anything that is not a + * usable term becomes no term at all, which is the browse feed. + * + * A missing term is returned as an *absent* key rather than + * `{ search: undefined }`, so the router has nothing to write back into the URL + * and `/search?search=%20` settles as `/search`. + */ +export const normalizeSearchRouteSearch = ( + input: Record, +): SearchRouteSearch => { + const search = normalizeSearchTerm(input.search) + + return search === undefined ? {} : { search } +} + +/** + * The route's search parameters, as the shared feed's own. + * + * The sort is not passed and is not missing: `searchFeedParamsFor` derives it + * from whether there is a term - relevance when there is one, newest when there + * is not - which is the rule the controls then start from and the rule the + * Next.js page has always applied. + * + * With no term this is `{ sort: 'newest' }`, which is exactly + * `DISCOVER_FEED_PARAMS`. That is deliberate: `/search` with an empty box and + * `/discover` are the same request over the same documents, so they share one + * cache entry rather than fetching it twice. + */ +export const searchRouteFeedParams = ({ + search, +}: SearchRouteSearch): SearchFeedParams => searchFeedParamsFor({ search }) diff --git a/apps/web/src/locales/@vitnode/core/pl.json b/apps/web/src/locales/@vitnode/core/pl.json index b42564635..f9023f682 100644 --- a/apps/web/src/locales/@vitnode/core/pl.json +++ b/apps/web/src/locales/@vitnode/core/pl.json @@ -8,11 +8,20 @@ "theme_switcher": "Zmień motyw" }, "search": { + "title": "Szukaj", + "desc": "Przeszukaj wszystko w społeczności.", "discoverTitle": "Odkrywaj", "discoverDesc": "Zobacz najnowszą aktywność w społeczności.", + "placeholder": "Szukaj…", "empty": "Nic tu jeszcze nie ma.", "loadMore": "Wczytaj więcej", "loading": "Wczytywanie…", + "sortBy": "Sortuj według", + "sort": { + "relevance": "Trafność", + "newest": "Najnowsze", + "oldest": "Najstarsze" + }, "types": { "blog_post": "Wpis", "unknown": "Treść" diff --git a/apps/web/src/routeTree.gen.ts b/apps/web/src/routeTree.gen.ts index c4ed15692..75e434876 100644 --- a/apps/web/src/routeTree.gen.ts +++ b/apps/web/src/routeTree.gen.ts @@ -13,7 +13,9 @@ import { Route as IndexRouteImport } from './routes/index' import { Route as AuthenticatedRouteImport } from './routes/_authenticated' import { Route as DiscoverRouteImport } from './routes/discover' import { Route as LoginRouteImport } from './routes/login' +import { Route as SearchRouteImport } from './routes/search' import { Route as AuthenticatedAccountRouteImport } from './routes/_authenticated/account' +import { Route as AuthenticatedFilesRouteImport } from './routes/_authenticated/files' import { Route as ApiSplatRouteImport } from './routes/api/$' import { Route as LoginSsoProviderIdRouteImport } from './routes/login_.sso.$providerId' @@ -36,11 +38,21 @@ const LoginRoute = LoginRouteImport.update({ path: '/login', getParentRoute: () => rootRouteImport, } as any) +const SearchRoute = SearchRouteImport.update({ + id: '/search', + path: '/search', + getParentRoute: () => rootRouteImport, +} as any) const AuthenticatedAccountRoute = AuthenticatedAccountRouteImport.update({ id: '/account', path: '/account', getParentRoute: () => AuthenticatedRoute, } as any) +const AuthenticatedFilesRoute = AuthenticatedFilesRouteImport.update({ + id: '/files', + path: '/files', + getParentRoute: () => AuthenticatedRoute, +} as any) const ApiSplatRoute = ApiSplatRouteImport.update({ id: '/api/$', path: '/api/$', @@ -56,7 +68,9 @@ export interface FileRoutesByFullPath { '/': typeof IndexRoute '/discover': typeof DiscoverRoute '/login': typeof LoginRoute + '/search': typeof SearchRoute '/account': typeof AuthenticatedAccountRoute + '/files': typeof AuthenticatedFilesRoute '/api/$': typeof ApiSplatRoute '/login/sso/$providerId': typeof LoginSsoProviderIdRoute } @@ -64,7 +78,9 @@ export interface FileRoutesByTo { '/': typeof IndexRoute '/discover': typeof DiscoverRoute '/login': typeof LoginRoute + '/search': typeof SearchRoute '/account': typeof AuthenticatedAccountRoute + '/files': typeof AuthenticatedFilesRoute '/api/$': typeof ApiSplatRoute '/login/sso/$providerId': typeof LoginSsoProviderIdRoute } @@ -74,7 +90,9 @@ export interface FileRoutesById { '/_authenticated': typeof AuthenticatedRouteWithChildren '/discover': typeof DiscoverRoute '/login': typeof LoginRoute + '/search': typeof SearchRoute '/_authenticated/account': typeof AuthenticatedAccountRoute + '/_authenticated/files': typeof AuthenticatedFilesRoute '/api/$': typeof ApiSplatRoute '/login_/sso/$providerId': typeof LoginSsoProviderIdRoute } @@ -84,7 +102,9 @@ export interface FileRouteTypes { | '/' | '/discover' | '/login' + | '/search' | '/account' + | '/files' | '/api/$' | '/login/sso/$providerId' fileRoutesByTo: FileRoutesByTo @@ -92,7 +112,9 @@ export interface FileRouteTypes { | '/' | '/discover' | '/login' + | '/search' | '/account' + | '/files' | '/api/$' | '/login/sso/$providerId' id: @@ -101,7 +123,9 @@ export interface FileRouteTypes { | '/_authenticated' | '/discover' | '/login' + | '/search' | '/_authenticated/account' + | '/_authenticated/files' | '/api/$' | '/login_/sso/$providerId' fileRoutesById: FileRoutesById @@ -111,6 +135,7 @@ export interface RootRouteChildren { AuthenticatedRoute: typeof AuthenticatedRouteWithChildren DiscoverRoute: typeof DiscoverRoute LoginRoute: typeof LoginRoute + SearchRoute: typeof SearchRoute ApiSplatRoute: typeof ApiSplatRoute LoginSsoProviderIdRoute: typeof LoginSsoProviderIdRoute } @@ -145,6 +170,13 @@ declare module '@tanstack/react-router' { preLoaderRoute: typeof LoginRouteImport parentRoute: typeof rootRouteImport } + '/search': { + id: '/search' + path: '/search' + fullPath: '/search' + preLoaderRoute: typeof SearchRouteImport + parentRoute: typeof rootRouteImport + } '/_authenticated/account': { id: '/_authenticated/account' path: '/account' @@ -152,6 +184,13 @@ declare module '@tanstack/react-router' { preLoaderRoute: typeof AuthenticatedAccountRouteImport parentRoute: typeof AuthenticatedRoute } + '/_authenticated/files': { + id: '/_authenticated/files' + path: '/files' + fullPath: '/files' + preLoaderRoute: typeof AuthenticatedFilesRouteImport + parentRoute: typeof AuthenticatedRoute + } '/api/$': { id: '/api/$' path: '/api/$' @@ -171,10 +210,12 @@ declare module '@tanstack/react-router' { interface AuthenticatedRouteChildren { AuthenticatedAccountRoute: typeof AuthenticatedAccountRoute + AuthenticatedFilesRoute: typeof AuthenticatedFilesRoute } const AuthenticatedRouteChildren: AuthenticatedRouteChildren = { AuthenticatedAccountRoute: AuthenticatedAccountRoute, + AuthenticatedFilesRoute: AuthenticatedFilesRoute, } const AuthenticatedRouteWithChildren = AuthenticatedRoute._addFileChildren( @@ -186,6 +227,7 @@ const rootRouteChildren: RootRouteChildren = { AuthenticatedRoute: AuthenticatedRouteWithChildren, DiscoverRoute: DiscoverRoute, LoginRoute: LoginRoute, + SearchRoute: SearchRoute, ApiSplatRoute: ApiSplatRoute, LoginSsoProviderIdRoute: LoginSsoProviderIdRoute, } diff --git a/apps/web/src/routes/_authenticated/files.tsx b/apps/web/src/routes/_authenticated/files.tsx new file mode 100644 index 000000000..7dff5bf0f --- /dev/null +++ b/apps/web/src/routes/_authenticated/files.tsx @@ -0,0 +1,261 @@ +import type { DataTableNavigation } from '@vitnode/core/components/table/navigation' + +import { useSuspenseQuery } from '@tanstack/react-query' +import { createFileRoute } from '@tanstack/react-router' +import { DataTableNavigationProvider } from '@vitnode/core/components/table/navigation' +import { HeaderContent } from '@vitnode/core/components/ui/header-content' +import { formatPageTitle } from '@vitnode/core/lib/metadata' +import { MyFilesTableContent } from '@vitnode/core/views/files/my-files-table-content' +import React from 'react' +import { createTranslator } from 'use-intl' + +import { RouteMessages } from '#/components/route-messages' +import { myFilesQuery, useMyFilesDeleteCallbacks } from '#/lib/files/my-files' +import { + myFilesRouteParams, + myFilesSearchFrom, + myFilesSearchParams, + normalizeMyFilesRouteSearch, +} from '#/lib/files/my-files-route' +import { intlQueryOptions } from '#/lib/i18n/query' +import { vitNodeShellConfig } from '#/vitnode.shell.config' + +/** + * The visitor's own files, rendered outside Next.js. + * + * One route file serving two public URLs. `/files` and `/pl/files` 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/files.tsx` to keep in step. The Next.js route at + * `packages/vitnode/src/routes/main/files/page.tsx` is still live and unchanged - + * this is a parallel slice until the cutover. + * + * ## Where it sits, and what that buys + * + * Under `_authenticated`, which is a pathless layout: the file's *location* is + * the guard, and this route contributes no URL segment of its own. There is + * deliberately no session check in this file. The Next.js page opens with + * `getSessionApi()` and `notFound()` because it has nowhere else to put the rule; + * here that rule is `routes/_authenticated.tsx`, it runs in `beforeLoad` before + * any of this renders, and it answers an anonymous visitor with + * `/login?returnTo=/files` - carrying whatever sort and page they were heading + * for, and no locale, because the rewrite writes that back on the way home. + * + * A second check here would not be defence in depth, it would be a second rule to + * keep in step with the first. The actual boundary is neither: `GET + * /api/@vitnode/core/users/files` derives the owner from the session cookie on + * every request, which is why a session that ends while this page is open shows + * up below as a failed query rather than as somebody else's files. + * + * ## One query contract, one cache entry + * + * The table is `myFilesQuery` and nothing else, in the loader and in the + * component: + * + * loader: ensureQueryData(myFilesQuery({ params })) + * component: useSuspenseQuery(myFilesQuery({ params })) + * after a delete: invalidate the family, and the component refetches + * + * Same key, same request, same refusal handling - so the page the server rendered + * is the page the browser reads, and there is no `initialData` anywhere: the + * loader has already put it in the entry the component reads and the SSR pass + * dehydrates it, so a second copy of those bytes could only disagree with the + * first. + * + * `params` is the *normalised* request from `loaderDeps`, handed to the component + * through the loader rather than derived a second time, so the two cannot drift + * apart through a difference in how each computed it. + */ + +/** + * What this page renders strings from. + * + * `core.files` is the heading, the columns, the empty state and every word of + * both delete dialogs. `core.global` is the rest of the table - the pager's + * labels, the search placeholder, the confirm dialog's buttons and the error + * toasts - and it is listed even though the root already provides it, because + * `RouteMessages` mounts its own provider over the root's rather than adding to + * it. + * + * 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 FILES_NAMESPACES = ['core.files', 'core.global'] as const + +export const Route = createFileRoute('/_authenticated/files')({ + component: MyFilesRoute, + /** + * The request, as the only thing the loader re-runs for. + * + * The *normalised* parameters rather than the raw search, and that is what + * makes this exact. The router hands `loaderDeps` the validated search merged + * over everything else that was in the query string, so keying on it directly + * would re-run the loader for a stray `?utm_source=` - and, worse, would treat + * `?first=10` and no `first` as two different pages of the same rows. + * Normalised, the dependency is precisely "which rows are being asked for", + * which is also what the query key is built from. + */ + loaderDeps: ({ search }) => ({ params: myFilesRouteParams(search) }), + /** + * Both reads this page needs, in parallel, before it renders. + * + * `context.locale` comes from the root route's `beforeLoad`, which resolved it + * from the public URL - so `/pl/files` fetches Polish messages, 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 page by + * `useSuspenseQuery` through the identical `myFilesQuery`. + * + * The session is *not* fetched here. `_authenticated`'s `beforeLoad` has + * already put it in the one cache entry every guard reads. + * + * A refusal from the files API is deliberately left to propagate. `401`, `403` + * and `429` reject as `MyFilesRequestError`, which fails this loader and shows + * the router's error path - the honest answer. The alternative, catching it and + * rendering an empty table, is indistinguishable from an account with nothing + * uploaded, which is the one thing this must never look like. + */ + loader: async ({ context, deps }) => { + const [intl] = await Promise.all([ + context.queryClient.ensureQueryData( + intlQueryOptions({ + locale: context.locale, + namespaces: FILES_NAMESPACES, + }), + ), + context.queryClient.ensureQueryData( + myFilesQuery({ params: deps.params }), + ), + ]) + + /** + * The heading and the tab title, translated once so they cannot disagree. + * + * The cast is what makes `createTranslator` usable here, and it is the same + * one `login.tsx` explains at length: its key type is derived from the + * *inferred* type of `messages`, and `AbstractIntlMessages` is a bare index + * signature - so `MessageKeys` cannot tell a leaf from a branch and collapses + * to `never`, making every key a type error. Naming the two keys this route + * reads is both the smallest fix and a true statement: rename either in + * `core/locales/en.json` and this stops compiling rather than rendering a raw + * message key into a ``. + */ + const t = createTranslator({ + locale: context.locale, + messages: intl.messages as { + core: { files: { desc: string; title: string } } + }, + namespace: 'core.files', + }) + + return { description: t('desc'), params: deps.params, title: t('title') } + }, + /** + * 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. + * + * The loader translates once, so the tab title and the `<h1>` are the same + * string by construction - which is what the Next.js route gets from calling + * `getTranslations` once per request. `formatPageTitle` applies the same + * `"<page> - <site>"` rule Next.js applies through `title.template`. + */ + head: ({ loaderData }) => ({ + meta: [ + // The Next.js page sets `robots: { index: false, follow: false }`, and this + // is that: a listing of one person's uploads, behind a login, with nothing + // on it a crawler may see or follow. Stated rather than assumed - TanStack + // Start emits no robots directive of its own. + { content: 'noindex, nofollow', name: 'robots' }, + ...(loaderData + ? [ + { + title: formatPageTitle( + vitNodeShellConfig.metadata, + loaderData.title, + ), + }, + { content: loaderData.description, name: 'description' }, + ] + : []), + ], + }), + validateSearch: normalizeMyFilesRouteSearch, +}) + +function MyFilesRoute() { + const { description, params, title } = Route.useLoaderData() + const search = Route.useSearch() + const navigate = Route.useNavigate() + const { data } = useSuspenseQuery(myFilesQuery({ params })) + const { onDeleteFile, onDeleteFiles } = useMyFilesDeleteCallbacks() + + /** + * The one thing the shared table cannot decide for itself: how to change a URL. + * + * `DataTable` mounts this for Next.js (`NextDataTableNavigation`, a + * locale-aware `push`); a TanStack route mounts it with the router's own + * navigate. Everything either side of it - which parameter a sort header + * rewrites, which ones a filter resets, what a page button does with a cursor - + * is `components/table/url-state.ts` and is shared. + * + * `to` is deliberately absent: with no destination the router stays on this + * route and changes only its search, which is the whole of what a table + * control does. `resetScroll: false` is Next's `scroll: false` - somebody + * sorting the last column of a long table is looking at the header they + * clicked. + * + * The promise is returned rather than dropped so the seam's `useTransition` + * stays pending for the whole navigation, which is what keeps the current rows + * on screen with a spinner instead of blanking the table. + */ + const navigation = React.useMemo<DataTableNavigation>( + () => ({ + navigate: async (nextSearch) => { + await navigate({ + resetScroll: false, + search: myFilesSearchFrom(nextSearch), + }) + }, + searchParams: myFilesSearchParams(search), + }), + [navigate, search], + ) + + return ( + <RouteMessages namespaces={FILES_NAMESPACES}> + <main className="container mx-auto flex flex-col gap-6 p-4"> + <HeaderContent desc={description} h1={title} /> + + <DataTableNavigationProvider value={navigation}> + {/* + The same component the Next.js page renders, handed the three things a + shared table cannot resolve for itself: the page, and the two deletes. + The columns, the preview, the metadata popover and the empty state are + core's and are not restated here - see `my-files-table-content.tsx`. + + Both callbacks end in a query invalidation of the whole `files/me` + family rather than in `revalidatePath`, and only when something + actually went: a `409` leaves the file where it was and the dialog + open, and a bulk run that deleted nothing must not drop the selection + that is showing which rows were kept. That rule is core's + (`shouldRefreshAfterBulkDelete`) and is applied by + `#/lib/files/my-files`, so both frameworks refresh on the same + condition. + */} + <MyFilesTableContent + data={data} + onDeleteFile={onDeleteFile} + onDeleteFiles={onDeleteFiles} + /> + </DataTableNavigationProvider> + </main> + </RouteMessages> + ) +} diff --git a/apps/web/src/routes/search.tsx b/apps/web/src/routes/search.tsx new file mode 100644 index 000000000..9e7164183 --- /dev/null +++ b/apps/web/src/routes/search.tsx @@ -0,0 +1,213 @@ +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 { SearchControlsContent } from '@vitnode/core/views/search/search-controls-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 { feedQueryOptions } from '#/lib/search/feed' +import { + normalizeSearchRouteSearch, + searchRouteFeedParams, +} from '#/lib/search/search-request' +import { vitNodeShellConfig } from '#/vitnode.shell.config' + +/** + * Search, rendered outside Next.js. + * + * One route file serving two public URLs. `/search` and `/pl/search` 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/search.tsx` to keep in step. The + * Next.js route at `packages/vitnode/src/routes/main/search/page.tsx` is still + * live and unchanged - this is a parallel slice until the cutover. + * + * Everything visible is shared: `HeaderContent` and `SearchControlsContent` - + * which is the search box, the type filters, the sort and the feed - are the same + * modules the Next.js page renders, with the three things a shared component + * cannot resolve for itself passed in: the locale, a `Link`, and how a feed page + * is fetched. + * + * ## One query contract, one cache entry + * + * The feed is `feedQueryOptions` and nothing else, in the loader and in the + * component: + * + * loader: ensureInfiniteQueryData(feedQueryOptions({ locale, params })) + * component: <SearchControlsContent feedQuery={params => feedQueryOptions({ locale, params })} /> + * + * Same key, same page function, same cursor rule - so the loader's page is the + * page the component renders, and `fetchNextPage` continues from it. There is no + * `initialData` anywhere on this route: the loader has already put page one in + * the entry the component reads and the SSR pass dehydrates it, so a second copy + * of those bytes could only disagree with the first. + */ + +/** + * What this page renders strings from. + * + * `core.global` is the shell's, `core.search` is everything else here - the + * heading, the placeholder, the sort labels, the type labels, the feed's empty + * state and its "load more". 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 SEARCH_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. There is no hand-written list of + * migrated routes anywhere in that decision - the route tree is the list - so the + * day `/blog` moves, this file does not change. + * + * 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 on every keystroke. External and unsafe URLs never reach it: + * `SearchFeedContent` classifies those and renders them itself, which is what + * keeps a plugin-authored `javascript:` url out of the router. + */ +const SearchFeedLink = ({ children, className, href }: SearchFeedLinkProps) => ( + <MigrationLink className={className} href={href}> + {children} + </MigrationLink> +) + +export const Route = createFileRoute('/search')({ + component: SearchRoute, + /** + * The loader re-runs when the term in the URL changes, and only then. + * + * Without this the loader would warm the feed for whatever term the page was + * first opened with and never again, so following a link from + * `/search?search=hono` to `/search?search=drizzle` would render the first + * result set and fetch the second from the browser. + */ + loaderDeps: ({ search }) => ({ search: search.search }), + /** + * Everything 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/search` 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 `feedQueryOptions` warms. 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. + * + * `params` is returned rather than rebuilt in the component for exactly that + * reason: the object handed to the controls as their starting point is + * *literally* the one whose cache entry was warmed, so the two cannot drift + * apart through a difference in how each derived it. + * + * The strings the metadata needs are returned too rather than looked up again: + * `createTranslator` is `use-intl`'s framework-free translator, over the + * messages just fetched. + */ + loader: async ({ context, deps }) => { + const params = searchRouteFeedParams({ search: deps.search }) + + const [intl] = await Promise.all([ + context.queryClient.ensureQueryData( + intlQueryOptions({ + locale: context.locale, + namespaces: SEARCH_NAMESPACES, + }), + ), + context.queryClient.ensureInfiniteQueryData( + feedQueryOptions({ locale: context.locale, params }), + ), + ]) + + const t = createTranslator({ + locale: context.locale, + messages: intl.messages, + namespace: 'core.search', + }) + + return { description: t('desc'), params, title: t('title') } + }, + /** + * 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. + * + * The loader translates once, so the tab title and the `<h1>` are the same + * string by construction - which is what the Next.js route gets from calling + * `getTranslations` once per request. `formatPageTitle` applies the same + * `"<page> - <site>"` rule Next.js applies through `title.template`. + */ + head: ({ loaderData }) => ({ + meta: [ + // Indexable, and stated rather than assumed: TanStack Start emits no + // robots directive of its own, and the Next.js route this replaces sets + // `robots: { index: true, follow: true }` explicitly. Whether a search + // page with an arbitrary term *should* be indexed is a question for the + // SEO pass, not for a migration. + { content: 'index, follow', name: 'robots' }, + ...(loaderData + ? [ + { + title: formatPageTitle( + vitNodeShellConfig.metadata, + loaderData.title, + ), + }, + { content: loaderData.description, name: 'description' }, + ] + : []), + ], + }), + validateSearch: normalizeSearchRouteSearch, +}) + +function SearchRoute() { + const locale = useLocale() + const { description, params, title } = Route.useLoaderData() + + return ( + <RouteMessages namespaces={SEARCH_NAMESPACES}> + <main className="container mx-auto flex max-w-3xl flex-col gap-6 p-4"> + <HeaderContent desc={description} h1={title} /> + + {/* + The term in the URL is the controls' starting point, so a *change* to + it has to become a new starting point - and the controls hold their + term in state, which React preserves across a re-render. Keyed on the + term, the loader re-running for `?search=drizzle` remounts them, and + they read the entry that loader just warmed instead of showing the + previous search over freshly-fetched-and-ignored results. + + `feedQuery` rather than a finished options object because the visitor + changes the request: every keystroke, filter and sort is a different + query, built here from the same factory the loader used, so all of them + share one contract and one cache. + */} + <SearchControlsContent + defaultParams={params} + feedQuery={(feedParams) => + feedQueryOptions({ locale, params: feedParams }) + } + key={params.search ?? ''} + LinkComponent={SearchFeedLink} + variant="timeline" + /> + </main> + </RouteMessages> + ) +} diff --git a/apps/web/src/server/auth.server.ts b/apps/web/src/server/auth.server.ts index 9d77e9385..5f71cd134 100644 --- a/apps/web/src/server/auth.server.ts +++ b/apps/web/src/server/auth.server.ts @@ -32,7 +32,7 @@ import { fetcherServer, saveApiCookies } from '#/server/fetcher.server' * browser <- saveApiCookies <- Set-Cookie * * Split out of `#/lib/auth/mutations` for the same reason - * `discover-feed.server.ts` is split out of `lib/search/discover-feed.ts`: that + * `search-feed.server.ts` is split out of `lib/search/feed.ts`: that * module is imported by the browser bundle, and this one imports the request * scope (`getRequestHeaders`, `setCookie`) and the `server-only` marker above. * Reached only from inside a `createServerFn` handler, which is what keeps it - diff --git a/apps/web/src/server/my-files.server.ts b/apps/web/src/server/my-files.server.ts new file mode 100644 index 000000000..1cf8c6736 --- /dev/null +++ b/apps/web/src/server/my-files.server.ts @@ -0,0 +1,46 @@ +import '@tanstack/react-start/server-only' +import type { + MyFilesPageFetcher, + MyFilesParams, +} from '@vitnode/core/views/files/my-files-query' + +import { + myFilesRequest, + MyFilesRequestError, + userFilesModuleRef, +} from '@vitnode/core/views/files/my-files-query' + +import { fetcherServer } from '#/server/fetcher.server' + +/** + * One page of the visitor's own files, fetched during SSR. + * + * The request and the refusal check are core's - the same two the browser + * fetcher uses - so a page rendered on the server and a page fetched 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`, and here that is not a nicety: the + * list is per-visitor and the API decides whose it is from the `Cookie` header. + * A render that forwarded nothing would be answered as an anonymous visitor - + * `401` - so this is the difference between a signed-in page and an error. It + * also resolves the API origin from the request being rendered, so a preview + * deployment calls its own hostname rather than a configured one. + * + * Only ever reached through the isomorphic transport in `#/lib/files/my-files`, + * which is what keeps this module - and the `server-only` marker above it - out + * of the browser bundle. + */ +export const fetchMyFilesPageOnServer: MyFilesPageFetcher = async ( + params: MyFilesParams, +) => { + const response = await fetcherServer( + userFilesModuleRef, + myFilesRequest(params), + ) + + if (!response.ok) throw new MyFilesRequestError(response.status, params) + + return await response.json() +} diff --git a/apps/web/src/server/discover-feed.server.ts b/apps/web/src/server/search-feed.server.ts similarity index 68% rename from apps/web/src/server/discover-feed.server.ts rename to apps/web/src/server/search-feed.server.ts index b41b11caf..aac242263 100644 --- a/apps/web/src/server/discover-feed.server.ts +++ b/apps/web/src/server/search-feed.server.ts @@ -13,7 +13,12 @@ import { import { fetcherServer } from '#/server/fetcher.server' /** - * One page of the Discover feed, fetched during SSR. + * One page of a search feed, fetched during SSR. + * + * Every feed this app renders on the server comes through here - `/discover` + * browsing newest-first, `/search` with a term - because they are one request + * with different parameters. There is deliberately no per-route copy: the whole + * point of `searchFeedRequest` is that the request is decided once. * * 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()` @@ -28,11 +33,11 @@ import { fetcherServer } from '#/server/fetcher.server' * 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. + * Only ever reached through the isomorphic transport in `#/lib/search/feed`, + * which is what keeps this module - and the `server-only` import above - out of + * the browser bundle. */ -export const fetchDiscoverFeedPageOnServer: SearchFeedPageFetcher = async ( +export const fetchSearchFeedPageOnServer: SearchFeedPageFetcher = async ( args: SearchFeedPageArgs, ) => { const response = await fetcherServer(searchModuleRef, searchFeedRequest(args)) diff --git a/apps/web/src/tests/isolation.test.ts b/apps/web/src/tests/isolation.test.ts index 2e1888f4e..bea90ee6c 100644 --- a/apps/web/src/tests/isolation.test.ts +++ b/apps/web/src/tests/isolation.test.ts @@ -365,16 +365,28 @@ describe('the whole graph this app imports stays Next-free', () => { 'apps/web/src/routes/_authenticated/account.tsx', 'apps/web/src/routes/login.tsx', 'apps/web/src/routes/login_.sso.$providerId.tsx', + // Stage 7. `/files` renders the whole data table - eight columns, the + // bulk-action bar and both confirm dialogs - which is the deepest this app + // reaches into the design system after the auth screens. That graph was + // Next-only until `next/dynamic` inside `ConfirmActionAlertDialog` became + // `React.lazy`, so it is exactly the graph worth walking here. + 'apps/web/src/lib/files/my-files-route.ts', + 'apps/web/src/lib/files/my-files.ts', + 'apps/web/src/routes/_authenticated/files.tsx', + 'apps/web/src/server/my-files.server.ts', '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/lib/search/feed.ts', + 'apps/web/src/lib/search/search-request.ts', 'apps/web/src/router.tsx', 'apps/web/src/routes/__root.tsx', 'apps/web/src/routes/discover.tsx', 'apps/web/src/routes/index.tsx', - 'apps/web/src/server/discover-feed.server.ts', + 'apps/web/src/routes/search.tsx', + 'apps/web/src/server/search-feed.server.ts', 'apps/web/src/server/locale.server.ts', 'apps/web/src/server/messages.server.ts', 'apps/web/src/start.ts', @@ -480,6 +492,171 @@ describe('the whole graph this app imports stays Next-free', () => { expect(reached.filter((one) => one.includes('navigation'))).toEqual([]) }) }) + + /** + * `/search`, on its own. + * + * Stated separately from `/discover` because it renders strictly more of the + * shared stack: the same feed, plus the controls above it - an input group, a + * native select, a row of buttons and a debounced callback. That is the design + * system, and the design system is where a stray `next/dynamic` or + * `next-intl/navigation` hides. `SearchControls` was Next-only for exactly + * that reason until the controls became `SearchControlsContent`. + */ + describe('the /search runtime graph reaches no Next.js', () => { + const SEARCH = ['apps/web/src/routes/search.tsx'] + + it('walks into the shared controls 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(SEARCH).visited] + + expect( + reached.some((path) => path.includes('search-controls-content')), + ).toBe(true) + expect(reached.some((path) => path.includes('search-feed-content'))).toBe( + true, + ) + expect(reached.some((path) => path.includes('input-group'))).toBe(true) + }) + + it('never reaches the Next wrapper the shared controls were split from', () => { + // `search-controls.tsx` resolves the locale through `next-intl` and takes + // its link from `@/lib/navigation`. Reaching it from here would mean the + // route imported the wrapper rather than the shared component. + const reached = [...reachableExternals(SEARCH).visited] + + expect( + reached.filter((path) => /search-(controls|feed)\.js$/.test(path)), + ).toEqual([]) + }) + + it.each([ + 'next', + 'next/cache', + 'next/dynamic', + 'next/server', + 'next-intl/navigation', + 'next-intl/server', + 'server-only', + ])('never reaches %s', (forbidden) => { + expect(offenders(SEARCH, [forbidden])).toEqual([]) + }) + + it('takes its translations from use-intl', () => { + const reached = [...reachableExternals(SEARCH).externals.keys()] + + expect(reached).toContain('use-intl') + }) + + it("only ever reaches next-intl's framework-free root entry", () => { + const reached = [...reachableExternals(SEARCH).externals.keys()] + + expect(reached.filter((one) => one.startsWith('next-intl/'))).toEqual([]) + }) + + it('never reaches a locale-aware navigation module', () => { + const reached = [...reachableExternals(SEARCH).externals.keys()] + + expect(reached.filter((one) => one.includes('navigation'))).toEqual([]) + }) + }) + + /** + * `/files`, on its own. + * + * The deepest graph this app has after the auth screens, and the one with the + * most ways to go wrong: the data table, its four URL controls, the bulk + * action bar, the row menu and both confirm dialogs. Three separate imports + * kept it Next-only until Stage 7 - `next/dynamic` inside + * `ConfirmActionAlertDialog`, `@/lib/navigation` inside four table controls, + * and a `"use server"` module behind the delete button - and none of the three + * was visible from the route file. + */ + describe('the /files runtime graph reaches no Next.js', () => { + const FILES = ['apps/web/src/routes/_authenticated/files.tsx'] + + it('walks into the table and the dialogs the route renders', () => { + // Without this the assertions below would pass on a graph that stopped at + // the route file - which is exactly the graph that cannot break. + const reached = [...reachableExternals(FILES).visited] + + expect( + reached.some((path) => path.includes('my-files-table-content')), + ).toBe(true) + expect(reached.some((path) => path.includes('table/content'))).toBe(true) + expect( + reached.some((path) => path.includes('confirm-action-alert-dialog')), + ).toBe(true) + }) + + it('never reaches the Next wrappers the shared halves were split from', () => { + // `my-files-table-view` fetches through `next/headers` and imports the + // server actions; `data-table` mounts `NextDataTableNavigation`. Reaching + // either would mean the route imported a wrapper rather than the shared + // component. + const reached = [...reachableExternals(FILES).visited] + + expect( + reached.filter((path) => + /(my-files-table-view|table\/data-table|navigation-next)\.js$/.test( + path, + ), + ), + ).toEqual([]) + }) + + it("never reaches the core package's delete server action", () => { + // Importing a `"use server"` module pulls the fetcher, `next/headers` and + // the whole API module graph in behind it. Both deletes are props. + // + // Note this is not a blanket ban on `*.server`: the route legitimately + // reaches `apps/web/src/server/my-files.server.ts`, which is this app's + // own SSR transport behind `createIsomorphicFn`. The two conventions share + // a suffix and nothing else. + const reached = [...reachableExternals(FILES).visited] + + expect( + reached.filter((path) => path.includes('delete-action.server')), + ).toEqual([]) + }) + + it.each([ + 'next', + 'next/cache', + 'next/dynamic', + 'next/headers', + 'next/navigation', + 'next/server', + 'next-intl/navigation', + 'next-intl/server', + 'server-only', + ])('never reaches %s', (forbidden) => { + expect(offenders(FILES, [forbidden])).toEqual([]) + }) + + it('never reaches the API the table is authorized by', () => { + // `my-files-query.ts` imports the files module as a *type* only, so the + // route literals still infer while Hono, Drizzle and `@/database` stay out + // of the bundle. A value import here is a server framework in the browser. + const reached = [...reachableExternals(FILES).externals.keys()] + + expect(reached).not.toContain('drizzle-orm') + expect(reached.filter((one) => one.startsWith('hono'))).toEqual([]) + }) + + it("only ever reaches next-intl's framework-free root entry", () => { + const reached = [...reachableExternals(FILES).externals.keys()] + + expect(reached.filter((one) => one.startsWith('next-intl/'))).toEqual([]) + }) + + it('never reaches a locale-aware navigation module', () => { + const reached = [...reachableExternals(FILES).externals.keys()] + + expect(reached.filter((one) => one.includes('navigation'))).toEqual([]) + }) + }) }) /** diff --git a/apps/web/src/tests/my-files-route.test.ts b/apps/web/src/tests/my-files-route.test.ts new file mode 100644 index 000000000..694bccdfd --- /dev/null +++ b/apps/web/src/tests/my-files-route.test.ts @@ -0,0 +1,443 @@ +import { hashKey, QueryClient } from '@tanstack/react-query' +import { defaultParseSearch } from '@tanstack/react-router' +import { + DEFAULT_TABLE_PAGE_SIZE, + toggleTableOrder, + withTablePage, + withTablePageSize, + withTableSearch, +} from '@vitnode/core/components/table/url-state' +import { + MY_FILES_MAX_PAGE_SIZE, + MY_FILES_QUERY_ROOT, +} from '@vitnode/core/views/files/my-files-query' +import { describe, expect, it } from 'vitest' + +import { invalidateMyFiles, myFilesQuery } from '#/lib/files/my-files' +import { + myFilesRouteParams, + myFilesSearchFrom, + myFilesSearchParams, + normalizeMyFilesRouteSearch, +} from '#/lib/files/my-files-route' +import { isTanStackOwnedPath } from '#/lib/migration-navigation' +import { getRouter } from '#/router' + +/** + * `/files`'s contract with its own URL, and with the cache underneath it. + * + * Pure functions only. `normalizeMyFilesRouteSearch` is what the route hands to + * `validateSearch`, so calling it directly is calling the route's schema - no + * router, no request, no rendering. The *meaning* of a files request is core's + * and is asserted in `packages/vitnode/src/views/files/my-files-query.test.ts`; + * what is asserted here is that this route asks for the right one, that a table + * control's URL survives the round trip through it, and that a delete + * invalidates the right family and nothing else. + */ + +/** + * The route's schema, over a query string as a visitor would type it. + * + * Through the router's *own* parser rather than `URLSearchParams`, because what + * reaches `validateSearch` is not a query string and not even strings: the + * default parser turns `?first=20` into the number `20`, `?x=true` into a + * boolean, and a repeated key into an array. Half the rules below exist for + * exactly that, so a test that flattened it first would be testing something + * else. + */ +const searchFor = (query: string) => + normalizeMyFilesRouteSearch(defaultParseSearch(query)) + +/** The cache entry one URL lands in. */ +const keyFor = (query: string) => + hashKey( + myFilesQuery({ params: myFilesRouteParams(searchFor(query)) }).queryKey, + ) + +describe('the route schema reads a table request out of the URL', () => { + it('is nothing at all for the page with no query string', () => { + // `/files` is the canonical address of this page. A schema that answered + // `{ first: 10 }` here would write `?first=10` into every link the router + // builds to it - including the one a guest's `?returnTo=` comes back through. + expect(searchFor('')).toEqual({}) + }) + + it('takes the six parameters the table writes', () => { + expect( + searchFor( + 'search=logo&orderBy=name&order=asc&first=20&cursor=eyJpZCI6MX0', + ), + ).toEqual({ + cursor: 'eyJpZCI6MX0', + first: 20, + order: 'asc', + orderBy: 'name', + search: 'logo', + }) + }) + + it('carries no parameter this route does not have', () => { + // Rule 3: nothing a visitor puts in the query string is accepted unless the + // route asked for it. `?tab=` is not validated, not carried, and not sent. + expect(searchFor('orderBy=name&tab=2&utm_source=x&__proto__=y')).toEqual({ + orderBy: 'name', + }) + }) + + it('spells the default page size as saying nothing', () => { + expect(searchFor(`first=${DEFAULT_TABLE_PAGE_SIZE}`)).toEqual({}) + expect(searchFor('first=20')).toEqual({ first: 20 }) + }) + + it('keeps a backwards page of the default size, which is not the same request', () => { + // `last` says *which direction*, so it survives at a size `first` would not. + expect( + searchFor(`last=${DEFAULT_TABLE_PAGE_SIZE}&cursor=eyJpZCI6MX0`), + ).toEqual({ + cursor: 'eyJpZCI6MX0', + last: DEFAULT_TABLE_PAGE_SIZE, + }) + }) + + it('reads the numbers the router has already parsed', () => { + // `?first=20` reaches `validateSearch` as a number and `?search=1` as one + // too, while core's normaliser is written against a query string, where + // everything is a string. Without the coercion in between, `search.trim()` + // throws inside the schema and a perfectly ordinary search becomes a router + // error screen. + expect(searchFor('first=20&search=1')).toEqual({ first: 20, search: '1' }) + }) + + it('takes the first value when a key is repeated', () => { + expect(searchFor('orderBy=name&orderBy=size')).toEqual({ orderBy: 'name' }) + }) +}) + +describe('a query string typed by hand renders the table anyway', () => { + it.each([ + ['an unknown sort column', 'orderBy=password'], + ['a sort direction that is not one', 'order=sideways'], + ['a page size that is not a number', 'first=abc'], + ['an empty page size', 'first='], + ['a page size of zero', 'first=0'], + ['a negative page size', 'first=-5'], + ['a fractional page size', 'first=10.5'], + ['a cursor that cannot be one', 'cursor=%F0%9F%92%A5'], + ['a search of nothing but blanks', 'search=%20%20'], + ])('renders the default table for %s', (_case, query) => { + // Not an error screen, and not a 400 from the API: an unusable value becomes + // an absent one, so an unrecognised `orderBy` falls back to the list's own + // `createdAt desc` rather than being sent. + expect(searchFor(query)).toEqual({}) + expect(keyFor(query)).toBe(keyFor('')) + }) + + it('clamps a page size past what the API will serve rather than 400ing', () => { + expect(searchFor('first=5000')).toEqual({ first: MY_FILES_MAX_PAGE_SIZE }) + }) + + it('never asks for both directions at once, which the API refuses', () => { + const search = searchFor('first=20&last=20') + + expect(search.first).toBe(20) + expect(search.last).toBeUndefined() + }) + + it('settles rather than drifting when applied to its own output', () => { + // The schema runs twice on every navigation - once on the query string a + // control produced, once more when the router validates the location that + // makes. A rule that moved the value on the second pass would drift a step + // per click. + for (const query of [ + '', + 'first=10', + 'first=5000', + 'orderBy=name&order=desc', + 'search=%20logo%20', + ]) { + const once = searchFor(query) + + expect(normalizeMyFilesRouteSearch(once)).toEqual(once) + } + }) +}) + +describe('the request the URL is asking for', () => { + it('always names a page size, because a request must', () => { + // The URL need not, and does not - see above. The default is applied here, + // where the query key can see it, rather than inside the URL builder. + expect(myFilesRouteParams(searchFor(''))).toEqual({ + first: String(DEFAULT_TABLE_PAGE_SIZE), + }) + }) + + it('sends the sort the URL asked for', () => { + expect(myFilesRouteParams(searchFor('orderBy=size&order=asc'))).toEqual({ + first: String(DEFAULT_TABLE_PAGE_SIZE), + order: 'asc', + orderBy: 'size', + }) + }) + + it('ignores whatever else the router merged into the search', () => { + // The router merges a route's validated search over the *raw* parsed one, so + // `Route.useSearch()` still carries the rest of the query string. Going back + // through the same normalisation is what makes the request depend on the six. + expect(myFilesRouteParams({ orderBy: 'name', tab: '2' })).toEqual( + myFilesRouteParams({ orderBy: 'name' }), + ) + }) +}) + +describe('one URL, one cache entry', () => { + it('is the same entry for two spellings of the same request', () => { + expect(keyFor(`first=${DEFAULT_TABLE_PAGE_SIZE}`)).toBe(keyFor('')) + expect(keyFor('search=logo')).toBe(keyFor('search=%20logo%20')) + expect(keyFor('orderBy=name&tab=2')).toBe(keyFor('orderBy=name')) + }) + + it('is a different entry for everything that changes the rows', () => { + const keys = [ + keyFor(''), + keyFor('first=20'), + keyFor('orderBy=name'), + keyFor('orderBy=name&order=asc'), + keyFor('search=logo'), + keyFor('cursor=eyJpZCI6MX0'), + ] + + expect(new Set(keys).size).toBe(keys.length) + }) + + it('hangs off the root a delete invalidates', () => { + expect( + myFilesQuery({ params: myFilesRouteParams({}) }).queryKey.slice( + 0, + MY_FILES_QUERY_ROOT.length, + ), + ).toEqual([...MY_FILES_QUERY_ROOT]) + }) +}) + +describe('the table changes the URL through the route, not around it', () => { + /** One control's click: read the URL, rewrite it, hand it back to the route. */ + const afterControl = ( + query: string, + control: (search: URLSearchParams) => string, + ) => myFilesSearchFrom(control(myFilesSearchParams(searchFor(query)))) + + const defaultOrder = { column: 'createdAt', order: 'desc' } as const + + it('hands the controls the validated search and nothing else', () => { + expect( + myFilesSearchParams(searchFor('orderBy=name&tab=2')).toString(), + ).toBe('orderBy=name') + expect(myFilesSearchParams(searchFor('')).toString()).toBe('') + }) + + it('sorts a column the table offers', () => { + expect( + afterControl('', (search) => + toggleTableOrder(search, { column: 'name', defaultOrder }), + ), + ).toEqual({ order: 'asc', orderBy: 'name' }) + }) + + it('flips a column that is already ascending', () => { + expect( + afterControl('orderBy=name&order=asc', (search) => + toggleTableOrder(search, { column: 'name', defaultOrder }), + ), + ).toEqual({ order: 'desc', orderBy: 'name' }) + }) + + it('cannot write a sort column this route does not have', () => { + // The return leg re-validates, so a control - or a plugin handing one a + // column list of its own - cannot put a column in the URL that the API would + // 400 on. The *direction* survives, and deliberately: `order` alone is what + // the list route reads as its own default column in that direction + // (`orderBy: query.orderBy ? ... : core_files.createdAt`), and the Next.js + // page produces exactly the same URL from the same click. One contract, two + // frameworks - not two normalisations that agree until they don't. + expect( + afterControl('', (search) => + toggleTableOrder(search, { column: 'password', defaultOrder }), + ), + ).toEqual({ order: 'asc' }) + }) + + it('pages forwards from the cursor the API handed back', () => { + expect( + afterControl('orderBy=name&order=asc', (search) => + withTablePage(search, { + cursor: 'eyJpZCI6MX0', + direction: 'next', + pageSize: DEFAULT_TABLE_PAGE_SIZE, + }), + ), + ).toEqual({ + cursor: 'eyJpZCI6MX0', + order: 'asc', + orderBy: 'name', + }) + }) + + it('pages backwards, and says so', () => { + expect( + afterControl('cursor=eyJpZCI6OX0', (search) => + withTablePage(search, { + cursor: 'eyJpZCI6MX0', + direction: 'previous', + pageSize: DEFAULT_TABLE_PAGE_SIZE, + }), + ), + ).toEqual({ cursor: 'eyJpZCI6MX0', last: DEFAULT_TABLE_PAGE_SIZE }) + }) + + it('changes the page size as a number, so the URL says `first=20`', () => { + // A *string* `'20'` is written to the address bar as `first=%2220%22` by + // TanStack Router's default serializer, which is neither what the Next.js + // page produces nor what a person pastes into a browser. + expect(afterControl('', (search) => withTablePageSize(search, 20))).toEqual( + { + first: 20, + }, + ) + }) + + it('returns the default page size to saying nothing', () => { + expect( + afterControl('first=20', (search) => + withTablePageSize(search, DEFAULT_TABLE_PAGE_SIZE), + ), + ).toEqual({}) + }) + + it('drops the cursor when the page size changes, and keeps the sort', () => { + expect( + afterControl('orderBy=name&order=asc&cursor=eyJpZCI6MX0', (search) => + withTablePageSize(search, 20), + ), + ).toEqual({ first: 20, order: 'asc', orderBy: 'name' }) + }) + + it('searches, and stops searching, without losing the sort', () => { + expect( + afterControl('orderBy=name&order=asc', (search) => + withTableSearch(search, 'logo'), + ), + ).toEqual({ order: 'asc', orderBy: 'name', search: 'logo' }) + + expect( + afterControl('orderBy=name&order=asc&search=logo', (search) => + withTableSearch(search, ''), + ), + ).toEqual({ order: 'asc', orderBy: 'name' }) + }) + + it('survives a full round trip unchanged when nothing was clicked', () => { + for (const query of [ + '', + 'first=20', + 'orderBy=name&order=desc', + 'search=logo&cursor=eyJpZCI6MX0', + ]) { + const search = searchFor(query) + + expect(myFilesSearchFrom(myFilesSearchParams(search).toString())).toEqual( + search, + ) + } + }) +}) + +describe('a delete makes the visitor’s files stale, and only those', () => { + const seed = () => { + const queryClient = new QueryClient() + const firstPage = myFilesQuery({ + params: myFilesRouteParams(searchFor('')), + }) + const sorted = myFilesQuery({ + params: myFilesRouteParams(searchFor('orderBy=name&order=asc')), + }) + const session = ['vitnode', 'session'] as const + + queryClient.setQueryData(firstPage.queryKey, { edges: [], pageInfo: {} }) + queryClient.setQueryData(sorted.queryKey, { edges: [], pageInfo: {} }) + queryClient.setQueryData(session, { user: { id: 1 } }) + + return { firstPage, queryClient, session, sorted } + } + + const isStale = (queryClient: QueryClient, queryKey: readonly unknown[]) => + queryClient.getQueryState(queryKey)?.isInvalidated === true + + it('marks every page, sort and search of the list, not just the one on screen', () => { + // A delete changes which rows exist, so the pages the visitor reaches by + // pressing a button - and reads from the cache - are wrong too. + const { firstPage, queryClient, sorted } = seed() + + void invalidateMyFiles(queryClient) + + expect(isStale(queryClient, firstPage.queryKey)).toBe(true) + expect(isStale(queryClient, sorted.queryKey)).toBe(true) + }) + + it('leaves everything else in the cache alone', () => { + // Emphatically not `invalidateQueries()` with no key: the session and the + // messages have not changed because a file was deleted. + const { queryClient, session } = seed() + + void invalidateMyFiles(queryClient) + + expect(isStale(queryClient, session)).toBe(false) + }) + + it('keeps the rows on screen while the fresh ones are fetched', () => { + // Invalidating rather than removing, so the table is not blanked under a + // dialog that is still open. + const { firstPage, queryClient } = seed() + + void invalidateMyFiles(queryClient) + + expect(queryClient.getQueryData(firstPage.queryKey)).toBeDefined() + }) +}) + +describe('`/files` is this app’s route now', () => { + const router = getRouter() + + it('is owned, so MigrationLink navigates to it client-side', () => { + // There is no list of migrated routes anywhere in that decision - the route + // tree is the list. Adding the route file is the whole of the handover. + expect(isTanStackOwnedPath(router, '/files')).toBe(true) + }) + + it('is owned under the locale prefix too, because that is the same route', () => { + expect(isTanStackOwnedPath(router, '/pl/files')).toBe(true) + }) + + it('is owned with the table’s own parameters on it', () => { + expect(isTanStackOwnedPath(router, '/files?orderBy=name&order=asc')).toBe( + true, + ) + }) + + it('does not drag the routes underneath it away from Next.js', () => { + // `matchRoutes` answers with the deepest *branch* it can resolve, so owning + // `/files` used to make anything below it look owned as well. + expect(isTanStackOwnedPath(router, '/files/12')).toBe(false) + }) + + it('sits under the pathless guard rather than at the top of the tree', () => { + const matched = router.matchRoutes('/files', undefined) as { + routeId: string + }[] + + expect(matched.map((match) => match.routeId)).toEqual([ + '__root__', + '/_authenticated', + '/_authenticated/files', + ]) + }) +}) diff --git a/apps/web/src/tests/plugin-routes.test.ts b/apps/web/src/tests/plugin-routes.test.ts index 980c0a704..133a65027 100644 --- a/apps/web/src/tests/plugin-routes.test.ts +++ b/apps/web/src/tests/plugin-routes.test.ts @@ -385,6 +385,25 @@ describe("the app's real route tree", () => { // Behind `_authenticated`, which is pathless: the guard adds no segment, so // the page is owned at its own path and the boundary is invisible here. ['/account', true], + // Stage 7. `/search` is a plain route; `/files` is a second page behind the + // pathless guard, so owning it must still be decided at `/files` and not at + // the boundary above it. + ['/search', true], + ['/pl/search', true], + ['/files', true], + ['/pl/files', true], + // A data table never navigates without a query string, and `matchRoutes` + // takes a pathname - so a table URL is the shape that would break if the + // query were not stripped before matching. + ['/files?orderBy=name&order=asc&first=20', true], + // Still the Next.js app's, and the case a migrated `/files` most easily + // annexes by accident: `/settings` is a sibling of nothing here, so a + // prefix-matching rule would answer for it. `/settings/security` is the + // nested one - see the `/login` note below for why that distinction is + // load-bearing rather than decorative. + ['/settings', false], + ['/settings/security', false], + ['/pl/settings/security', false], ])('answers %s as owned: %s', (href, owned) => { expect(isTanStackOwnedPath(getRouter(), href)).toBe(owned) }) diff --git a/apps/web/src/tests/router-query.test.ts b/apps/web/src/tests/router-query.test.ts index 821f61d7d..2e8f238ec 100644 --- a/apps/web/src/tests/router-query.test.ts +++ b/apps/web/src/tests/router-query.test.ts @@ -109,14 +109,20 @@ describe('the Query SSR integration is installed', () => { describe('nothing but the router creates a query client', () => { const appFiles = [ 'routes/__root.tsx', + 'routes/_authenticated/files.tsx', 'routes/discover.tsx', 'routes/index.tsx', + 'routes/search.tsx', 'components/route-messages.tsx', + 'lib/files/my-files-route.ts', + 'lib/files/my-files.ts', 'lib/i18n/client.ts', 'lib/i18n/query.ts', 'lib/i18n/shared.ts', 'lib/search/discover-feed.ts', 'lib/search/discover-request.ts', + 'lib/search/feed.ts', + 'lib/search/search-request.ts', ] it.each(appFiles)('%s mounts no QueryClientProvider', (file) => { diff --git a/apps/web/src/tests/search-request.test.ts b/apps/web/src/tests/search-request.test.ts new file mode 100644 index 000000000..67fecbf99 --- /dev/null +++ b/apps/web/src/tests/search-request.test.ts @@ -0,0 +1,172 @@ +import { hashKey } from '@tanstack/react-query' +import { MAX_SEARCH_TERM_LENGTH } from '@vitnode/core/views/search/search-params' +import { describe, expect, it } from 'vitest' + +import { discoverFeedQueryKey } from '#/lib/search/discover-feed' +import { DISCOVER_FEED_PARAMS } from '#/lib/search/discover-request' +import { feedQueryKey, feedQueryOptions } from '#/lib/search/feed' +import { + normalizeSearchRouteSearch, + searchRouteFeedParams, +} from '#/lib/search/search-request' + +/** + * `/search`'s contract with its own URL, and with the cache underneath it. + * + * Pure functions only. `normalizeSearchRouteSearch` is what the route hands to + * `validateSearch`, so calling it directly is calling the route's schema - no + * router, no request, no rendering. The feed's *behaviour* is core's and is + * asserted in `packages/vitnode/src/views/search`; what is asserted here is that + * this route asks for the right feed. + */ + +const paramsFor = (input: Record<string, unknown>) => + searchRouteFeedParams(normalizeSearchRouteSearch(input)) + +const hashOf = (input: Record<string, unknown>) => + hashKey(feedQueryKey({ locale: 'en', params: paramsFor(input) })) + +describe('the route schema reads a term out of the URL', () => { + it('takes the term somebody searched for', () => { + expect(normalizeSearchRouteSearch({ search: 'hono' })).toEqual({ + search: 'hono', + }) + }) + + it('trims it', () => { + expect(normalizeSearchRouteSearch({ search: ' hono ' })).toEqual({ + search: 'hono', + }) + }) + + it('ignores every other parameter in the query string', () => { + // The sort and the type filters are controls, not URL state - see + // `lib/search/search-request.ts`. A stray `?sort=` must not become one by + // accident. + expect( + normalizeSearchRouteSearch({ + search: 'hono', + sort: 'oldest', + types: 'blog_post', + }), + ).toEqual({ search: 'hono' }) + }) +}) + +describe('a malformed query string renders the page anyway', () => { + it.each([ + ['nothing at all', {}], + ['a bare ?search=', { search: '' }], + ['blanks', { search: ' ' }], + ['a repeated ?search=', { search: ['a', 'b'] }], + ['a number', { search: 42 }], + ['null', { search: null }], + ['an object', { search: { toString: () => 'hono' } }], + ])('reads %s as no term', (_case, input) => { + // Not an error boundary and not a 404: a search page is the one page whose + // query string is typed by strangers, so anything unusable is the browse + // feed. + expect(normalizeSearchRouteSearch(input)).toEqual({}) + }) + + it('returns an absent key rather than an explicit undefined', () => { + // So the router has nothing to write back into the URL, and + // `/search?search=%20` settles as `/search`. + expect(Object.keys(normalizeSearchRouteSearch({ search: ' ' }))).toEqual([]) + }) + + it('caps a term that was never typed by hand', () => { + const { search } = normalizeSearchRouteSearch({ + search: 'x'.repeat(MAX_SEARCH_TERM_LENGTH * 10), + }) + + expect(search).toHaveLength(MAX_SEARCH_TERM_LENGTH) + }) + + it('never throws, whatever it is handed', () => { + for (const input of [ + {}, + { search: [] }, + { search: [[]] }, + { search: Number.NaN }, + { search: Symbol('hono') }, + { other: 'ignored' }, + ]) { + expect(() => + normalizeSearchRouteSearch(input as Record<string, unknown>), + ).not.toThrow() + } + }) +}) + +describe('the feed the route asks for', () => { + it('searches by relevance when there is a term', () => { + expect(paramsFor({ search: 'hono' })).toEqual({ + search: 'hono', + sort: 'relevance', + }) + }) + + it('browses newest-first when there is not', () => { + expect(paramsFor({})).toEqual({ sort: 'newest' }) + }) + + it('is the Discover feed when the box is empty', () => { + // `/search` with nothing typed and `/discover` are the same request over the + // same documents. Sharing the entry is the point: a visitor arriving from + // one has the other already in hand. + expect(paramsFor({})).toEqual(DISCOVER_FEED_PARAMS) + expect(hashOf({})).toBe(hashKey(discoverFeedQueryKey('en'))) + }) + + it('is a different entry per term', () => { + expect(hashOf({ search: 'hono' })).not.toBe(hashOf({ search: 'drizzle' })) + }) + + it('is a different entry per language', () => { + // `/search?search=hono` and `/pl/search?search=hono` are two feeds over two + // sets of documents, so a language switch changes the key rather than the + // value under it. + expect(hashOf({ search: 'hono' })).not.toBe( + hashKey( + feedQueryKey({ locale: 'pl', params: paramsFor({ search: 'hono' }) }), + ), + ) + }) + + it('is one entry however the URL spelled the term', () => { + expect(hashOf({ search: ' hono ' })).toBe(hashOf({ search: 'hono' })) + }) +}) + +describe('the loader and the component read one query definition', () => { + it('names the same cache entry from the same parameters', () => { + // The loader ensures `feedQueryOptions({ locale, params })`; the mounted + // `SearchFeedContent` is handed the same factory with the same parameters. + // A mismatch here is an SSR page that renders, then refetches page one from + // the browser and flickers back to a skeleton. + const params = paramsFor({ search: 'hono' }) + + expect(hashKey(feedQueryOptions({ locale: 'en', params }).queryKey)).toBe( + hashKey(feedQueryKey({ locale: 'en', params })), + ) + }) + + it('starts every feed from no cursor', () => { + // `null`, spelled as the absence of a cursor: the API's schema rejects + // `cursor=` outright, so an empty one would 400 the first page of a visit. + expect( + feedQueryOptions({ locale: 'en', params: paramsFor({}) }) + .initialPageParam, + ).toBeNull() + }) + + it('passes no initialData, because the loader warmed the entry', () => { + // Two copies of page one - one in the cache, one in the options - can + // disagree. The loader's copy is the only one. + expect( + feedQueryOptions({ locale: 'en', params: paramsFor({ search: 'hono' }) }) + .initialData, + ).toBeUndefined() + }) +}) diff --git a/packages/vitnode/src/api/models/storage.ts b/packages/vitnode/src/api/models/storage.ts index f80c1fbd1..eede27005 100644 --- a/packages/vitnode/src/api/models/storage.ts +++ b/packages/vitnode/src/api/models/storage.ts @@ -3,6 +3,8 @@ import type { Context } from "hono"; import { and, eq } from "drizzle-orm"; import { HTTPException } from "hono/http-exception"; +import type { StorageFileInUseBody } from "@/lib/files/in-use"; + import { core_content_file_refs } from "@/database/content"; import { core_files } from "@/database/files"; import { isPgReferenceViolation } from "@/lib/api/pg-error"; @@ -11,6 +13,7 @@ import { generateStorageFileName, replaceFileExtension, } from "@/lib/api/upload"; +import { STORAGE_FILE_IN_USE } from "@/lib/files/in-use"; import { formatBytes } from "@/lib/format-bytes"; const DEFAULT_IMAGE_QUALITY = 85; @@ -58,26 +61,16 @@ export interface StorageFileUploadResult extends StorageUploadResult { size: number; } -/** Why {@link StorageModel.deleteFile} refused. */ -export const STORAGE_FILE_IN_USE = "FILE_IN_USE"; - /** - * The body of that refusal, and the reason it is not just a code. + * Why {@link StorageModel.deleteFile} refused, and the body it refuses with. * - * "In use" covers two situations a person has to act on differently: content - * that would break, and history that would merely lose a restore. `content` is - * the one that is final; `revisions` is how many retained revisions hold the - * file, so a client can offer to force past them and say how much it is giving - * up. + * Defined in `@/lib/files/in-use` and re-exported here, so every existing + * importer keeps working. The definition had to move because the browser reads + * the same code off the same 409 - and importing it from this module dragged + * Hono, Drizzle and `@/database` into the client bundle behind one string. */ -export interface StorageFileInUseBody { - code: typeof STORAGE_FILE_IN_USE; - /** A live content column or gallery row still points at this file. */ - content: boolean; - id: number; - /** Retained revisions pinning it - releasable with `force`. */ - revisions: number; -} +export type { StorageFileInUseBody } from "@/lib/files/in-use"; +export { STORAGE_FILE_IN_USE } from "@/lib/files/in-use"; export interface StorageDeleteFileOptions { /** diff --git a/packages/vitnode/src/components/confirm-action/confirm-action-alert-dialog.tsx b/packages/vitnode/src/components/confirm-action/confirm-action-alert-dialog.tsx index 4f9b8c2b6..40f9d75c2 100644 --- a/packages/vitnode/src/components/confirm-action/confirm-action-alert-dialog.tsx +++ b/packages/vitnode/src/components/confirm-action/confirm-action-alert-dialog.tsx @@ -1,7 +1,6 @@ "use client"; import { useTranslations } from "next-intl"; -import dynamic from "next/dynamic"; import React from "react"; import { @@ -14,7 +13,17 @@ import { AlertDialogTrigger, } from "../ui/alert-dialog"; -const ContentConfirmAction = dynamic(async () => +/** + * `React.lazy` rather than `next/dynamic`, which is what this used to be. + * + * The two are the same thing here - this is a client component, the import is + * already wrapped in the `<React.Suspense>` below, and `next/dynamic` defaults + * to server rendering the chunk - but only one of them resolves outside a + * Next.js app. Every confirm dialog in VitNode goes through this component, + * including the ones on the shared `/files` table, so that single import was + * enough to make the whole screen Next.js-only. + */ +const ContentConfirmAction = React.lazy(async () => import("./content").then(module => ({ default: module.ContentConfirmAction, })), diff --git a/packages/vitnode/src/components/table/content.tsx b/packages/vitnode/src/components/table/content.tsx index 8117e068b..75734df46 100644 --- a/packages/vitnode/src/components/table/content.tsx +++ b/packages/vitnode/src/components/table/content.tsx @@ -4,9 +4,9 @@ import { useTranslations } from "next-intl"; import type { AlignDataTable, ColumnDef, - DataTable, + DataTableProps, DataTableTMin, -} from "./data-table"; +} from "./data-table-content"; import { cn } from "../../lib/utils"; import { @@ -46,7 +46,7 @@ export function ContentDataTable<T extends DataTableTMin>({ searchPlaceholder, filters, ...props -}: React.ComponentProps<typeof DataTable<T>>) { +}: DataTableProps<T>) { const t = useTranslations("core.global"); const hasToolbar = Boolean(search) || Boolean(filters?.length); const allColumns: ColumnDef<T>[] = bulkActions diff --git a/packages/vitnode/src/components/table/data-table-content.tsx b/packages/vitnode/src/components/table/data-table-content.tsx new file mode 100644 index 000000000..fb997fcd5 --- /dev/null +++ b/packages/vitnode/src/components/table/data-table-content.tsx @@ -0,0 +1,169 @@ +import React from "react"; + +import type { FilterDataTable } from "./filters"; +import type { PaginationDataTable } from "./pagination"; +import type { SearchDataTable } from "./search"; + +import { cn } from "../../lib/utils"; +import { Skeleton } from "../ui/skeleton"; +import { + Table, + TableBody, + TableCell, + TableHead, + TableHeader, + TableRow, +} from "../ui/table"; + +export interface DataTableTMin { + id: number; +} + +export interface SearchParamsDataTable<T = unknown> { + cursor?: string; + first?: string; + last?: string; + order?: "asc" | "desc"; + orderBy?: keyof T; +} + +export type AlignDataTable = "center" | "left" | "right"; + +interface ColumnDefBase<T extends DataTableTMin> { + align?: AlignDataTable; + cell?: (data: { allData: T[]; row: T }) => React.ReactNode; + className?: string; + header: React.ReactNode; +} + +interface AccessorColumnDef<T extends DataTableTMin> extends ColumnDefBase<T> { + accessorKey: keyof T; + id?: string; +} + +interface DisplayColumnDef<T extends DataTableTMin> extends ColumnDefBase<T> { + accessorKey?: never; + id: string; +} + +export type ColumnDef<T extends DataTableTMin> = + AccessorColumnDef<T> | DisplayColumnDef<T>; + +/** + * Everything a data table is told, in one place both frameworks can import. + * + * Named rather than inferred from the component because the component the props + * belong to is now the Next.js one - `content.tsx` and the sort header would + * otherwise reach for `ComponentProps<typeof DataTable>` and, through it, for a + * module a TanStack Start route cannot load. + */ +export type DataTableProps<T extends DataTableTMin> = Omit< + React.ComponentProps<typeof Table>, + "columns" +> & + React.ComponentProps<typeof PaginationDataTable> & + React.ComponentProps<typeof SearchDataTable> & { + bulkActions?: React.ReactNode; + columns: ColumnDef<T>[]; + customNoResults?: { + description?: string; + footer?: React.ReactNode; + icon?: React.ReactNode; + title?: string; + }; + edges: T[]; + filters?: FilterDataTable[]; + id: string; + order: { + columns?: (keyof T)[]; + defaultOrder: { + column: keyof T; + order: "asc" | "desc"; + }; + }; + search?: boolean; + }; + +const SKELETON_HEAD_WIDTHS = ["w-24", "w-16", "w-20", "w-14"]; +const SKELETON_CELL_WIDTHS = ["w-full", "w-3/4", "w-1/2", "w-5/6", "w-2/3"]; + +/** + * The table's shape before its rows arrive. + * + * It lives beside the types rather than with `DataTable` because a Suspense + * fallback is the one part of the table a route renders *outside* the table - + * and a TanStack Start route reaching into the Next.js module for it would drag + * `next-intl`'s navigation in behind it. + */ +export const DataTableSkeleton = ({ + columns, + rows = 6, + toolbar = false, +}: { + columns: number; + rows?: number; + toolbar?: boolean; +}) => { + const headerIds = Array.from({ length: columns }, (_, i) => `s-head-${i}`); + const rowIds = Array.from({ length: rows }, (_, i) => `s-row-${i}`); + + return ( + <div className="space-y-4"> + {toolbar && ( + <div className="flex flex-col gap-2 sm:flex-row sm:items-center"> + <div className="flex-1"> + <Skeleton className="h-9 w-full" /> + </div> + </div> + )} + + <div className="[&>div]:rounded-md [&>div]:border"> + <Table className="min-w-full"> + <TableHeader className="bg-card"> + <TableRow> + {headerIds.map((hid, i) => ( + <TableHead key={hid}> + <Skeleton + className={cn( + "h-4", + SKELETON_HEAD_WIDTHS[i % SKELETON_HEAD_WIDTHS.length], + )} + /> + </TableHead> + ))} + </TableRow> + </TableHeader> + <TableBody> + {rowIds.map((rid, i) => ( + <TableRow className="h-9" key={rid}> + {headerIds.map((hid, j) => ( + <TableCell key={`${rid}-${hid}`}> + <Skeleton + className={cn( + "h-4", + SKELETON_CELL_WIDTHS[ + (i + j) % SKELETON_CELL_WIDTHS.length + ], + )} + /> + </TableCell> + ))} + </TableRow> + ))} + </TableBody> + </Table> + </div> + + <div className="flex w-full flex-col-reverse items-center justify-end gap-4 overflow-auto p-1 sm:flex-row sm:gap-8"> + <div className="flex flex-wrap items-center justify-center gap-4 sm:gap-6 lg:gap-8"> + <Skeleton className="h-8 w-[4.5rem]" /> + + <div className="flex items-center space-x-2"> + <Skeleton className="size-8" /> + <Skeleton className="size-8" /> + </div> + </div> + </div> + </div> + ); +}; diff --git a/packages/vitnode/src/components/table/data-table.tsx b/packages/vitnode/src/components/table/data-table.tsx index 0a317df7a..75d0ec60e 100644 --- a/packages/vitnode/src/components/table/data-table.tsx +++ b/packages/vitnode/src/components/table/data-table.tsx @@ -1,160 +1,39 @@ -import React from "react"; +import type { DataTableProps, DataTableTMin } from "./data-table-content"; -import type { FilterDataTable } from "./filters"; -import type { PaginationDataTable } from "./pagination"; -import type { SearchDataTable } from "./search"; - -import { cn } from "../../lib/utils"; import { ErrorView } from "../../views/error/error-view"; -import { Skeleton } from "../ui/skeleton"; -import { - Table, - TableBody, - TableCell, - TableHead, - TableHeader, - TableRow, -} from "../ui/table"; import { ContentDataTable } from "./content"; - -export interface DataTableTMin { - id: number; -} - -export interface SearchParamsDataTable<T = unknown> { - cursor?: string; - first?: string; - last?: string; - order?: "asc" | "desc"; - orderBy?: keyof T; -} - -export type AlignDataTable = "center" | "left" | "right"; - -interface ColumnDefBase<T extends DataTableTMin> { - align?: AlignDataTable; - cell?: (data: { allData: T[]; row: T }) => React.ReactNode; - className?: string; - header: React.ReactNode; -} - -interface AccessorColumnDef<T extends DataTableTMin> extends ColumnDefBase<T> { - accessorKey: keyof T; - id?: string; -} - -interface DisplayColumnDef<T extends DataTableTMin> extends ColumnDefBase<T> { - accessorKey?: never; - id: string; -} - -export type ColumnDef<T extends DataTableTMin> = - AccessorColumnDef<T> | DisplayColumnDef<T>; - -const SKELETON_HEAD_WIDTHS = ["w-24", "w-16", "w-20", "w-14"]; -const SKELETON_CELL_WIDTHS = ["w-full", "w-3/4", "w-1/2", "w-5/6", "w-2/3"]; - -export const DataTableSkeleton = ({ - columns, - rows = 6, - toolbar = false, -}: { - columns: number; - rows?: number; - toolbar?: boolean; -}) => { - const headerIds = Array.from({ length: columns }, (_, i) => `s-head-${i}`); - const rowIds = Array.from({ length: rows }, (_, i) => `s-row-${i}`); - - return ( - <div className="space-y-4"> - {toolbar && ( - <div className="flex flex-col gap-2 sm:flex-row sm:items-center"> - <div className="flex-1"> - <Skeleton className="h-9 w-full" /> - </div> - </div> - )} - - <div className="[&>div]:rounded-md [&>div]:border"> - <Table className="min-w-full"> - <TableHeader className="bg-card"> - <TableRow> - {headerIds.map((hid, i) => ( - <TableHead key={hid}> - <Skeleton - className={cn( - "h-4", - SKELETON_HEAD_WIDTHS[i % SKELETON_HEAD_WIDTHS.length], - )} - /> - </TableHead> - ))} - </TableRow> - </TableHeader> - <TableBody> - {rowIds.map((rid, i) => ( - <TableRow className="h-9" key={rid}> - {headerIds.map((hid, j) => ( - <TableCell key={`${rid}-${hid}`}> - <Skeleton - className={cn( - "h-4", - SKELETON_CELL_WIDTHS[ - (i + j) % SKELETON_CELL_WIDTHS.length - ], - )} - /> - </TableCell> - ))} - </TableRow> - ))} - </TableBody> - </Table> - </div> - - <div className="flex w-full flex-col-reverse items-center justify-end gap-4 overflow-auto p-1 sm:flex-row sm:gap-8"> - <div className="flex flex-wrap items-center justify-center gap-4 sm:gap-6 lg:gap-8"> - <Skeleton className="h-8 w-[4.5rem]" /> - - <div className="flex items-center space-x-2"> - <Skeleton className="size-8" /> - <Skeleton className="size-8" /> - </div> - </div> - </div> - </div> - ); -}; - -export function DataTable<T extends DataTableTMin>( - props: Omit<React.ComponentProps<typeof Table>, "columns"> & - React.ComponentProps<typeof PaginationDataTable> & - React.ComponentProps<typeof SearchDataTable> & { - bulkActions?: React.ReactNode; - columns: ColumnDef<T>[]; - customNoResults?: { - description?: string; - footer?: React.ReactNode; - icon?: React.ReactNode; - title?: string; - }; - edges: T[]; - filters?: FilterDataTable[]; - id: string; - order: { - columns?: (keyof T)[]; - defaultOrder: { - column: keyof T; - order: "asc" | "desc"; - }; - }; - search?: boolean; - }, -) { +import { NextDataTableNavigation } from "./navigation-next"; + +export type { + AlignDataTable, + ColumnDef, + DataTableProps, + DataTableTMin, + SearchParamsDataTable, +} from "./data-table-content"; +export { DataTableSkeleton } from "./data-table-content"; + +/** + * {@link ContentDataTable}, wired to Next.js. + * + * The props are unchanged, so every AdminCP view and every `/files` page sees + * exactly the component they always did. This supplies the one thing the shared + * table cannot resolve for itself - how to change the URL - and the failure + * screen, which is `next-intl`'s locale-aware navigation wearing two buttons. + * + * The provider is a client component and the table it wraps is not: passing the + * table as `children` is what keeps it that way, so the `cell` functions in + * `columns` are called on the server and never have to cross a serialization + * boundary. + */ +export function DataTable<T extends DataTableTMin>(props: DataTableProps<T>) { if (!(props.edges && props.pageInfo)) { return <ErrorView code={500} />; } - return <ContentDataTable<T> {...props} />; + return ( + <NextDataTableNavigation> + <ContentDataTable<T> {...props} /> + </NextDataTableNavigation> + ); } diff --git a/packages/vitnode/src/components/table/filters.tsx b/packages/vitnode/src/components/table/filters.tsx index bab6afa98..ee583a92c 100644 --- a/packages/vitnode/src/components/table/filters.tsx +++ b/packages/vitnode/src/components/table/filters.tsx @@ -2,11 +2,9 @@ import { CheckIcon, PlusCircleIcon, Trash2 } from "lucide-react"; import { useTranslations } from "next-intl"; -import { useSearchParams } from "next/navigation"; import React from "react"; import { useDebouncedCallback } from "use-debounce"; -import { usePathname, useRouter } from "@/lib/navigation"; import { cn } from "@/lib/utils"; import { Badge } from "../ui/badge"; @@ -23,6 +21,8 @@ import { import { Popover, PopoverContent, PopoverTrigger } from "../ui/popover"; import { Separator } from "../ui/separator"; import { Spinner } from "../ui/spinner"; +import { useDataTableUrl } from "./navigation"; +import { readTableFilter, withTableFilter } from "./url-state"; export interface FilterOption { keywords?: string[]; @@ -39,18 +39,13 @@ export interface FilterDataTable { function FilterItem({ filter }: { filter: FilterDataTable }) { const t = useTranslations("core.global"); - const searchParams = useSearchParams(); - const pathname = usePathname(); - const { push } = useRouter(); - const [isPending, startTransition] = React.useTransition(); + const { isPending, navigate, searchParams } = useDataTableUrl(); const isAsync = Boolean(filter.onSearch); const [asyncOptions, setAsyncOptions] = React.useState<FilterOption[]>([]); const [isSearching, setIsSearching] = React.useState(false); - const selected = (searchParams.get(filter.id)?.split(",") ?? []).filter( - Boolean, - ); + const selected = readTableFilter(searchParams, filter.id); const selectedSet = new Set(selected); const options = isAsync ? asyncOptions : (filter.options ?? []); @@ -76,21 +71,7 @@ function FilterItem({ filter }: { filter: FilterDataTable }) { }; const applySelection = (values: string[]) => { - startTransition(() => { - const params = new URLSearchParams(searchParams.toString()); - - if (values.length) { - params.set(filter.id, values.join(",")); - } else { - params.delete(filter.id); - } - - params.delete("cursor"); - params.delete("first"); - params.delete("last"); - - push(`${pathname}?${params.toString()}`, { scroll: false }); - }); + navigate(withTableFilter(searchParams, { id: filter.id, values })); }; const toggle = (value: string) => { diff --git a/packages/vitnode/src/components/table/navigation-next.tsx b/packages/vitnode/src/components/table/navigation-next.tsx new file mode 100644 index 000000000..69ba9199e --- /dev/null +++ b/packages/vitnode/src/components/table/navigation-next.tsx @@ -0,0 +1,49 @@ +"use client"; + +import { useSearchParams } from "next/navigation"; +import React from "react"; + +import { usePathname, useRouter } from "@/lib/navigation"; + +import type { DataTableNavigation } from "./navigation"; + +import { DataTableNavigationProvider } from "./navigation"; + +/** + * {@link DataTableNavigationProvider}, wired to Next.js. + * + * The whole of the framework coupling the data table used to spread across four + * control components, in one place: the current search parameters, the pathname + * `next-intl` has already stripped the locale prefix from, and a push that does + * not scroll. `DataTable` mounts this, so every existing page keeps the + * behaviour it had without knowing anything changed. + * + * `usePathname` is the locale-aware one on purpose. `next/navigation`'s returns + * `/pl/files`, and pushing that through a router that prefixes the locale again + * gives `/pl/pl/files`. + */ +export const NextDataTableNavigation = ({ + children, +}: { + children: React.ReactNode; +}) => { + const searchParams = useSearchParams(); + const pathname = usePathname(); + const { push } = useRouter(); + + const value = React.useMemo<DataTableNavigation>( + () => ({ + navigate: nextSearch => { + push(`${pathname}?${nextSearch}`, { scroll: false }); + }, + searchParams, + }), + [pathname, push, searchParams], + ); + + return ( + <DataTableNavigationProvider value={value}> + {children} + </DataTableNavigationProvider> + ); +}; diff --git a/packages/vitnode/src/components/table/navigation.tsx b/packages/vitnode/src/components/table/navigation.tsx new file mode 100644 index 000000000..f8cd3e877 --- /dev/null +++ b/packages/vitnode/src/components/table/navigation.tsx @@ -0,0 +1,108 @@ +"use client"; + +import React from "react"; + +/** + * The one thing a `DataTable` cannot decide for itself. + * + * Every control in the table turns the current query string into a new one - a + * pure function, in `url-state.ts` - and then has to get the page there. That + * last step is the single question whose answer differs between the two + * frameworks: Next.js wants `next-intl`'s locale-aware router pointed at a + * pathname it has to look up, TanStack Start wants `router.navigate` and no + * pathname at all. Both can be expressed as "here is the search string, and + * here is a function that goes to it", so the table takes those two and stops + * caring. + * + * Deliberately two members and no more. A table needs to read its own search + * parameters and to replace them; it never needs a pathname, a locale, params, + * prefetching or history state, so widening this later is a decision somebody + * has to make on purpose rather than one that leaks in. It is not a router. + */ +export interface DataTableNavigation { + /** + * Goes to `nextSearch` - a query string with no leading `?`, exactly as + * `URLSearchParams.toString()` produces it. + * + * Implementations must not scroll: a person sorting the last column of a long + * table is looking at the header they clicked, and yanking them to the top of + * the page loses their place. + * + * Returning a promise is optional and only affects the pending indicator. + * Next's `push` resolves through the transition it was called in, so it + * returns nothing; a router whose `navigate` is awaitable should return it, so + * the spinner lasts as long as the navigation does instead of flashing. + */ + navigate: (nextSearch: string) => Promise<void> | void; + /** The query string the table is currently rendering. Never mutated. */ + searchParams: URLSearchParams; +} + +const DataTableNavigationContext = + React.createContext<DataTableNavigation | null>(null); + +/** + * Context rather than props, and not by preference. + * + * In Next.js the table is assembled by a Server Component: `DataTable` renders + * on the server, and a `navigate` function cannot cross that boundary as a prop. + * The controls that need it are client components several levels down, so the + * value has to be created on the client and read from there - the same shape + * `SelectionProviderDataTable` already uses, for the same reason. + */ +export const DataTableNavigationProvider = ({ + children, + value, +}: { + children: React.ReactNode; + value: DataTableNavigation; +}) => ( + <DataTableNavigationContext.Provider value={value}> + {children} + </DataTableNavigationContext.Provider> +); + +/** + * The seam, as a control sees it: where the table is, and how to move it. + * + * The transition is here rather than in each control because every one of them + * wants the same thing from it - a pending flag to swap a spinner in for while + * the next page is fetched - and because it is what keeps the old rows on + * screen instead of blanking the table mid-navigation. + * + * `navigate` is awaited inside the transition so that a router returning a + * promise keeps the control pending for the whole navigation. Awaiting a + * `void` return costs one microtask and changes nothing: the navigation itself + * was already started synchronously, inside the transition. + * + * The one it hands back returns nothing, deliberately. A control has no use for + * the navigation's promise - that is what `isPending` is for - and typing it as + * awaitable would make every call site a floating promise. + */ +export const useDataTableUrl = (): { + isPending: boolean; + navigate: (nextSearch: string) => void; + searchParams: URLSearchParams; +} => { + const navigation = React.use(DataTableNavigationContext); + + if (!navigation) { + throw new Error( + "A DataTable control must be rendered inside a DataTableNavigationProvider.", + ); + } + + const { navigate, searchParams } = navigation; + const [isPending, startTransition] = React.useTransition(); + + const navigateInTransition = React.useCallback( + (nextSearch: string) => { + startTransition(async () => { + await navigate(nextSearch); + }); + }, + [navigate], + ); + + return { isPending, navigate: navigateInTransition, searchParams }; +}; diff --git a/packages/vitnode/src/components/table/order-table-head.tsx b/packages/vitnode/src/components/table/order-table-head.tsx index 2805ee6ec..d49f81abc 100644 --- a/packages/vitnode/src/components/table/order-table-head.tsx +++ b/packages/vitnode/src/components/table/order-table-head.tsx @@ -1,41 +1,37 @@ "use client"; import { ArrowDown, ArrowUp, ChevronsUpDown } from "lucide-react"; -import { useSearchParams } from "next/navigation"; import React from "react"; -import { usePathname, useRouter } from "@/lib/navigation"; - -import type { DataTable, DataTableTMin } from "./data-table"; +import type { DataTableProps, DataTableTMin } from "./data-table-content"; import { Button } from "../ui/button"; import { Loader } from "../ui/loader"; +import { useDataTableUrl } from "./navigation"; +import { readTableOrder, toggleTableOrder } from "./url-state"; export function OrderTableHeadDataTable<T extends DataTableTMin>({ id, children, order: { defaultOrder }, -}: Pick<React.ComponentProps<typeof DataTable<T>>, "order"> & { +}: Pick<DataTableProps<T>, "order"> & { children: React.ReactNode; id: keyof T; }) { - const [isPending, startTransition] = React.useTransition(); - const searchParams = useSearchParams(); - const pathname = usePathname(); - const { push } = useRouter(); - - const currentOrderBy = - searchParams.get("orderBy") ?? defaultOrder.column.toString(); - const currentOrder = searchParams.get("order") ?? defaultOrder.order; - - const isActive = currentOrderBy === id.toString(); - const nextOrder = isActive && currentOrder === "asc" ? "desc" : "asc"; + const { isPending, navigate, searchParams } = useDataTableUrl(); + const column = id.toString(); + const fallback = { + column: defaultOrder.column.toString(), + order: defaultOrder.order, + }; + const current = readTableOrder(searchParams, fallback); + const isActive = current.column === column; let icon: React.ReactNode; if (isPending) { icon = <Loader small />; } else if (isActive) { - icon = currentOrder === "asc" ? <ArrowUp /> : <ArrowDown />; + icon = current.order === "asc" ? <ArrowUp /> : <ArrowDown />; } else { icon = <ChevronsUpDown />; } @@ -44,14 +40,9 @@ export function OrderTableHeadDataTable<T extends DataTableTMin>({ <Button className="[&_svg]:text-muted-foreground -ml-2 flex h-8 items-center gap-1.5 rounded-md px-2 py-1.5 [&_svg]:size-4 [&_svg]:shrink-0" onClick={() => { - startTransition(() => { - const params = new URLSearchParams(searchParams.toString()); - params.set("orderBy", id.toString()); - params.set("order", nextOrder); - push(`${pathname}?${params.toString()}`, { - scroll: false, - }); - }); + navigate( + toggleTableOrder(searchParams, { column, defaultOrder: fallback }), + ); }} size="sm" variant="ghost" diff --git a/packages/vitnode/src/components/table/pagination.tsx b/packages/vitnode/src/components/table/pagination.tsx index 461ca4e21..f97795376 100644 --- a/packages/vitnode/src/components/table/pagination.tsx +++ b/packages/vitnode/src/components/table/pagination.tsx @@ -2,11 +2,8 @@ import { ChevronLeftIcon, ChevronRightIcon } from "lucide-react"; import { useTranslations } from "next-intl"; -import { useSearchParams } from "next/navigation"; import React from "react"; -import { usePathname, useRouter } from "@/lib/navigation"; - import { Button } from "../ui/button"; import { Select, @@ -16,6 +13,12 @@ import { SelectValue, } from "../ui/select"; import { Skeleton } from "../ui/skeleton"; +import { useDataTableUrl } from "./navigation"; +import { + readTablePageSize, + withTablePage, + withTablePageSize, +} from "./url-state"; const PAGE_SIZE_OPTIONS = [10, 20, 30, 40]; @@ -32,16 +35,8 @@ export const PaginationDataTable = ({ }; }) => { const t = useTranslations("core.global"); - const { push } = useRouter(); - const [isPending, startTransition] = React.useTransition(); - const pathname = usePathname(); - const searchParams = useSearchParams(); - const pagination = { - first: searchParams.get("first"), - last: searchParams.get("last"), - cursor: searchParams.get("cursor"), - }; - const pageSize = pagination.first ?? pagination.last ?? 10; + const { isPending, navigate, searchParams } = useDataTableUrl(); + const pageSize = readTablePageSize(searchParams); return ( <div className="flex w-full flex-col-reverse items-center justify-end gap-4 overflow-auto p-1 sm:flex-row sm:gap-8"> @@ -52,15 +47,7 @@ export const PaginationDataTable = ({ if (value == null) { return; } - startTransition(() => { - const params = new URLSearchParams(searchParams.toString()); - params.set("first", value as string); - params.delete("last"); - params.delete("cursor"); - push(`${pathname}?${params.toString()}`, { - scroll: false, - }); - }); + navigate(withTablePageSize(searchParams, value as string)); }} value={`${pageSize}`} > @@ -89,19 +76,13 @@ export const PaginationDataTable = ({ className="bg-card size-8" disabled={!hasPreviousPage} onClick={() => { - startTransition(() => { - const params = new URLSearchParams(searchParams.toString()); - params.set("last", `${Number(pageSize)}`); - if (startCursor) { - params.set("cursor", startCursor); - } else { - params.delete("cursor"); - } - params.delete("first"); - push(`${pathname}?${params.toString()}`, { - scroll: false, - }); - }); + navigate( + withTablePage(searchParams, { + cursor: startCursor, + direction: "previous", + pageSize, + }), + ); }} size="icon" variant="outline" @@ -118,19 +99,13 @@ export const PaginationDataTable = ({ className="bg-card size-8" disabled={!hasNextPage || isPending} onClick={() => { - startTransition(() => { - const params = new URLSearchParams(searchParams.toString()); - params.set("first", `${Number(pageSize)}`); - if (endCursor) { - params.set("cursor", endCursor); - } else { - params.delete("cursor"); - } - params.delete("last"); - push(`${pathname}?${params.toString()}`, { - scroll: false, - }); - }); + navigate( + withTablePage(searchParams, { + cursor: endCursor, + direction: "next", + pageSize, + }), + ); }} size="icon" variant="outline" diff --git a/packages/vitnode/src/components/table/search.tsx b/packages/vitnode/src/components/table/search.tsx index d537dc214..eb096ff45 100644 --- a/packages/vitnode/src/components/table/search.tsx +++ b/packages/vitnode/src/components/table/search.tsx @@ -2,18 +2,17 @@ import { Search } from "lucide-react"; import { useTranslations } from "next-intl"; -import { useSearchParams } from "next/navigation"; import React from "react"; import { useDebouncedCallback } from "use-debounce"; -import { usePathname, useRouter } from "@/lib/navigation"; - import { InputGroup, InputGroupAddon, InputGroupInput, } from "../ui/input-group"; import { Spinner } from "../ui/spinner"; +import { useDataTableUrl } from "./navigation"; +import { readTableSearch, withTableSearch } from "./url-state"; export function SearchDataTable({ searchPlaceholder, @@ -21,13 +20,10 @@ export function SearchDataTable({ searchPlaceholder?: string; }) { const t = useTranslations("core.global"); - const searchParams = useSearchParams(); - const searchValue = searchParams.get("search") ?? ""; + const { isPending, navigate, searchParams } = useDataTableUrl(); + const searchValue = readTableSearch(searchParams); const [value, setValue] = React.useState(searchValue); const [prevSearchValue, setPrevSearchValue] = React.useState(searchValue); - const [isPending, startTransition] = React.useTransition(); - const pathname = usePathname(); - const { push } = useRouter(); if (searchValue !== prevSearchValue) { setPrevSearchValue(searchValue); @@ -35,17 +31,7 @@ export function SearchDataTable({ } const handleChangeSearch = useDebouncedCallback((value: string) => { - startTransition(() => { - const params = new URLSearchParams(searchParams.toString()); - - if (value.length >= 3) { - params.set("search", value); - } else { - params.delete("search"); - } - - push(`${pathname}?${params.toString()}`, { scroll: false }); - }); + navigate(withTableSearch(searchParams, value)); }, 500); return ( diff --git a/packages/vitnode/src/components/table/table-boundaries.test.ts b/packages/vitnode/src/components/table/table-boundaries.test.ts new file mode 100644 index 000000000..585f3677a --- /dev/null +++ b/packages/vitnode/src/components/table/table-boundaries.test.ts @@ -0,0 +1,254 @@ +// @vitest-environment node +import { existsSync, readFileSync, statSync } from "node:fs"; +import { dirname, join, relative, resolve } from "node:path"; +import { fileURLToPath } from "node:url"; +import { describe, expect, it } from "vitest"; + +const here = dirname(fileURLToPath(import.meta.url)); +const srcRoot = resolve(here, "../.."); + +/** + * The data table, split down the middle. + * + * The same boundary `feed-boundaries.test.ts` and `auth-boundaries.test.ts` + * draw, with the same machinery and for the same reason: a shared component + * that reaches `@/lib/navigation` - or anything else built on Next's request + * scope - cannot be rendered by a TanStack Start route, and nothing about that + * failure is visible until somebody tries. + * + * The table is the widest instance of it in the codebase. Four separate + * controls used to import Next's router directly, so every AdminCP screen and + * `/files` inherited the coupling from a header cell. + */ +const SHARED = { + content: join(here, "content.tsx"), + filters: join(here, "filters.tsx"), + orderHead: join(here, "order-table-head.tsx"), + pagination: join(here, "pagination.tsx"), + search: join(here, "search.tsx"), + seam: join(here, "navigation.tsx"), + skeletonAndTypes: join(here, "data-table-content.tsx"), + urlState: join(here, "url-state.ts"), +}; + +/** The Next.js half: locale-aware navigation, and the error screen built on it. */ +const NEXT_WRAPPERS = { + navigation: join(here, "navigation-next.tsx"), + table: join(here, "data-table.tsx"), +}; + +/** + * The specifier a `from "..."` resolves to, or `null` when it leaves the + * package. + */ +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 shared table imports its own + * props type from the module the Next.js wrapper lives beside, and that import + * is erased at compile time rather than reaching a bundle. + */ +const runtimeImports = (path: string): string[] => { + const source = readFileSync(path, "utf8").replace( + /(^|[\n;])\s*import\s+type\s[\s\S]*?from\s*["'][^"']+["']/g, + "$1", + ); + + return [ + ...source.matchAll( + /(?:^|[^\w$.])from\s*["']([^"']+)["']|import\s*\(\s*["']([^"']+)["']|(?:^|[\n;}])\s*import\s*["']([^"']+)["']/g, + ), + ] + .map(match => match[1] ?? match[2] ?? match[3]) + .filter((specifier): specifier is string => Boolean(specifier)); +}; + +/** Every external specifier reachable from an entry, with the chain that got there. */ +const externalGraph = (entry: string): Map<string, string[]> => { + const found = new Map<string, string[]>(); + const parents = new Map<string, string>(); + const seen = new Set<string>(); + + const chain = (file: string): string => { + const parts: string[] = []; + for (let at: string | undefined = file; at; at = parents.get(at)) { + parts.unshift(relative(srcRoot, at)); + } + + return parts.join(" -> "); + }; + + const walk = (file: string) => { + if (seen.has(file)) return; + seen.add(file); + + for (const specifier of runtimeImports(file)) { + const target = resolveSpecifier(specifier, file); + + if (target) { + if (!parents.has(target)) parents.set(target, file); + walk(target); + continue; + } + + found.set(specifier, [...(found.get(specifier) ?? []), chain(file)]); + } + }; + + walk(entry); + + return found; +}; + +const matches = (specifier: string, forbidden: string): boolean => + specifier === forbidden || specifier.startsWith(`${forbidden}/`); + +const offenders = (entry: string, forbidden: string[]): string[] => + [...externalGraph(entry)] + .filter(([specifier]) => forbidden.some(one => matches(specifier, one))) + .flatMap(([specifier, chains]) => chains.map(at => `${specifier} in ${at}`)) + .sort(); + +/** Anything that only resolves inside a Next.js app. */ +const NEXT_ONLY = ["next", "server-only"]; + +/** + * `next-intl`'s Next-only halves. + * + * The root entry is deliberately absent: it re-exports `use-intl`, which is + * framework-free. These four 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", +]; + +const sharedEntries = Object.entries(SHARED).map(([name, path]) => ({ + name, + path, +})); + +describe("the import scan finds what it is looking for", () => { + // Every assertion below is a "found nothing" one, which a scanner that + // silently matches nothing also satisfies. The Next wrappers are the control: + // they provably import the things the shared table must not. + it("finds the Next-only imports in the Next wrappers", () => { + expect(offenders(NEXT_WRAPPERS.navigation, NEXT_ONLY)).not.toEqual([]); + expect(offenders(NEXT_WRAPPERS.navigation, NEXT_INTL_RUNTIME)).not.toEqual( + [], + ); + }); + + it("walks past the entry file into its dependencies", () => { + // `lib/navigation` is two hops from the table, not one: nothing in + // `data-table.tsx` imports `next-intl` itself. + expect( + offenders(NEXT_WRAPPERS.table, ["next-intl/navigation"]).join(), + ).toContain("data-table.tsx -> "); + expect(runtimeImports(NEXT_WRAPPERS.table)).not.toContain( + "next-intl/navigation", + ); + }); +}); + +describe("the shared data table is framework-neutral", () => { + it.each(sharedEntries)("$name reaches nothing from next/*", ({ path }) => { + expect(offenders(path, NEXT_ONLY)).toEqual([]); + }); + + it.each(sharedEntries)( + "$name reaches none of next-intl's Next-only entrypoints", + ({ path }) => { + expect(offenders(path, NEXT_INTL_RUNTIME)).toEqual([]); + }, + ); + + it.each(sharedEntries)( + "$name never reaches the locale-aware navigation module", + ({ path }) => { + const reached = [...externalGraph(path).keys()]; + + expect(reached.some(one => one.includes("lib/navigation"))).toBe(false); + }, + ); + + it("keeps the URL arithmetic free of every import", () => { + // The point of `url-state.ts`: no router, no React, nothing to mock. If an + // import ever appears here, the seam has started growing a second job. + expect(runtimeImports(SHARED.urlState)).toEqual([]); + }); +}); + +describe("the shared controls take their navigation from the seam", () => { + const withoutComments = (path: string): string => + readFileSync(path, "utf8") + .replace(/\/\*[\s\S]*?\*\//g, "") + .replace(/\/\/.*$/gm, ""); + + const controls = [ + SHARED.filters, + SHARED.orderHead, + SHARED.pagination, + SHARED.search, + ]; + + it("asks the seam where it is rather than a router", () => { + for (const path of controls) { + const code = withoutComments(path); + + expect(code).toContain("useDataTableUrl"); + expect(code).not.toContain("useSearchParams"); + expect(code).not.toContain("useRouter"); + expect(code).not.toContain("usePathname"); + } + }); + + it("builds no URLs of its own", () => { + // Every `new URLSearchParams(...)` in a control was a copy of the current + // search about to be edited by hand. That is `url-state.ts`'s job now, and + // a control that starts doing it again is a rule nobody can test. + for (const path of controls) { + expect(withoutComments(path)).not.toContain("new URLSearchParams"); + } + }); + + it("never names a pathname, because it is not given one", () => { + for (const path of controls) { + expect(withoutComments(path)).not.toContain("pathname"); + } + }); +}); + +describe("the Next wrapper keeps the Next-only pieces", () => { + it("is the only half that knows about next-intl navigation", () => { + expect( + offenders(NEXT_WRAPPERS.table, ["next-intl/navigation"]), + ).not.toEqual([]); + expect(offenders(SHARED.content, ["next-intl/navigation"])).toEqual([]); + }); + + it("is where the scroll-free push and the locale-aware pathname live", () => { + const code = readFileSync(NEXT_WRAPPERS.navigation, "utf8"); + + expect(code).toContain("scroll: false"); + expect(code).toContain('from "@/lib/navigation"'); + }); +}); diff --git a/packages/vitnode/src/components/table/url-state.test.ts b/packages/vitnode/src/components/table/url-state.test.ts new file mode 100644 index 000000000..ea9542653 --- /dev/null +++ b/packages/vitnode/src/components/table/url-state.test.ts @@ -0,0 +1,334 @@ +import { describe, expect, it } from "vitest"; + +import { + DEFAULT_TABLE_PAGE_SIZE, + readTableFilter, + readTableOrder, + readTablePageSize, + readTableSearch, + toggleTableOrder, + withTableFilter, + withTableOrder, + withTablePage, + withTablePageSize, + withTableSearch, +} from "./url-state"; + +/** What every control is handed: the sort, the page and something else's. */ +const FULL = "search=foo&page=3&tab=media&orderBy=name&order=asc&first=20"; + +const DEFAULT_ORDER = { column: "createdAt", order: "desc" } as const; + +describe("reading what the URL asks for", () => { + it("falls back to the table's own default order", () => { + expect(readTableOrder("", DEFAULT_ORDER)).toEqual(DEFAULT_ORDER); + expect(readTableOrder("tab=media", DEFAULT_ORDER)).toEqual(DEFAULT_ORDER); + }); + + it("prefers what the URL says over the default", () => { + expect(readTableOrder("orderBy=name&order=asc", DEFAULT_ORDER)).toEqual({ + column: "name", + order: "asc", + }); + }); + + it("takes half an order from the URL and half from the default", () => { + expect(readTableOrder("orderBy=name", DEFAULT_ORDER)).toEqual({ + column: "name", + order: "desc", + }); + expect(readTableOrder("order=asc", DEFAULT_ORDER)).toEqual({ + column: "createdAt", + order: "asc", + }); + }); + + it("reads the page size from whichever direction is paging", () => { + expect(readTablePageSize("first=20")).toBe(20); + expect(readTablePageSize("last=30")).toBe(30); + }); + + it("falls back when there is no page size, or a nonsensical one", () => { + expect(readTablePageSize("")).toBe(DEFAULT_TABLE_PAGE_SIZE); + expect(readTablePageSize("first=abc")).toBe(DEFAULT_TABLE_PAGE_SIZE); + expect(readTablePageSize("first=0")).toBe(DEFAULT_TABLE_PAGE_SIZE); + expect(readTablePageSize("first=-5")).toBe(DEFAULT_TABLE_PAGE_SIZE); + }); + + it("reads the search box back out of the URL", () => { + expect(readTableSearch(FULL)).toBe("foo"); + expect(readTableSearch("tab=media")).toBe(""); + }); + + it("reads a filter's comma-separated values, ignoring empty ones", () => { + expect(readTableFilter("roles=1,2,3", "roles")).toEqual(["1", "2", "3"]); + expect(readTableFilter("roles=", "roles")).toEqual([]); + expect(readTableFilter("roles=1,,2", "roles")).toEqual(["1", "2"]); + expect(readTableFilter("tab=media", "roles")).toEqual([]); + }); + + it("accepts a query string with or without its leading question mark", () => { + expect(readTableSearch("?search=foo")).toBe("foo"); + expect(readTablePageSize("?first=20")).toBe(20); + }); +}); + +describe("sorting", () => { + it("adds a sort to a URL that had none, keeping the rest", () => { + expect(withTableOrder("search=foo&page=3", DEFAULT_ORDER)).toBe( + "search=foo&page=3&orderBy=createdAt&order=desc", + ); + }); + + it("replaces a sort in place rather than appending a second one", () => { + expect( + withTableOrder("orderBy=name&order=asc&tab=media", { + column: "size", + order: "desc", + }), + ).toBe("orderBy=size&order=desc&tab=media"); + }); + + it("flips the column that is already sorted ascending", () => { + expect( + toggleTableOrder("orderBy=name&order=asc", { + column: "name", + defaultOrder: DEFAULT_ORDER, + }), + ).toBe("orderBy=name&order=desc"); + }); + + it("starts a descending column again at ascending", () => { + expect( + toggleTableOrder("orderBy=name&order=desc", { + column: "name", + defaultOrder: DEFAULT_ORDER, + }), + ).toBe("orderBy=name&order=asc"); + }); + + it("starts a different column at ascending", () => { + expect( + toggleTableOrder("orderBy=name&order=asc", { + column: "size", + defaultOrder: DEFAULT_ORDER, + }), + ).toBe("orderBy=size&order=asc"); + }); + + it("treats the default order as the one the URL is already on", () => { + // Nothing in the URL, so the table is sorted by `createdAt desc`; clicking + // that same header must start over at ascending rather than re-assert desc. + expect( + toggleTableOrder("", { + column: "createdAt", + defaultOrder: DEFAULT_ORDER, + }), + ).toBe("orderBy=createdAt&order=asc"); + }); + + it("keeps the search, the page and everything else", () => { + expect( + toggleTableOrder(FULL, { column: "size", defaultOrder: DEFAULT_ORDER }), + ).toBe("search=foo&page=3&tab=media&orderBy=size&order=asc&first=20"); + }); + + it("does not move the visitor back to the first page", () => { + // Sorting has never reset the cursor, and the API answers a cursor against + // the new order rather than the old one. + const next = new URLSearchParams( + toggleTableOrder("cursor=abc&first=20", { + column: "name", + defaultOrder: DEFAULT_ORDER, + }), + ); + + expect(next.get("cursor")).toBe("abc"); + expect(next.get("first")).toBe("20"); + }); +}); + +describe("changing the page size", () => { + it("asks for that many rows from the start", () => { + expect(withTablePageSize("cursor=abc&last=10&tab=media", 40)).toBe( + "tab=media&first=40", + ); + }); + + it("takes the raw value a select hands it", () => { + expect(withTablePageSize("", "20")).toBe("first=20"); + }); + + it("keeps the sort and the search", () => { + expect(withTablePageSize(FULL, 40)).toBe( + "search=foo&page=3&tab=media&orderBy=name&order=asc&first=40", + ); + }); +}); + +describe("paging", () => { + it("steps forwards from the end cursor", () => { + expect( + withTablePage("tab=media", { + cursor: "end-1", + direction: "next", + pageSize: 20, + }), + ).toBe("tab=media&first=20&cursor=end-1"); + }); + + it("steps backwards from the start cursor", () => { + expect( + withTablePage("tab=media", { + cursor: "start-1", + direction: "previous", + pageSize: 20, + }), + ).toBe("tab=media&last=20&cursor=start-1"); + }); + + it("drops the direction it is no longer going", () => { + const forwards = new URLSearchParams( + withTablePage("last=20&cursor=start-1", { + cursor: "end-1", + direction: "next", + pageSize: 20, + }), + ); + + expect(forwards.get("first")).toBe("20"); + expect(forwards.has("last")).toBe(false); + + const backwards = new URLSearchParams( + withTablePage("first=20&cursor=end-1", { + cursor: "start-1", + direction: "previous", + pageSize: 20, + }), + ); + + expect(backwards.get("last")).toBe("20"); + expect(backwards.has("first")).toBe(false); + }); + + it("removes the cursor rather than writing an empty one", () => { + expect( + withTablePage("cursor=abc&first=20", { + cursor: null, + direction: "next", + pageSize: 20, + }), + ).toBe("first=20"); + }); + + it("keeps the sort, the search and unrelated parameters", () => { + expect( + withTablePage(FULL, { + cursor: "end-1", + direction: "next", + pageSize: 20, + }), + ).toBe( + "search=foo&page=3&tab=media&orderBy=name&order=asc&first=20&cursor=end-1", + ); + }); +}); + +describe("searching", () => { + it("writes a long enough term", () => { + expect(withTableSearch("tab=media", "foo")).toBe("tab=media&search=foo"); + }); + + it("removes the term rather than emptying it", () => { + expect(withTableSearch("search=foo&tab=media", "")).toBe("tab=media"); + expect(withTableSearch("search=foo&tab=media", "fo")).toBe("tab=media"); + }); + + it("replaces the term in place", () => { + expect(withTableSearch("search=foo&tab=media", "barbar")).toBe( + "search=barbar&tab=media", + ); + }); + + it("keeps the sort", () => { + const next = new URLSearchParams( + withTableSearch("orderBy=name&order=asc", "foo"), + ); + + expect(next.get("orderBy")).toBe("name"); + expect(next.get("order")).toBe("asc"); + }); + + it("encodes the way the URL always has", () => { + expect(withTableSearch("", "hello world")).toBe("search=hello+world"); + expect(withTableSearch("", "a&b=c")).toBe("search=a%26b%3Dc"); + }); +}); + +describe("filtering", () => { + it("joins the selected values into one parameter", () => { + expect( + withTableFilter("tab=media", { id: "roles", values: ["1", "2"] }), + ).toBe("tab=media&roles=1%2C2"); + }); + + it("removes the parameter when nothing is selected", () => { + expect( + withTableFilter("roles=1,2&tab=media", { id: "roles", values: [] }), + ).toBe("tab=media"); + }); + + it("returns to the first page, because the rows changed", () => { + expect( + withTableFilter("cursor=abc&first=20&last=10&tab=media", { + id: "roles", + values: ["1"], + }), + ).toBe("tab=media&roles=1"); + }); + + it("keeps the sort and the search", () => { + expect(withTableFilter(FULL, { id: "roles", values: ["1"] })).toBe( + "search=foo&page=3&tab=media&orderBy=name&order=asc&roles=1", + ); + }); + + it("survives a round trip through the reader", () => { + const values = ["1", "2", "3"]; + + expect( + readTableFilter(withTableFilter("", { id: "roles", values }), "roles"), + ).toEqual(values); + }); +}); + +describe("the helpers are pure", () => { + it("never mutates the params it was handed", () => { + const params = new URLSearchParams(FULL); + + withTableOrder(params, { column: "size", order: "desc" }); + withTablePageSize(params, 40); + withTablePage(params, { cursor: "x", direction: "next", pageSize: 40 }); + withTableSearch(params, "bar"); + withTableFilter(params, { id: "roles", values: ["1"] }); + + expect(params.toString()).toBe(FULL); + }); + + it("gives the same answer every time", () => { + const once = toggleTableOrder(FULL, { + column: "size", + defaultOrder: DEFAULT_ORDER, + }); + + expect( + toggleTableOrder(FULL, { column: "size", defaultOrder: DEFAULT_ORDER }), + ).toBe(once); + }); + + it("settles rather than drifting when applied to its own output", () => { + const once = withTableFilter(FULL, { id: "roles", values: ["1"] }); + + expect(withTableFilter(once, { id: "roles", values: ["1"] })).toBe(once); + }); +}); diff --git a/packages/vitnode/src/components/table/url-state.ts b/packages/vitnode/src/components/table/url-state.ts new file mode 100644 index 000000000..99abeceed --- /dev/null +++ b/packages/vitnode/src/components/table/url-state.ts @@ -0,0 +1,235 @@ +/** + * Every URL a `DataTable` control can ask for, as plain functions. + * + * A sort header, a page button, the search box and a filter dropdown all do the + * same thing: take the query string the page is on, change one thing about it, + * and hand the result to whatever knows how to navigate. Only that last step + * differs between Next.js and TanStack Start - the rest is string arithmetic, + * so it lives here, framework-free and testable without a router. + * + * Two rules hold everywhere below, because the table's URLs are also its API + * arguments and a control that forgot one would silently drop the visitor's + * work: + * + * - **Unrelated parameters survive.** Every helper copies the search it was + * given and edits the copy, so a plugin's own `?tab=` outlives a sort click. + * - **A parameter is removed rather than emptied.** `?search=` and `?cursor=` + * with no value are not the same request as their absence, and the API reads + * the presence of a cursor to decide which page it is on. + * + * Which parameters *reset* is deliberately uneven, and matches what the table + * has always done: paging and filtering rewrite the cursor, sorting and + * searching leave it alone. + */ + +/** The parameter a page size is written to when paging forwards. */ +const FIRST = "first"; +/** The parameter a page size is written to when paging backwards. */ +const LAST = "last"; +/** The row the next page starts from. Meaningless without `first` or `last`. */ +const CURSOR = "cursor"; + +/** + * The page size a table shows when the URL does not ask for one. + * + * Also the fallback for a `first`/`last` that is not a positive number: the + * value goes straight back into the URL when the visitor pages, and `?first=NaN` + * is not a request the API can answer. + */ +export const DEFAULT_TABLE_PAGE_SIZE = 10; + +/** + * How much has to be typed before a search reaches the URL. + * + * Below it the parameter is removed instead, so backspacing to nothing restores + * the unfiltered table rather than searching for an empty string. + */ +export const MIN_TABLE_SEARCH_LENGTH = 3; + +export type TableOrderDirection = "asc" | "desc"; + +/** Whatever the caller has: a raw query string, or the params it parsed. */ +export type TableSearch = string | URLSearchParams; + +export interface TableOrder { + column: string; + order: TableOrderDirection; +} + +/** + * A copy of the given search, safe to edit. + * + * Copying is the point: callers hand in the params object the router owns - + * Next's is frozen and throws on `set` - and every helper here returns a new + * string rather than mutating what it was passed. + */ +const copy = (search: TableSearch): URLSearchParams => + new URLSearchParams(search.toString()); + +/** Removes the whole cursor triplet, so the result is "page one" again. */ +const resetPagination = (params: URLSearchParams): void => { + params.delete(CURSOR); + params.delete(FIRST); + params.delete(LAST); +}; + +/** The column and direction the table is sorted by right now. */ +export const readTableOrder = ( + search: TableSearch, + defaultOrder: TableOrder, +): TableOrder => { + const params = copy(search); + + return { + column: params.get("orderBy") ?? defaultOrder.column, + order: + (params.get("order") as null | TableOrderDirection) ?? defaultOrder.order, + }; +}; + +/** Sorts by `column` in `order`, leaving the current page alone. */ +export const withTableOrder = ( + search: TableSearch, + { column, order }: TableOrder, +): string => { + const params = copy(search); + params.set("orderBy", column); + params.set("order", order); + + return params.toString(); +}; + +/** + * What clicking a sort header does. + * + * A column that is already sorted ascending flips to descending; anything else + * - a different column, or the same one descending - starts again at ascending, + * which is what makes a third click on the same header undo the second. + */ +export const toggleTableOrder = ( + search: TableSearch, + { column, defaultOrder }: { column: string; defaultOrder: TableOrder }, +): string => { + const current = readTableOrder(search, defaultOrder); + const isActive = current.column === column; + + return withTableOrder(search, { + column, + order: isActive && current.order === "asc" ? "desc" : "asc", + }); +}; + +/** How many rows the URL is asking for, whichever direction it is paging. */ +export const readTablePageSize = (search: TableSearch): number => { + const params = copy(search); + const size = Number(params.get(FIRST) ?? params.get(LAST)); + + return Number.isInteger(size) && size > 0 ? size : DEFAULT_TABLE_PAGE_SIZE; +}; + +/** + * Shows `pageSize` rows, from the beginning. + * + * The cursor goes with the old page size: a cursor is a position in a result + * set the visitor is no longer looking at, and keeping it would land them + * somewhere they never asked to be. + */ +export const withTablePageSize = ( + search: TableSearch, + pageSize: number | string, +): string => { + const params = copy(search); + params.set(FIRST, `${pageSize}`); + params.delete(LAST); + params.delete(CURSOR); + + return params.toString(); +}; + +/** + * Steps one page forwards or backwards from `cursor`. + * + * Direction is the parameter name: `first` reads forwards from the cursor, + * `last` reads backwards from it, and the two are mutually exclusive, so the + * one not being used is removed rather than left behind to contradict it. + * + * A missing cursor - the API returns none for an empty page - removes the + * parameter instead of writing `?cursor=`, which lands on the first page. + */ +export const withTablePage = ( + search: TableSearch, + { + cursor, + direction, + pageSize, + }: { + cursor: null | string; + direction: "next" | "previous"; + pageSize: number | string; + }, +): string => { + const params = copy(search); + const [take, drop] = direction === "next" ? [FIRST, LAST] : [LAST, FIRST]; + + params.set(take, `${Number(pageSize)}`); + + if (cursor) { + params.set(CURSOR, cursor); + } else { + params.delete(CURSOR); + } + + params.delete(drop); + + return params.toString(); +}; + +/** What the search box should show for the URL it is on. */ +export const readTableSearch = (search: TableSearch): string => + copy(search).get("search") ?? ""; + +/** + * Searches for `value`, or stops searching when it is too short. + * + * The page is deliberately *not* reset. It never has been, and the table reads + * a cursor the API hands back rather than a page number, so a stale one is + * corrected by the next response instead of being guessed at here. + */ +export const withTableSearch = (search: TableSearch, value: string): string => { + const params = copy(search); + + if (value.length >= MIN_TABLE_SEARCH_LENGTH) { + params.set("search", value); + } else { + params.delete("search"); + } + + return params.toString(); +}; + +/** The values a faceted filter currently has selected. */ +export const readTableFilter = (search: TableSearch, id: string): string[] => + (copy(search).get(id)?.split(",") ?? []).filter(Boolean); + +/** + * Selects `values` for one filter, and returns to the first page. + * + * Unlike sorting, a filter changes *which* rows exist, so the cursor the + * visitor was holding points into a result set that no longer exists. + */ +export const withTableFilter = ( + search: TableSearch, + { id, values }: { id: string; values: string[] }, +): string => { + const params = copy(search); + + if (values.length) { + params.set(id, values.join(",")); + } else { + params.delete(id); + } + + resetPagination(params); + + return params.toString(); +}; diff --git a/packages/vitnode/src/lib/files/in-use.ts b/packages/vitnode/src/lib/files/in-use.ts index e23008da4..1e23ee82f 100644 --- a/packages/vitnode/src/lib/files/in-use.ts +++ b/packages/vitnode/src/lib/files/in-use.ts @@ -1,6 +1,33 @@ -import type { StorageFileInUseBody } from "@/api/models/storage"; +/** + * Why {@link StorageModel.deleteFile} refused. + * + * Declared here rather than in `@/api/models/storage`, which is where it used to + * live and which still re-exports it, because {@link readFileInUse} runs in a + * browser. Importing the constant from the storage model pulled Hono, Drizzle + * and the whole `@/database` tree into the client bundle of every surface that + * deletes a file - a value import is a value import, however small the value. + * This module now has no runtime imports at all, which is the property that + * makes it safe from either framework. + */ +export const STORAGE_FILE_IN_USE = "FILE_IN_USE"; -import { STORAGE_FILE_IN_USE } from "@/api/models/storage"; +/** + * The body of that refusal, and the reason it is not just a code. + * + * "In use" covers two situations a person has to act on differently: content + * that would break, and history that would merely lose a restore. `content` is + * the one that is final; `revisions` is how many retained revisions hold the + * file, so a client can offer to force past them and say how much it is giving + * up. + */ +export interface StorageFileInUseBody { + code: typeof STORAGE_FILE_IN_USE; + /** A live content column or gallery row still points at this file. */ + content: boolean; + id: number; + /** Retained revisions pinning it - releasable with `force`. */ + revisions: number; +} /** * What a refused file delete was blocked by, as the two surfaces need it. @@ -15,7 +42,15 @@ export interface FileInUse { revisions: number; } -/** What both file-delete server actions return. */ +/** + * What every file-delete path returns - the Next.js server actions and the + * browser mutation the TanStack Start app calls. + * + * A closed result rather than a thrown error, because all three refusals are + * ordinary answers a person acts on: `409` is offered as a confirmation, `404` + * is "already gone", and anything else is "try again". Only a transport failure + * rejects. + */ export interface DeleteFileResult { data?: true; error?: { diff --git a/packages/vitnode/src/locales/en.json b/packages/vitnode/src/locales/en.json index 772c6228c..54f19c006 100644 --- a/packages/vitnode/src/locales/en.json +++ b/packages/vitnode/src/locales/en.json @@ -58,6 +58,7 @@ "empty": "Nothing found yet.", "loadMore": "Load more", "loading": "Loading…", + "sortBy": "Sort by", "sort": { "relevance": "Relevance", "newest": "Newest", diff --git a/packages/vitnode/src/views/files/actions/file-row-actions.tsx b/packages/vitnode/src/views/files/actions/file-row-actions.tsx index 9c88cdf45..820a52cf3 100644 --- a/packages/vitnode/src/views/files/actions/file-row-actions.tsx +++ b/packages/vitnode/src/views/files/actions/file-row-actions.tsx @@ -1,9 +1,9 @@ "use client"; import { DownloadIcon, LoaderCircleIcon, Trash2Icon } from "lucide-react"; -import { useTranslations } from "next-intl"; import React from "react"; import { toast } from "sonner"; +import { useTranslations } from "use-intl"; import type { userFilesModule } from "@/api/modules/users/files/files.module"; import type { FileInUse } from "@/lib/files/in-use"; @@ -14,14 +14,31 @@ import { TooltipWithContent } from "@/components/ui/tooltip"; import { CONFIG_PLUGIN } from "@/config"; import { clientModule, fetcherClient } from "@/lib/fetcher-client"; -import { deleteMyFileAction } from "./delete-action.server"; +import type { DeleteMyFile } from "../my-files-delete"; +/** + * Download and delete, for one row of the visitor's files. + * + * The delete arrives as a prop. It used to import the Next.js server action + * directly, which is what made this component - and therefore the whole table - + * unrenderable outside Next.js: importing a `"use server"` module pulls the + * fetcher, `next/headers` and the entire API module graph in behind it. The two + * frameworks end a delete differently (`revalidatePath` against a query + * invalidation) and agree on everything before that, so the last step is the + * prop and the rest is `my-files-delete.ts`. + * + * The download is *not* a prop, and the asymmetry is deliberate: it is a + * same-origin `GET` whose body becomes a blob in this browser, so there is + * nothing a server could do with it and nothing to invalidate afterwards. + */ export const MyFileRowActions = ({ id, name, + onDelete, }: { id: number; name: string; + onDelete: DeleteMyFile; }) => { const t = useTranslations("core.files"); const tGlobal = useTranslations("core.global.errors"); @@ -93,7 +110,7 @@ export const MyFileRowActions = ({ if (!open) setHeldByRevisions(null); }} onSubmit={async ({ onClose }) => { - const result = await deleteMyFileAction({ + const result = await onDelete({ force: heldByRevisions !== null, id, }); diff --git a/packages/vitnode/src/views/files/actions/files-bulk-actions.tsx b/packages/vitnode/src/views/files/actions/files-bulk-actions.tsx index 49dac2bb3..631b2aa5e 100644 --- a/packages/vitnode/src/views/files/actions/files-bulk-actions.tsx +++ b/packages/vitnode/src/views/files/actions/files-bulk-actions.tsx @@ -1,9 +1,9 @@ "use client"; import { Trash2Icon } from "lucide-react"; -import { useTranslations } from "next-intl"; import React from "react"; import { toast } from "sonner"; +import { useTranslations } from "use-intl"; import type { BulkDeleteFilesResult } from "@/lib/files/bulk-delete"; @@ -11,9 +11,27 @@ import { ConfirmActionAlertDialog } from "@/components/confirm-action/confirm-ac import { useDataTableSelection } from "@/components/table/selection"; import { Button } from "@/components/ui/button"; -import { deleteMyFilesAction } from "./delete-action.server"; +import type { DeleteMyFiles } from "../my-files-delete"; -export const MyFilesBulkActions = () => { +/** + * Delete, for every ticked row of the visitor's files. + * + * The two-pass shape is the point and it is unchanged: the first pass deletes + * what it can and comes back with the ids that only *history* is holding, and + * the dialog stays open offering to force past exactly those - never the whole + * selection again. Files a live page still points at are refused in both passes, + * because there is no version of "delete anyway" that leaves a published page + * unbroken. + * + * The run itself is a prop, for the reason set out in `my-files-delete.ts`: the + * accounting is shared and only the last step - revalidate or invalidate - + * belongs to a framework. + */ +export const MyFilesBulkActions = ({ + onDeleteFiles, +}: { + onDeleteFiles: DeleteMyFiles; +}) => { const t = useTranslations("core.files"); const tGlobal = useTranslations("core.global.errors"); const { selected } = useDataTableSelection(); @@ -52,7 +70,7 @@ export const MyFilesBulkActions = () => { if (!open) setHeldByRevisions([]); }} onSubmit={async ({ onClose }) => { - const result = await deleteMyFilesAction({ force: isForcing, ids }); + const result = await onDeleteFiles({ force: isForcing, ids }); report(result); if (!isForcing && result.heldByRevisions.length > 0) { diff --git a/packages/vitnode/src/views/files/files-boundaries.test.ts b/packages/vitnode/src/views/files/files-boundaries.test.ts new file mode 100644 index 000000000..8ed240092 --- /dev/null +++ b/packages/vitnode/src/views/files/files-boundaries.test.ts @@ -0,0 +1,260 @@ +// @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, "../.."); + +/** + * `/files`, split down the middle. + * + * The same boundary `auth-boundaries.test.ts` and `feed-boundaries.test.ts` + * draw, with the same machinery and for the same reason: a shared module that + * reaches `next/headers`, a server action or `@/lib/navigation` cannot be loaded + * by a TanStack Start route, and nothing about that failure is visible until + * somebody tries. A scan is the only way to state it, because the offending + * import is usually three files away from the one being written - this feature's + * was `next/dynamic`, inside the confirm dialog, behind the delete button. + */ +const SHARED = { + bulkActions: join(here, "actions/files-bulk-actions.tsx"), + deletes: join(here, "my-files-delete.ts"), + query: join(here, "my-files-query.ts"), + rowActions: join(here, "actions/file-row-actions.tsx"), + table: join(here, "my-files-table-content.tsx"), +}; + +/** The Next.js half: `next/headers`, `notFound`, and the server actions. */ +const NEXT_WRAPPER = join(here, "my-files-table-view.tsx"); + +const resolveSpecifier = (specifier: string, from: string): null | string => { + let base: string; + + if (specifier.startsWith("@/")) base = join(srcRoot, specifier.slice(2)); + else if (specifier.startsWith(".")) base = resolve(dirname(from), specifier); + else return null; + + for (const suffix of [".ts", ".tsx", "/index.ts", "/index.tsx"]) { + const candidate = base + suffix; + if (existsSync(candidate) && statSync(candidate).isFile()) return candidate; + } + + return existsSync(base) && statSync(base).isFile() ? base : null; +}; + +/** + * Every specifier a file imports **at runtime**. + * + * `import type` statements are stripped first: the query module imports the + * files API module's *type* to keep the fetcher's route literals inferring, and + * that module is a Hono server module. It is erased at compile time and never + * reaches a bundle, so counting it would fail this suite 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: it re-exports `use-intl`, which is + * framework-free, and `apps/web` already renders core components that import it. + * These four 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", +]; + +const sharedEntries = Object.entries(SHARED).map(([name, path]) => ({ + name, + path, +})); + +describe("the import scan finds what it is looking for", () => { + // Most assertions below are "found nothing" ones, which a scanner that + // silently matches nothing also satisfies. The Next wrapper is the control: it + // provably imports the things the shared modules must not. + it("finds the Next-only imports in the Next wrapper", () => { + expect(offenders(NEXT_WRAPPER, NEXT_ONLY)).not.toEqual([]); + }); + + it("walks past the entry file into its dependencies", () => { + // `next/headers` is two hops from the wrapper - through `@/lib/fetcher` - + // not one. + expect(offenders(NEXT_WRAPPER, ["next/headers"]).join()).toContain( + "lib/fetcher.ts", + ); + }); +}); + +describe("the shared files modules are framework-neutral", () => { + it.each(sharedEntries)("$name reaches nothing from next/*", ({ path }) => { + // The table is in here too, which is only true because it renders + // `ContentDataTable`: `DataTable` mounts the Next.js navigation provider, + // and every one of the table's controls reads the URL through the seam in + // `components/table/navigation` instead of `next/navigation`. + expect(offenders(path, NEXT_ONLY)).toEqual([]); + }); + + it.each(sharedEntries)( + "$name reaches none of next-intl's Next-only entrypoints", + ({ path }) => { + expect(offenders(path, NEXT_INTL_RUNTIME)).toEqual([]); + }, + ); + + it.each(sharedEntries)( + "$name never reaches the locale-aware navigation module directly", + ({ path }) => { + const reached = [...externalGraph(path).keys()]; + + expect(reached.some(one => one.includes("next-intl/navigation"))).toBe( + false, + ); + }, + ); + + it.each(sharedEntries)("$name never reaches a server action", ({ path }) => { + // A `"use server"` module is the other way Next.js gets in: importing one + // pulls the fetcher, `next/headers` and the whole API module graph behind + // it. Both deletes are a prop instead. + const reached = [...externalGraph(path).keys()]; + + expect(reached.some(one => one.endsWith(".server"))).toBe(false); + expect(runtimeImports(path).some(one => one.includes(".server"))).toBe( + false, + ); + }); + + it("never imports the API's storage model for one string", () => { + // `readFileInUse` needs the `FILE_IN_USE` code, which used to live in + // `@/api/models/storage` - a value import that dragged Hono, Drizzle and + // `@/database` into the browser bundle of every surface that deletes a file. + const reached = [...externalGraph(SHARED.deletes).keys()]; + + expect(reached).not.toContain("drizzle-orm"); + expect(reached.some(one => one.startsWith("hono"))).toBe(false); + }); +}); + +describe("the shared table takes its framework parts as props", () => { + const withoutComments = (path: string): string => + readFileSync(path, "utf8") + .replace(/\/\*[\s\S]*?\*\//g, "") + .replace(/\/\/.*$/gm, ""); + + it("is handed a page rather than fetching one", () => { + const code = withoutComments(SHARED.table); + + expect(code).toContain("data: MyFilesPage;"); + expect(code).not.toContain("useQuery"); + expect(code).not.toContain("fetcher"); + }); + + it("is handed both deletes rather than calling a mutation", () => { + const code = withoutComments(SHARED.table); + + expect(code).toContain("onDeleteFile: DeleteMyFile;"); + expect(code).toContain("onDeleteFiles: DeleteMyFiles;"); + }); + + it("renders the framework-neutral table, not the Next.js one", () => { + // `DataTable` *is* the Next.js wiring - it mounts `NextDataTableNavigation`. + // The shared table renders `ContentDataTable` and leaves the provider to + // whoever is rendering it. + const code = withoutComments(SHARED.table); + + expect(code).toContain("ContentDataTable"); + expect(code).not.toContain("components/table/data-table"); + }); +}); + +describe("the Next wrapper keeps the Next-only pieces", () => { + it("is the only half that fetches, refuses and revalidates", () => { + const code = readFileSync(NEXT_WRAPPER, "utf8"); + + expect(code).toContain("notFound"); + expect(runtimeImports(NEXT_WRAPPER)).toContain("@/lib/fetcher"); + expect( + runtimeImports(NEXT_WRAPPER).some(one => + one.includes("delete-action.server"), + ), + ).toBe(true); + }); + + it("builds its request from the shared contract rather than its own", () => { + // The point of the split: a URL means the same thing in both apps because + // both call these two functions, not because two places look alike. + const code = readFileSync(NEXT_WRAPPER, "utf8"); + + expect(code).toContain("normalizeMyFilesParams"); + expect(code).toContain("myFilesRequest"); + }); +}); diff --git a/packages/vitnode/src/views/files/my-files-delete.ts b/packages/vitnode/src/views/files/my-files-delete.ts new file mode 100644 index 000000000..32792616d --- /dev/null +++ b/packages/vitnode/src/views/files/my-files-delete.ts @@ -0,0 +1,152 @@ +import type { BulkDeleteFilesResult } from "@/lib/files/bulk-delete"; +import type { DeleteFileResult } from "@/lib/files/in-use"; + +import { fetcherClient } from "@/lib/fetcher-client"; +import { runBulkFileDelete } from "@/lib/files/bulk-delete"; +import { readFileInUse } from "@/lib/files/in-use"; + +import { userFilesModuleRef } from "./my-files-query"; + +/** + * Deleting the visitor's own files, as a contract both frameworks satisfy. + * + * The API already accepts an authenticated `DELETE` from anywhere: it derives + * the owner from the session cookie and refuses a file that is not theirs. So + * the browser calls it directly - same origin, cookie attached by the browser + * itself - and there is deliberately no server function in between. A server + * function here would be a `POST` back to this app that then calls Hono, which + * is two round trips and a second place to get the semantics wrong, in exchange + * for nothing: this mutation needs no server-only secret, and it sets no cookie + * that would have to be copied onto a response. + * + * The Next.js app keeps its server actions, which is not a contradiction. There + * the delete has to end with `revalidatePath`, and that only exists on a server; + * see `actions/delete-action.server.ts`. What both sides share is the *shape* - + * the two callback types below, the 409 handling, and the bulk semantics - so + * one table component can be handed either. + */ + +/** Deleting one file. `force` releases retained revisions; see {@link FileInUse}. */ +export interface DeleteMyFileArgs { + force?: boolean; + id: number; +} + +/** Deleting a selection. The ids are exactly the rows that were ticked. */ +export interface DeleteMyFilesArgs { + force?: boolean; + ids: number[]; +} + +/** + * What the shared table is handed instead of a mutation. + * + * A plain async function returning a closed result, so a Next.js server action + * and a browser fetch are the same prop. Nothing framework-shaped survives in + * either direction. + */ +export type DeleteMyFile = ( + args: DeleteMyFileArgs, +) => Promise<DeleteFileResult>; + +export type DeleteMyFiles = ( + args: DeleteMyFilesArgs, +) => Promise<BulkDeleteFilesResult>; + +/** + * One delete, as arguments to whichever fetcher is carrying it. + * + * `force` is omitted rather than sent as `"false"`, so the URL of an ordinary + * delete says nothing about forcing at all - the route's schema accepts both, + * but a request that never mentions it cannot be misread by a proxy or a log. + */ +export const deleteMyFileRequest = ({ force = false, id }: DeleteMyFileArgs) => + ({ + args: { + params: { id: String(id) }, + query: force ? { force: "true" as const } : {}, + }, + method: "delete" as const, + module: "files" as const, + path: "/{id}" as const, + prefixPath: "/users", + }) as const; + +/** + * Deletes one of the visitor's files from the browser. + * + * Never rejects, and that is the contract rather than an oversight. Every way + * this can fail is something the person has to be told in the dialog they are + * standing in: a `409` is offered back as a confirmation, a `404` means somebody + * already deleted it, and anything else is "try again". A rejected promise would + * have to be caught by every caller to say the same thing, and `runBulkFileDelete` + * would have to catch it a second time. + * + * The `500` case is why the `catch` exists at all: `rawApiFetch` throws on those + * with the failing URL attached, and that throw is a server error like any other + * - reported as `status: 500`, not as a crashed dialog. + */ +export const deleteMyFileInBrowser: DeleteMyFile = async ({ + force = false, + id, +}) => { + try { + const response = await fetcherClient(userFilesModuleRef, { + ...deleteMyFileRequest({ force, id }), + options: { credentials: "include" }, + }); + + if (response.status !== 200) { + return { + error: { + inUse: await readFileInUse(response), + status: response.status, + }, + }; + } + + return { data: true }; + } catch { + // `rawApiFetch` throws on a 500 with the server's own error text, which has + // already been logged where a log belongs. The caller needs a status, not a + // stack. + return { error: { status: 500 } }; + } +}; + +/** + * Deletes a selection from the browser, one request per file. + * + * `runBulkFileDelete` is core's own, unchanged and shared with the Next.js + * server action: there is no bulk endpoint, so each id is the same single-file + * delete the row action calls, the per-file semantics stay identical, and the + * fan-out is capped. It never rejects and sorts the outcomes into the three + * refusals a person acts on differently. + */ +export const deleteMyFilesInBrowser: DeleteMyFiles = async ({ + force = false, + ids, +}) => + await runBulkFileDelete( + ids, + async id => await deleteMyFileInBrowser({ force, id }), + ); + +/** + * Whether a bulk run changed anything the table is showing. + * + * The rule the Next.js action already applies before it calls `revalidatePath`, + * lifted out so the TanStack Start app applies the identical one before it + * invalidates. A run that was refused outright leaves the page exactly as it + * was, and refetching would drop the selection that is showing which rows were + * kept - which is the only thing telling the person what to do next. + * + * Deliberately not "did anything happen": files blocked by content and files + * held by revisions are both *unchanged*, and both are reported in the dialog + * rather than by the table reloading underneath it. + */ +export const shouldRefreshAfterBulkDelete = ( + result: BulkDeleteFilesResult, +): boolean => result.deleted > 0; + +export type { BulkDeleteFilesResult, DeleteFileResult }; diff --git a/packages/vitnode/src/views/files/my-files-query.test.ts b/packages/vitnode/src/views/files/my-files-query.test.ts new file mode 100644 index 000000000..51020a4d3 --- /dev/null +++ b/packages/vitnode/src/views/files/my-files-query.test.ts @@ -0,0 +1,339 @@ +import { describe, expect, it } from "vitest"; + +import type { BulkDeleteFilesResult } from "@/lib/files/bulk-delete"; + +import { readFileInUse, STORAGE_FILE_IN_USE } from "@/lib/files/in-use"; + +import { + deleteMyFileRequest, + shouldRefreshAfterBulkDelete, +} from "./my-files-delete"; +import { + describeMyFilesParams, + isMyFilesRequestError, + MY_FILES_MAX_PAGE_SIZE, + MY_FILES_QUERY_ROOT, + myFilesQueryKey, + myFilesRequest, + MyFilesRequestError, + normalizeMyFilesParams, +} from "./my-files-query"; + +/** + * The pure half of the files contract. + * + * Everything below is a function over plain values: a URL becomes parameters, + * parameters become a request and a cache key, a response status becomes an + * error, and a bulk result becomes a yes-or-no about refreshing. Nothing here + * opens a socket or renders a component - the API has its own suite, and how the + * table looks is Playwright's. + */ + +describe("normalizeMyFilesParams", () => { + it("always asks for a page, so the size is never invented later", () => { + // The old code left this to `withPagination`, which wrote `first=10` inside + // the URL builder where the cache key could not see it. + expect(normalizeMyFilesParams()).toEqual({ first: "10" }); + expect(normalizeMyFilesParams({})).toEqual({ first: "10" }); + }); + + it("collapses the ways of spelling the same request into one", () => { + const key = JSON.stringify(normalizeMyFilesParams({ first: "10" })); + + expect(JSON.stringify(normalizeMyFilesParams({}))).toBe(key); + expect(JSON.stringify(normalizeMyFilesParams({ search: "" }))).toBe(key); + expect(JSON.stringify(normalizeMyFilesParams({ search: " " }))).toBe(key); + expect(JSON.stringify(normalizeMyFilesParams({ cursor: "" }))).toBe(key); + }); + + it("keeps a page size the API will accept, and repairs one it will not", () => { + expect(normalizeMyFilesParams({ first: "40" }).first).toBe("40"); + expect(normalizeMyFilesParams({ first: "abc" }).first).toBe("10"); + expect(normalizeMyFilesParams({ first: "0" }).first).toBe("10"); + expect(normalizeMyFilesParams({ first: "-5" }).first).toBe("10"); + expect(normalizeMyFilesParams({ first: "1.5" }).first).toBe("10"); + expect(normalizeMyFilesParams({ first: "9007199254740993" }).first).toBe( + "10", + ); + }); + + it("clamps to the largest page the API serves rather than letting it 400", () => { + expect(normalizeMyFilesParams({ first: "5000" }).first).toBe( + String(MY_FILES_MAX_PAGE_SIZE), + ); + expect(normalizeMyFilesParams({ last: "5000" }).last).toBe( + String(MY_FILES_MAX_PAGE_SIZE), + ); + }); + + it("pages backwards when the URL only asks for `last`", () => { + expect(normalizeMyFilesParams({ cursor: "abc", last: "20" })).toEqual({ + cursor: "abc", + last: "20", + }); + }); + + it("never sends both `first` and `last`, which the API refuses", () => { + const params = normalizeMyFilesParams({ first: "20", last: "30" }); + + expect(params).toEqual({ first: "20" }); + expect(params.last).toBeUndefined(); + }); + + it("falls back to a forward page when `last` is unusable", () => { + expect(normalizeMyFilesParams({ last: "nope" })).toEqual({ first: "10" }); + }); + + it("keeps a cursor that could be one and drops anything else", () => { + expect(normalizeMyFilesParams({ cursor: "aB9_-" }).cursor).toBe("aB9_-"); + expect( + normalizeMyFilesParams({ cursor: "not a cursor" }).cursor, + ).toBeUndefined(); + expect( + normalizeMyFilesParams({ cursor: "a".repeat(513) }).cursor, + ).toBeUndefined(); + }); + + it("only sorts by a column the route will sort by", () => { + expect(normalizeMyFilesParams({ order: "asc", orderBy: "name" })).toEqual({ + first: "10", + order: "asc", + orderBy: "name", + }); + expect( + normalizeMyFilesParams({ orderBy: "folder" }).orderBy, + ).toBeUndefined(); + expect( + normalizeMyFilesParams({ orderBy: "id; drop table" }).orderBy, + ).toBeUndefined(); + expect(normalizeMyFilesParams({ order: "sideways" }).order).toBeUndefined(); + }); + + it("trims a search and drops it when nothing is left", () => { + expect(normalizeMyFilesParams({ search: " cat.png " }).search).toBe( + "cat.png", + ); + expect(normalizeMyFilesParams({ search: "\t\n" }).search).toBeUndefined(); + }); + + it("takes the first value when a key is repeated in the query string", () => { + // Both routers surface `?orderBy=name&orderBy=size` as an array, and only + // one value can reach the API. + expect(normalizeMyFilesParams({ orderBy: ["name", "size"] }).orderBy).toBe( + "name", + ); + expect(normalizeMyFilesParams({ first: [] }).first).toBe("10"); + }); + + it("treats a `null` from URLSearchParams.get as absent", () => { + expect( + normalizeMyFilesParams({ + cursor: null, + first: null, + order: null, + orderBy: null, + search: null, + }), + ).toEqual({ first: "10" }); + }); +}); + +describe("myFilesRequest", () => { + it("names the route the list actually lives at", () => { + expect(myFilesRequest(normalizeMyFilesParams())).toEqual({ + args: { query: { first: "10" } }, + method: "get", + module: "files", + path: "/", + prefixPath: "/users", + }); + }); + + it("sends the normalised parameters and nothing else", () => { + const params = normalizeMyFilesParams({ + cursor: "abc", + orderBy: "size", + search: " report ", + unknown: "value", + } as Parameters<typeof normalizeMyFilesParams>[0]); + + expect(myFilesRequest(params).args.query).toEqual({ + cursor: "abc", + first: "10", + orderBy: "size", + search: "report", + }); + }); +}); + +describe("myFilesQueryKey", () => { + it("hangs off the root an invalidation can name", () => { + expect(myFilesQueryKey(normalizeMyFilesParams()).slice(0, 2)).toEqual([ + ...MY_FILES_QUERY_ROOT, + ]); + }); + + it("is the same key for two spellings of the same request", () => { + expect(myFilesQueryKey(normalizeMyFilesParams({ search: "" }))).toEqual( + myFilesQueryKey(normalizeMyFilesParams({ first: "10" })), + ); + }); + + it("is a different key for everything that changes the rows", () => { + const base = myFilesQueryKey(normalizeMyFilesParams()); + const differing = [ + { first: "40" }, + { cursor: "abc" }, + { orderBy: "name" }, + { order: "asc" }, + { search: "cat" }, + { last: "20" }, + ]; + + for (const raw of differing) { + expect(myFilesQueryKey(normalizeMyFilesParams(raw))).not.toEqual(base); + } + }); + + it("does not vary by language, because the rows do not", () => { + // Only the column headings are translated, and the renderer resolves those. + // A locale in the key would refetch an identical list on every switch. + expect( + JSON.stringify(myFilesQueryKey(normalizeMyFilesParams())), + ).not.toContain("locale"); + }); +}); + +describe("MyFilesRequestError", () => { + it("carries the status a caller has to tell apart", () => { + const error = new MyFilesRequestError(401, { first: "10" }); + + expect(error.status).toBe(401); + expect(error.params).toEqual({ first: "10" }); + }); + + it("says which page was being asked for", () => { + expect( + new MyFilesRequestError(429, { first: "10", search: "cat" }).message, + ).toContain("first=10, search=cat"); + expect(new MyFilesRequestError(500, {}).message).toContain("no filters"); + }); + + it("is recognised across two copies of the module", () => { + // `@vitnode/core` is imported from `dist` by the apps and from `src` by its + // own tests, so `instanceof` can answer `false` for a genuine one. + const fromAnotherCopy = Object.assign(new Error("..."), { + name: "MyFilesRequestError", + status: 403, + }); + + expect(isMyFilesRequestError(new MyFilesRequestError(403, {}))).toBe(true); + expect(isMyFilesRequestError(fromAnotherCopy)).toBe(true); + }); + + it("is not confused with an ordinary failure", () => { + expect(isMyFilesRequestError(new Error("network"))).toBe(false); + expect(isMyFilesRequestError({ status: 401 })).toBe(false); + expect(isMyFilesRequestError(null)).toBe(false); + }); +}); + +describe("describeMyFilesParams", () => { + it("says so when there is nothing to say", () => { + expect(describeMyFilesParams({})).toBe("no filters"); + }); +}); + +describe("deleteMyFileRequest", () => { + it("addresses one file by id", () => { + expect(deleteMyFileRequest({ id: 7 })).toEqual({ + args: { params: { id: "7" }, query: {} }, + method: "delete", + module: "files", + path: "/{id}", + prefixPath: "/users", + }); + }); + + it("mentions forcing only when it is forcing", () => { + expect(deleteMyFileRequest({ force: true, id: 7 }).args.query).toEqual({ + force: "true", + }); + expect(deleteMyFileRequest({ force: false, id: 7 }).args.query).toEqual({}); + }); +}); + +describe("readFileInUse", () => { + const body = (value: unknown, status = 409): Response => + new Response(JSON.stringify(value), { status }); + + it("reads the two things a refusal is made of", async () => { + await expect( + readFileInUse( + body({ + code: STORAGE_FILE_IN_USE, + content: false, + id: 1, + revisions: 3, + }), + ), + ).resolves.toEqual({ content: false, revisions: 3 }); + }); + + it("is not a reason unless the API said it was", async () => { + await expect(readFileInUse(body({}, 404))).resolves.toBeUndefined(); + await expect( + readFileInUse(body({ code: "SOMETHING_ELSE" })), + ).resolves.toBeUndefined(); + await expect(readFileInUse(body(null))).resolves.toBeUndefined(); + }); + + it("survives a proxy that answered with something else entirely", async () => { + const html = new Response("<html>gateway timeout</html>", { status: 409 }); + + await expect(readFileInUse(html)).resolves.toBeUndefined(); + }); + + it("defaults the fields a malformed body left out", async () => { + await expect( + readFileInUse(body({ code: STORAGE_FILE_IN_USE })), + ).resolves.toEqual({ content: false, revisions: 0 }); + }); +}); + +describe("shouldRefreshAfterBulkDelete", () => { + const result = ( + partial: Partial<BulkDeleteFilesResult>, + ): BulkDeleteFilesResult => ({ + blockedByContent: 0, + deleted: 0, + failed: 0, + heldByRevisions: [], + ...partial, + }); + + it("refreshes when rows actually went", () => { + expect(shouldRefreshAfterBulkDelete(result({ deleted: 1 }))).toBe(true); + }); + + it("leaves a refused run exactly as it was", () => { + // Refetching here would drop the selection that is showing which rows were + // kept, which is the only thing telling the person what to do next. + expect(shouldRefreshAfterBulkDelete(result({ blockedByContent: 3 }))).toBe( + false, + ); + expect( + shouldRefreshAfterBulkDelete(result({ heldByRevisions: [1, 2] })), + ).toBe(false); + expect(shouldRefreshAfterBulkDelete(result({ failed: 2 }))).toBe(false); + expect(shouldRefreshAfterBulkDelete(result({}))).toBe(false); + }); + + it("refreshes a partial run, because something did change", () => { + expect( + shouldRefreshAfterBulkDelete( + result({ blockedByContent: 1, deleted: 2, heldByRevisions: [9] }), + ), + ).toBe(true); + }); +}); diff --git a/packages/vitnode/src/views/files/my-files-query.ts b/packages/vitnode/src/views/files/my-files-query.ts new file mode 100644 index 000000000..817f1fd78 --- /dev/null +++ b/packages/vitnode/src/views/files/my-files-query.ts @@ -0,0 +1,388 @@ +import { queryOptions } from "@tanstack/react-query"; + +import type { userFilesModule } from "@/api/modules/users/files/files.module"; + +import { DEFAULT_TABLE_PAGE_SIZE } from "@/components/table/url-state"; +import { CONFIG_PLUGIN } from "@/config"; +import { clientModule, fetcherClient } from "@/lib/fetcher-client"; + +/** + * The signed-in visitor's own files, as one query definition. + * + * Everything about *what* that list is lives here and nowhere else: which URL + * parameters mean something, what a page is, how a refusal is told apart from a + * page, and the cache entry the whole thing lands in. A view renders whatever + * this produces and owns none of it. + * + * The split is the lesson `search-feed-query.ts` already paid for. When a + * component built one request and a loader built another, the two agreed on the + * cache key and on nothing else - so the server-rendered page came from one + * contract and every navigation after hydration came from a second one with + * different defaults and no status checking. 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 myFilesQueryOptions} takes a `fetchPage` and defaults it to the + * browser's, which is the only one a shared module can assume. + * + * ## Hono is still the boundary + * + * Nothing below authorizes anything. `GET /api/@vitnode/core/users/files` + * derives the owner from the session cookie and scopes the query to it, so a + * request this module builds for a visitor who has just been signed out comes + * back `401` - and {@link MyFilesRequestError} is what makes that a failed query + * rather than an empty table. + */ + +/** + * The files 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 userFilesModuleRef = clientModule<typeof userFilesModule>( + CONFIG_PLUGIN.pluginId, +); + +/** The module is mounted under `/users`, not at the plugin root. */ +const FILES_PREFIX_PATH = "/users"; + +/** The columns the list route will sort by. Anything else is ignored. */ +export const MY_FILES_ORDER_BY = ["createdAt", "name", "size"] as const; +export type MyFilesOrderBy = (typeof MY_FILES_ORDER_BY)[number]; + +export const MY_FILES_ORDER = ["asc", "desc"] as const; +export type MyFilesOrder = (typeof MY_FILES_ORDER)[number]; + +/** + * The largest page the API will serve, whatever the URL asks for. + * + * Mirrors `MAX_PAGE_SIZE` in `@/api/lib/with-pagination`. Clamping here as well + * as there is not redundancy: past this number the API answers `400`, and a + * table that 400s because somebody typed `?first=5000` is a broken page rather + * than a refused one. + */ +export const MY_FILES_MAX_PAGE_SIZE = 100; + +/** + * The list route's query, after normalisation - and therefore also the shape + * that identifies a cache entry. + * + * Every field is optional and every present field is known-good: this is what + * {@link normalizeMyFilesParams} produces and the only thing the request builder + * and the query key accept. + */ +export interface MyFilesParams { + cursor?: string; + first?: string; + last?: string; + order?: MyFilesOrder; + orderBy?: MyFilesOrderBy; + search?: string; +} + +/** + * The URL as either framework hands it over, before anything has checked it. + * + * Values are widened to `string[]` because a query string may repeat a key and + * both routers surface that as an array; they are widened to `null` because + * `URLSearchParams.get` returns one. None of it is trusted - see + * {@link normalizeMyFilesParams}. + */ +export type RawMyFilesParams = Partial< + Record<keyof MyFilesParams, null | string | string[] | undefined> +>; + +/** The first value for a key, since only one can reach the API. */ +const readOne = (value: null | string | string[] | undefined): string => { + if (Array.isArray(value)) return value[0] ?? ""; + + return value ?? ""; +}; + +/** + * A page size, or `undefined` when the URL did not ask for a usable one. + * + * `?first=abc`, `?first=0` and `?first=-1` are all refused by the API with a + * `400`, so accepting them here would turn a hand-edited URL into an error + * screen. They fall back to the default page size instead, which is the page the + * visitor would have got had they not edited anything. + */ +const readPageSize = (raw: string): string | undefined => { + if (!/^\d+$/.test(raw)) return undefined; + + const size = Number(raw); + if (!Number.isSafeInteger(size) || size < 1) return undefined; + + return String(Math.min(size, MY_FILES_MAX_PAGE_SIZE)); +}; + +/** + * The URL's parameters, reduced to the ones this list actually has. + * + * Pure, and the single place a raw query string becomes a request. Both + * frameworks call it - Next.js on `searchParams`, TanStack Start on the route's + * validated search - so a hand-edited URL behaves identically in both, and the + * cache key below is built from the *result* rather than from whatever was + * typed. That last part is what makes `?first=10`, `?first=010` and no `first` + * at all one cache entry rather than three. + * + * The rules, each of which exists because the alternative is a broken page: + * + * - **A page size is always present.** The fetcher used to lean on + * `withPagination`, which quietly wrote `first=10` inside the URL builder + * where the query key could not see it. Two requests that differed only in + * that invisible default shared a key. + * - **`first` wins over `last`.** They are mutually exclusive and the API + * `400`s on both; the table never emits both, so a URL that has them was + * written by hand and forward is the direction it would have meant. + * - **`search` is trimmed and dropped when empty.** The API trims it too and + * treats blank as no filter, so `?search=` and no `search` must not be two + * entries holding the same rows. + * - **A cursor is shape-checked only.** It is opaque and belongs to the + * ordering that minted it; whether it decodes is the API's business. A value + * that cannot be one is dropped rather than sent, since the API answers `400` + * for it and the honest reading of a corrupt cursor is "start again". + */ +export const normalizeMyFilesParams = ( + raw: RawMyFilesParams = {}, +): MyFilesParams => { + const params: MyFilesParams = {}; + + const cursor = readOne(raw.cursor); + if (/^[A-Za-z0-9_-]{1,512}$/.test(cursor)) params.cursor = cursor; + + const first = readPageSize(readOne(raw.first)); + const last = readPageSize(readOne(raw.last)); + + if (first !== undefined) { + params.first = first; + } else if (last === undefined) { + params.first = String(DEFAULT_TABLE_PAGE_SIZE); + } else { + params.last = last; + } + + const orderBy = readOne(raw.orderBy) as MyFilesOrderBy; + if (MY_FILES_ORDER_BY.includes(orderBy)) params.orderBy = orderBy; + + const order = readOne(raw.order) as MyFilesOrder; + if (MY_FILES_ORDER.includes(order)) params.order = order; + + const search = readOne(raw.search).trim(); + if (search) params.search = search; + + return params; +}; + +/** + * One page of the list, as arguments to whichever fetcher is carrying it. + * + * `withPagination` is deliberately absent. That flag makes the URL builder + * invent `first` and an empty `search` at the last possible moment, which is + * exactly the invisible state the query key cannot include - so the defaults are + * applied by {@link normalizeMyFilesParams} instead, in a value both the request + * and the key are built from. + */ +export const myFilesRequest = (params: MyFilesParams) => ({ + args: { query: params }, + method: "get" as const, + module: "files" as const, + path: "/" as const, + prefixPath: FILES_PREFIX_PATH, +}); + +/** One row of the table, as JSON delivers it. */ +export interface MyFile { + /** ISO string over the wire; a `Date` when a Next.js render passes it in. */ + createdAt: Date | string; + dimensions: null | { height: number; width: number }; + folder: string; + id: number; + metadata: Record<string, unknown>; + mimeType: null | string; + name: string; + size: number; + /** `null` when no storage adapter is configured, so there is nothing to link. */ + url: null | string; +} + +export interface MyFilesPage { + edges: MyFile[]; + pageInfo: { + count: number; + endCursor: null | string; + hasNextPage: boolean; + hasPreviousPage: boolean; + startCursor: null | string; + totalCount: number; + }; +} + +/** How a page is actually fetched. See {@link myFilesQueryOptions}. */ +export type MyFilesPageFetcher = ( + params: MyFilesParams, +) => Promise<MyFilesPage>; + +/** The `name` every {@link MyFilesRequestError} carries. See below. */ +const MY_FILES_REQUEST_ERROR = "MyFilesRequestError"; + +/** + * The parameters a failed request was carrying, for its message. + * + * Its own function because an error message is the only trace a production + * failure leaves, and "which page was it asking for" is the first question + * anyone reading one has. + */ +export const describeMyFilesParams = (params: MyFilesParams): string => + Object.entries(params) + .map(([key, value]) => `${key}=${value}`) + .join(", ") || "no filters"; + +/** + * The files API refused, and this is what it refused with. + * + * A thrown error rather than a returned one, because the alternative is the bug + * this class exists to prevent: the fetchers hand non-2xx responses back rather + * than throwing on them, and `json()` would happily parse a `401` or a `429` + * body. Read as a page it has no `edges`, so the table renders empty - a failure + * that looks exactly like an account with nothing uploaded. TanStack Query can + * only retry, report, or keep the last good page if the promise actually + * rejects. + * + * `status` is on the error rather than folded into the message so a caller can + * tell the finite cases apart without parsing English: `401` and `403` mean the + * session ended or was never allowed - the route guard is a navigation rule, not + * the boundary, so this is the *authorization* answer and it can arrive on a + * page the guard already let through. `429` is the rate limiter. `404` and `400` + * are a request nobody should have been able to build. A `500` never reaches + * here at all: `rawApiFetch` throws on those with the body attached. + * + * Recognised by `name` rather than by `instanceof`, and that is not fussiness. + * `@vitnode/core` is imported from `dist` by the apps and from `src` by its own + * tests, so two copies of this class can exist in one process and `instanceof` + * would answer `false` across them. + */ +export class MyFilesRequestError extends Error { + constructor(status: number, params: MyFilesParams) { + super( + `The files API answered ${status} for the current user's files (${describeMyFilesParams(params)}).`, + ); + this.name = MY_FILES_REQUEST_ERROR; + this.params = params; + this.status = status; + } + readonly params: MyFilesParams; + + readonly status: number; +} + +export const isMyFilesRequestError = ( + error: unknown, +): error is MyFilesRequestError => + error instanceof Error && error.name === MY_FILES_REQUEST_ERROR; + +/** + * One page, fetched from the browser. + * + * `fetcherClient` builds the same same-origin `/api/@vitnode/core/users/files` + * URL every other VitNode client call uses, so the browser attaches the session + * cookie itself and a `429` is routed to the global rate-limit notice on the way + * through. + */ +export const fetchMyFilesPageInBrowser: MyFilesPageFetcher = async params => { + const response = await fetcherClient( + userFilesModuleRef, + myFilesRequest(params), + ); + + if (!response.ok) throw new MyFilesRequestError(response.status, params); + + return await response.json(); +}; + +/** + * The root every cache entry for this list hangs off. + * + * Exported so an invalidation can name the whole family - one delete makes every + * page, sort and search of the visitor's own files stale, not just the one they + * are looking at. TanStack Query matches keys by prefix, so this invalidates + * exactly those and nothing else. + */ +export const MY_FILES_QUERY_ROOT = ["files", "me"] as const; + +/** + * The cache entry one page of the list reads and writes. + * + * The normalised parameters, and only those. Everything that changes which rows + * come back is in there - page, size, sort, search - and nothing that does not. + * + * The locale is deliberately absent. File names, folders, sizes and metadata are + * the visitor's own data and identical in every language; the only translated + * things on the page are the column headings, which the renderer resolves. Two + * entries holding identical rows would mean a language switch silently refetched + * a list that had not changed. + * + * An object in a key is safe - Query hashes keys structurally rather than by + * identity - which is exactly why the object has to be the *normalised* one. + */ +export const myFilesQueryKey = (params: MyFilesParams) => + [...MY_FILES_QUERY_ROOT, params] as const; + +/** + * The visitor's files, as the one query definition every caller shares. + * + * A route loader warms it before the component renders: + * + * context.queryClient.ensureQueryData(myFilesQueryOptions({ params })) + * + * and the component reads the very same options back: + * + * const { data } = useQuery(myFilesQueryOptions({ params })) + * + * Same key, same request, same status checking - so the loader's page is the + * page the component renders, and a delete that invalidates + * {@link MY_FILES_QUERY_ROOT} refetches through the identical contract. + * + * `fetchPage` is the seam. It defaults to the browser's fetcher, which is what a + * hydrated page wants; 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. + * + * ## It asks once + * + * `retry: false`, against Query's default of three attempts. Every failure this + * read can produce is made worse by repeating it: a `429` is answered by sending + * the same request two more times, which is the thing the limiter is asking this + * app to stop doing, and a `401` is not going to become a `200` because we asked + * again. A page that has genuinely gone wrong should say so on the first + * attempt, and the visitor retries by reloading - a decision they can make and a + * rate limiter can see coming. + * + * No `staleTime`. Freshness is whatever the API's own caching gives, plus + * VitNode's client defaults (`refetchOnMount` and `refetchOnWindowFocus` both + * off), so a hydrated table is not refetched behind the reader; a delete is what + * makes it stale, explicitly. + */ +export const myFilesQueryOptions = ({ + fetchPage = fetchMyFilesPageInBrowser, + params, +}: { + fetchPage?: MyFilesPageFetcher; + params: MyFilesParams; +}) => + queryOptions({ + queryFn: async () => await fetchPage(params), + queryKey: myFilesQueryKey(params), + retry: false, + }); + +/** + * What {@link MyFilesTableContent} accepts, and the reason it accepts only this. + * + * Typed as the factory's own return type on purpose: a caller cannot hand the + * table 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 MyFilesQueryOptions = ReturnType<typeof myFilesQueryOptions>; diff --git a/packages/vitnode/src/views/files/my-files-table-content.tsx b/packages/vitnode/src/views/files/my-files-table-content.tsx new file mode 100644 index 000000000..4e23d3881 --- /dev/null +++ b/packages/vitnode/src/views/files/my-files-table-content.tsx @@ -0,0 +1,178 @@ +"use client"; + +import { FileIcon, FolderIcon } from "lucide-react"; +import { useTranslations } from "use-intl"; + +import { DateFormat } from "@/components/date-format"; +import { FilePreview } from "@/components/files/file-preview"; +import { MetadataCell } from "@/components/files/metadata-cell"; +import { ContentDataTable } from "@/components/table/content"; +import { formatBytes } from "@/lib/format-bytes"; + +import type { DeleteMyFile, DeleteMyFiles } from "./my-files-delete"; +import type { MyFile, MyFilesPage } from "./my-files-query"; + +import { MyFileRowActions } from "./actions/file-row-actions"; +import { MyFilesBulkActions } from "./actions/files-bulk-actions"; + +/** + * The visitor's own files, as a table both frameworks render. + * + * Everything that used to make this a Next.js Server Component has been taken + * out and turned into a parameter: it no longer fetches, no longer calls + * `getTranslations`, and no longer reaches for `notFound()`. What is left is the + * part that was always worth sharing - the eight columns, the preview, the + * metadata popover, the relative date, the empty state, and which columns are + * sortable - and the three things it cannot resolve for itself arrive as props. + * + * Next.js my-files-table-view.tsx fetch + notFound + server actions + * TanStack Start routes/_authenticated/files loader + useQuery + browser deletes + * \ / + * MyFilesTableContent + * + * ## What it does not own + * + * **Fetching.** It is handed a page. Which page, and how it was fetched, is + * `my-files-query.ts`'s - the same definition a TanStack loader warms and a + * Next.js Server Component awaits. + * + * **Deleting.** Two callbacks, because the two frameworks genuinely differ: one + * ends in `revalidatePath`, the other in a query invalidation, and neither can + * be expressed in the other's runtime. What they share - the `409` handling, the + * force pass, the bulk accounting - is in `my-files-delete.ts` and in + * `lib/files/`, so the difference really is only the last line. + * + * **Navigating.** Sorting, paging and searching rewrite the URL, and this + * component never learns how. `ContentDataTable`'s controls read that from + * `DataTableNavigationProvider`, which the caller mounts - `DataTable` does it + * for Next.js, a TanStack route does it with `router.navigate`. Rendering + * `ContentDataTable` rather than `DataTable` is the whole of the difference: + * `DataTable` *is* the Next.js wiring. + * + * ## The one thing this costs Next.js + * + * `"use client"`, so the eight `cell` functions run in the browser rather than + * on the server as they did - which is what `DataTable` passing its table as + * `children` exists to preserve for the AdminCP's tables. It is paid here and + * nowhere else, and it is close to free on this particular table: `FilePreview`, + * `MetadataCell`, `DateFormat` and both action components were already client + * components, so the only thing that newly reaches the browser is the column + * list itself. The rendered HTML is unchanged, because Next.js server-renders + * client components too. + * + * The alternative was a second set of columns for Next.js to render on the + * server, which is the duplication this whole file exists to remove. + */ +export const MyFilesTableContent = ({ + data, + onDeleteFile, + onDeleteFiles, +}: { + data: MyFilesPage; + onDeleteFile: DeleteMyFile; + onDeleteFiles: DeleteMyFiles; +}) => { + const t = useTranslations("core.files"); + + return ( + <ContentDataTable<MyFile> + bulkActions={<MyFilesBulkActions onDeleteFiles={onDeleteFiles} />} + columns={[ + { + accessorKey: "url", + header: t("list.preview"), + className: "w-16", + cell: ({ row }) => ( + <FilePreview + mimeType={row.mimeType} + name={row.name} + url={row.url} + /> + ), + }, + { + accessorKey: "name", + header: t("list.name"), + cell: ({ row }) => ( + <div className="flex max-w-xs flex-col"> + <span className="truncate">{row.name}</span> + <p className="text-muted-foreground truncate text-sm"> + {row.mimeType ?? row.folder} + </p> + </div> + ), + }, + { + accessorKey: "folder", + header: t("list.folder"), + cell: ({ row }) => ( + <div className="text-muted-foreground flex items-center gap-2"> + <FolderIcon className="size-4 shrink-0" /> + <span className="truncate">{row.folder}</span> + </div> + ), + }, + { + accessorKey: "size", + header: t("list.size"), + cell: ({ row }) => formatBytes(row.size), + }, + { + accessorKey: "dimensions", + header: t("list.dimensions"), + cell: ({ row }) => + row.dimensions ? ( + `${row.dimensions.width}x${row.dimensions.height}` + ) : ( + <span className="text-muted-foreground">—</span> + ), + }, + { + accessorKey: "metadata", + header: t("list.metadata"), + cell: ({ row }) => ( + <MetadataCell + emptyLabel={t("metadata.empty")} + metadata={row.metadata} + title={t("metadata.title")} + /> + ), + }, + { + accessorKey: "createdAt", + header: t("list.createdAt"), + cell: ({ row }) => <DateFormat date={row.createdAt} />, + }, + { + id: "actions", + header: "", + align: "right", + className: "w-10", + cell: ({ row }) => ( + <MyFileRowActions + id={row.id} + name={row.name} + onDelete={onDeleteFile} + /> + ), + }, + ]} + customNoResults={{ + title: t("noResults.title"), + description: t("noResults.description"), + icon: <FileIcon />, + }} + edges={data.edges} + id="my-files-table" + order={{ + columns: ["name", "size", "createdAt"], + defaultOrder: { + column: "createdAt", + order: "desc", + }, + }} + pageInfo={data.pageInfo} + search + /> + ); +}; diff --git a/packages/vitnode/src/views/files/my-files-table-view.tsx b/packages/vitnode/src/views/files/my-files-table-view.tsx index 4753ba86d..c7660daa9 100644 --- a/packages/vitnode/src/views/files/my-files-table-view.tsx +++ b/packages/vitnode/src/views/files/my-files-table-view.tsx @@ -1,38 +1,51 @@ -import { FileIcon, FolderIcon } from "lucide-react"; -import { getTranslations } from "next-intl/server"; import { notFound } from "next/navigation"; import { userFilesModule } from "@/api/modules/users/files/files.module"; -import { DateFormat } from "@/components/date-format"; -import { FilePreview } from "@/components/files/file-preview"; -import { MetadataCell } from "@/components/files/metadata-cell"; -import { - DataTable, - type SearchParamsDataTable, -} from "@/components/table/data-table"; +import { NextDataTableNavigation } from "@/components/table/navigation-next"; import { fetcher } from "@/lib/fetcher"; -import { formatBytes } from "@/lib/format-bytes"; -import { MyFileRowActions } from "./actions/file-row-actions"; -import { MyFilesBulkActions } from "./actions/files-bulk-actions"; +import type { RawMyFilesParams } from "./my-files-query"; + +import { + deleteMyFileAction, + deleteMyFilesAction, +} from "./actions/delete-action.server"; +import { myFilesRequest, normalizeMyFilesParams } from "./my-files-query"; +import { MyFilesTableContent } from "./my-files-table-content"; +/** + * The Next.js half of `/files`: read the page, then hand it to the shared table. + * + * Everything Next.js about the feature is in this file. It is a Server + * Component, so it fetches with `fetcher()` - which reads the visitor's cookies + * through `next/headers` - and answers a refusal with `notFound()`, which only + * exists here. The two delete callbacks are the server actions, unchanged: they + * end in `revalidatePath`, which is how a Next.js page refreshes and is the one + * step that cannot be shared. + * + * The request itself is *not* Next.js's. `normalizeMyFilesParams` and + * `myFilesRequest` are the same two functions the TanStack Start loader calls, + * so a URL means the same thing in both apps rather than in two places that + * merely look alike. + * + * `NextDataTableNavigation` is mounted here rather than inherited from + * `DataTable`, because the shared table renders `ContentDataTable` - the half + * that has no idea how to change a URL. This is the same provider `DataTable` + * mounts, so sorting, paging and searching behave exactly as they did. + */ export const MyFilesTableView = async ({ searchParams, }: { - searchParams: Promise<SearchParamsDataTable>; + /** + * `RawMyFilesParams` rather than `SearchParamsDataTable`, which is what this + * used to say and which has no `search` key - the search box has always + * written one, and it reached the API only because Next.js hands the whole + * query string over at runtime whatever the type claims. + */ + searchParams: Promise<RawMyFilesParams>; }) => { - const query = await searchParams; - const [t, res] = await Promise.all([ - getTranslations("core.files"), - fetcher(userFilesModule, { - path: "/", - method: "get", - module: "files", - prefixPath: "/users", - args: { query }, - withPagination: true, - }), - ]); + const params = normalizeMyFilesParams(await searchParams); + const res = await fetcher(userFilesModule, myFilesRequest(params)); if (res.status !== 200) { return notFound(); @@ -41,98 +54,12 @@ export const MyFilesTableView = async ({ const data = await res.json(); return ( - <DataTable - bulkActions={<MyFilesBulkActions />} - columns={[ - { - accessorKey: "url", - header: t("list.preview"), - className: "w-16", - cell: ({ row }) => ( - <FilePreview - mimeType={row.mimeType} - name={row.name} - url={row.url} - /> - ), - }, - { - accessorKey: "name", - header: t("list.name"), - cell: ({ row }) => ( - <div className="flex max-w-xs flex-col"> - <span className="truncate">{row.name}</span> - <p className="text-muted-foreground truncate text-sm"> - {row.mimeType ?? row.folder} - </p> - </div> - ), - }, - { - accessorKey: "folder", - header: t("list.folder"), - cell: ({ row }) => ( - <div className="text-muted-foreground flex items-center gap-2"> - <FolderIcon className="size-4 shrink-0" /> - <span className="truncate">{row.folder}</span> - </div> - ), - }, - { - accessorKey: "size", - header: t("list.size"), - cell: ({ row }) => formatBytes(row.size), - }, - { - accessorKey: "dimensions", - header: t("list.dimensions"), - cell: ({ row }) => - row.dimensions ? ( - `${row.dimensions.width}x${row.dimensions.height}` - ) : ( - <span className="text-muted-foreground">—</span> - ), - }, - { - accessorKey: "metadata", - header: t("list.metadata"), - cell: ({ row }) => ( - <MetadataCell - emptyLabel={t("metadata.empty")} - metadata={row.metadata} - title={t("metadata.title")} - /> - ), - }, - { - accessorKey: "createdAt", - header: t("list.createdAt"), - cell: ({ row }) => <DateFormat date={row.createdAt} />, - }, - { - id: "actions", - header: "", - align: "right", - className: "w-10", - cell: ({ row }) => <MyFileRowActions id={row.id} name={row.name} />, - }, - ]} - customNoResults={{ - title: t("noResults.title"), - description: t("noResults.description"), - icon: <FileIcon />, - }} - edges={data.edges} - id="my-files-table" - order={{ - columns: ["name", "size", "createdAt"], - defaultOrder: { - column: "createdAt", - order: "desc", - }, - }} - pageInfo={data.pageInfo} - search - /> + <NextDataTableNavigation> + <MyFilesTableContent + data={data} + onDeleteFile={deleteMyFileAction} + onDeleteFiles={deleteMyFilesAction} + /> + </NextDataTableNavigation> ); }; diff --git a/packages/vitnode/src/views/search/feed-boundaries.test.ts b/packages/vitnode/src/views/search/feed-boundaries.test.ts index e4a43c160..e51a24e69 100644 --- a/packages/vitnode/src/views/search/feed-boundaries.test.ts +++ b/packages/vitnode/src/views/search/feed-boundaries.test.ts @@ -10,6 +10,18 @@ const srcRoot = resolve(here, "../.."); const SHARED_ENTRY = join(here, "search-feed-content.tsx"); const NEXT_WRAPPER = join(here, "search-feed.tsx"); +/** + * The search page's controls, and the Next wrapper they were split out of. + * + * The same boundary as the feed's, one level up and with more at stake: the + * controls render an input group, a native select and a row of buttons, so this + * is where a stray Next.js import inside the *design system* would show up. That + * is not hypothetical - `HeaderContent` was Next-only for one back button, and + * `use-captcha` made every `AutoForm` Next-only for one navigation import. + */ +const SHARED_CONTROLS = join(here, "search-controls-content.tsx"); +const NEXT_CONTROLS = join(here, "search-controls.tsx"); + /** * The other half of what a migrated feed page renders. * @@ -210,6 +222,55 @@ describe("the shared header is framework-neutral", () => { }); }); +describe("the shared search controls are framework-neutral", () => { + it("reaches nothing from next/* or server-only", () => { + expect(offenders(SHARED_CONTROLS, NEXT_ONLY)).toEqual([]); + }); + + it("reaches none of next-intl's Next-only entrypoints", () => { + expect(offenders(SHARED_CONTROLS, NEXT_INTL_RUNTIME)).toEqual([]); + }); + + it("never reaches the locale-aware navigation module", () => { + const reached = [...externalGraph(SHARED_CONTROLS).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_CONTROLS); + + expect(imports).toContain("use-intl"); + expect(imports).not.toContain("next-intl"); + }); + + it("walks into the design system it renders", () => { + // Otherwise the assertions above would pass on a graph that stopped at the + // controls themselves - which is exactly the graph that cannot break. + const reached = [...externalGraph(SHARED_CONTROLS).keys()]; + const visited = runtimeImports(SHARED_CONTROLS); + + expect(visited).toContain("@/components/ui/input-group"); + expect(reached).toContain("lucide-react"); + }); + + it("takes its query and its link as props rather than building either", () => { + const code = readFileSync(SHARED_CONTROLS, "utf8") + .replace(/\/\*[\s\S]*?\*\//g, "") + .replace(/\/\/.*$/gm, ""); + + // Neither a locale nor a transport: the whole request is `feedQuery`'s, and + // `feedQuery` comes from whichever app is rendering this. + expect(code).not.toContain("useLocale"); + expect(code).not.toContain("searchFeedQueryOptions"); + expect(code).toContain("feedQuery: SearchFeedQueryFactory;"); + expect(code).toContain("LinkComponent: SearchFeedLinkComponent;"); + // One feed, and it is the shared one. A second renderer here is the drift + // this boundary exists to prevent. + expect(code.match(/<SearchFeedContent/g)).toHaveLength(1); + }); +}); + 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([]); @@ -219,4 +280,20 @@ describe("the Next wrapper keeps the Next-only pieces", () => { it("resolves the locale itself", () => { expect(readFileSync(NEXT_WRAPPER, "utf8")).toContain("useLocale()"); }); + + it("is where the search controls' Next-only half lives too", () => { + // The control for the suite above: `search-controls.tsx` provably imports + // what `search-controls-content.tsx` must not. + expect(offenders(NEXT_CONTROLS, NEXT_INTL_RUNTIME)).not.toEqual([]); + expect(readFileSync(NEXT_CONTROLS, "utf8")).toContain("useLocale()"); + }); + + it("hands the shared controls the same link the feed gets", () => { + // Two copies would be two component types, and the search page would + // remount its whole result list on every keystroke. + expect(runtimeImports(NEXT_CONTROLS)).toContain("./search-feed"); + expect(readFileSync(NEXT_WRAPPER, "utf8")).toContain( + "export const NextSearchFeedLink", + ); + }); }); diff --git a/packages/vitnode/src/views/search/search-controls-content.tsx b/packages/vitnode/src/views/search/search-controls-content.tsx new file mode 100644 index 000000000..a7cc3cca0 --- /dev/null +++ b/packages/vitnode/src/views/search/search-controls-content.tsx @@ -0,0 +1,194 @@ +"use client"; + +import { SearchIcon } from "lucide-react"; +import React from "react"; +import { useDebouncedCallback } from "use-debounce"; +import { useTranslations } from "use-intl"; + +import { Button } from "@/components/ui/button"; +import { + InputGroup, + InputGroupAddon, + InputGroupInput, +} from "@/components/ui/input-group"; +import { + NativeSelect, + NativeSelectOption, +} from "@/components/ui/native-select"; + +import type { + SearchFeedLinkComponent, + SearchFeedVariant, +} from "./search-feed-content"; +import type { + SearchFeedParams, + SearchFeedQueryOptions, +} from "./search-feed-query"; +import type { SearchSort } from "./search-params"; + +import { getSearchTypeRenderer, searchTypeKeys } from "./registry"; +import { SearchFeedContent } from "./search-feed-content"; +import { + appliedSearchTerm, + defaultSearchSort, + parseSearchTypes, + SEARCH_SORT_VALUES, + SEARCH_TERM_DEBOUNCE_MS, + searchFeedParamsFor, + sortForAppliedTerm, +} from "./search-params"; + +/** + * The search feed a set of controls is currently asking for. + * + * A factory rather than a finished options object, because the controls are what + * *changes* the request: the visitor types, toggles a type or picks a sort, and + * the feed has to become a different query. So the caller supplies the one thing + * only it knows - how a page is fetched in this app - and the controls supply the + * parameters. + * + * Both frameworks build it from the same `searchFeedQueryOptions`, so the query + * key, the cursor rule, the response check and the paging behaviour are the + * feed's, not this component's. See {@link SearchControlsContent}. + */ +export type SearchFeedQueryFactory = ( + params: SearchFeedParams, +) => SearchFeedQueryOptions; + +/** + * The search page's controls and its results, with nothing framework-shaped in + * them. + * + * This is the whole of the search *interaction* - the debounced term, the type + * filters, the sort, and the feed underneath - and it runs unchanged under + * Next.js and under TanStack Start. Exactly two things are injected, and they + * are the same two {@link SearchFeedContent} already needed: + * + * - **`feedQuery`.** See {@link SearchFeedQueryFactory}. The transport is the + * only part of a feed that genuinely differs between a server rendering a + * request and a browser, so it is the only part that crosses this boundary. + * - **`LinkComponent`.** A search hit points wherever the indexed content lives, + * and turning that into a client-side navigation is the router's job. + * + * 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. + * + * ## The URL is deliberately not written to + * + * The term, the types and the sort are component state. Only the *initial* term + * comes from outside, as `defaultParams`, which is how the page has always + * worked: `/search?search=hello` opens on a search for "hello" and everything + * after that is local. A route that wants the URL to follow the controls has to + * decide what a shareable search URL is - which sort belongs in it, whether every + * keystroke is a history entry - and that is a product question, not a migration + * one. + */ +export const SearchControlsContent = ({ + LinkComponent, + defaultParams, + feedQuery, + variant = "timeline", +}: { + defaultParams: SearchFeedParams; + feedQuery: SearchFeedQueryFactory; + LinkComponent: SearchFeedLinkComponent; + variant?: SearchFeedVariant; +}) => { + const t = useTranslations("core.search"); + const [term, setTerm] = React.useState(defaultParams.search ?? ""); + const [appliedTerm, setAppliedTerm] = React.useState( + defaultParams.search ?? "", + ); + const [types, setTypes] = React.useState<string[]>(() => + parseSearchTypes(defaultParams.types), + ); + const [sort, setSort] = React.useState<SearchSort>( + () => defaultParams.sort ?? defaultSearchSort(defaultParams.search), + ); + + /** + * The searched-for term, a moment after the visitor stops typing. + * + * `appliedSearchTerm` owns the decision - including "do nothing", which is + * what a one- or two-character term gets - so this only has to act on it. A + * term that *is* applied also moves the sort off the browse default, and only + * off that one: see `sortForAppliedTerm`. + */ + const applyTerm = useDebouncedCallback((value: string) => { + const next = appliedSearchTerm(value); + + if (next === null) return; + + setAppliedTerm(next); + if (next.length > 0) setSort(sortForAppliedTerm); + }, SEARCH_TERM_DEBOUNCE_MS); + + const toggleType = (key: string) => { + setTypes(prev => + prev.includes(key) ? prev.filter(item => item !== key) : [...prev, key], + ); + }; + + /** + * Rebuilt on every render, deliberately: it is derived from state that changes + * as the visitor types, so memoising on it would be memoising on a moving + * value. Query hashes keys structurally, so an equal object is the same cache + * entry. + */ + const params = searchFeedParamsFor({ search: appliedTerm, sort, types }); + + return ( + <div className="flex flex-col gap-4"> + <InputGroup> + <InputGroupInput + aria-label={t("placeholder")} + onChange={e => { + setTerm(e.target.value); + applyTerm(e.target.value); + }} + placeholder={t("placeholder")} + type="search" + value={term} + /> + <InputGroupAddon> + <SearchIcon /> + </InputGroupAddon> + </InputGroup> + + <div className="flex flex-wrap items-center gap-2"> + {searchTypeKeys.map(key => ( + <Button + aria-pressed={types.includes(key)} + key={key} + onClick={() => toggleType(key)} + size="sm" + variant={types.includes(key) ? "default" : "outline"} + > + {t(getSearchTypeRenderer(key).labelKey)} + </Button> + ))} + + <NativeSelect + aria-label={t("sortBy")} + className="ms-auto" + onChange={e => setSort(e.target.value as SearchSort)} + size="sm" + value={sort} + > + {SEARCH_SORT_VALUES.map(value => ( + <NativeSelectOption key={value} value={value}> + {t(`sort.${value}`)} + </NativeSelectOption> + ))} + </NativeSelect> + </div> + + <SearchFeedContent + LinkComponent={LinkComponent} + queryOptions={feedQuery(params)} + variant={variant} + /> + </div> + ); +}; diff --git a/packages/vitnode/src/views/search/search-controls.tsx b/packages/vitnode/src/views/search/search-controls.tsx index d01cc00a7..2c5620a58 100644 --- a/packages/vitnode/src/views/search/search-controls.tsx +++ b/packages/vitnode/src/views/search/search-controls.tsx @@ -1,32 +1,34 @@ "use client"; -import { SearchIcon } from "lucide-react"; -import { useTranslations } from "next-intl"; -import React from "react"; -import { useDebouncedCallback } from "use-debounce"; +import { useLocale } from "next-intl"; -import { Button } from "@/components/ui/button"; -import { - InputGroup, - InputGroupAddon, - InputGroupInput, -} from "@/components/ui/input-group"; -import { - NativeSelect, - NativeSelectOption, -} from "@/components/ui/native-select"; - -import type { SearchFeedParams } from "./search-feed"; +import type { SearchFeedParams } from "./search-feed-query"; import type { SearchFeedPage } from "./types"; -import { getSearchTypeRenderer, searchTypeKeys } from "./registry"; -import { SearchFeed } from "./search-feed"; - -type SortValue = "newest" | "oldest" | "relevance"; - -const SORT_VALUES: SortValue[] = ["relevance", "newest", "oldest"]; -const MIN_TERM_LENGTH = 3; - +import { SearchControlsContent } from "./search-controls-content"; +import { NextSearchFeedLink } from "./search-feed"; +import { searchFeedQueryOptions } from "./search-feed-query"; + +/** + * {@link SearchControlsContent}, wired to Next.js. + * + * Everything the search page *does* lives in the shared component; this supplies + * the three things that cannot be shared, and the props are unchanged, so + * `SearchView` sees exactly the component it 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 same one `SearchFeed` hands the feed. + * - **The query**, built from `searchFeedQueryOptions` per set of parameters, on + * the browser's transport. That is the default and the right one here: this is + * a client component, and `fetchNextPage` runs in the browser either way. + * + * `initialData` stays supported because Next.js has nowhere else to put a page it + * already fetched: `SearchView` renders the first page in a Server Component and + * hands it down, with no Query cache to hydrate from. An app that *does* hydrate + * one must not use it - see `searchFeedQueryOptions`, and `routes/search.tsx` in + * `apps/web` for the shape that does. + */ export const SearchControls = ({ defaultParams, initialData, @@ -34,87 +36,16 @@ export const SearchControls = ({ defaultParams: SearchFeedParams; initialData?: SearchFeedPage; }) => { - const t = useTranslations("core.search"); - const [term, setTerm] = React.useState(defaultParams.search ?? ""); - const [appliedTerm, setAppliedTerm] = React.useState( - defaultParams.search ?? "", - ); - const [types, setTypes] = React.useState<string[]>( - defaultParams.types ? defaultParams.types.split(",") : [], - ); - const [sort, setSort] = React.useState<SortValue>( - defaultParams.sort ?? "newest", - ); - - const applyTerm = useDebouncedCallback((value: string) => { - if (value.length >= MIN_TERM_LENGTH) { - setAppliedTerm(value); - setSort(prev => (prev === "newest" ? "relevance" : prev)); - } else if (value.length === 0) { - setAppliedTerm(""); - } - }, 500); - - const toggleType = (key: string) => { - setTypes(prev => - prev.includes(key) ? prev.filter(item => item !== key) : [...prev, key], - ); - }; - - const params: SearchFeedParams = { - search: appliedTerm || undefined, - types: types.length ? types.join(",") : undefined, - sort, - }; + const locale = useLocale(); return ( - <div className="flex flex-col gap-4"> - <InputGroup> - <InputGroupInput - onChange={e => { - setTerm(e.target.value); - applyTerm(e.target.value); - }} - placeholder={t("placeholder")} - type="search" - value={term} - /> - <InputGroupAddon> - <SearchIcon /> - </InputGroupAddon> - </InputGroup> - - <div className="flex flex-wrap items-center gap-2"> - {searchTypeKeys.map(key => ( - <Button - key={key} - onClick={() => toggleType(key)} - size="sm" - variant={types.includes(key) ? "default" : "outline"} - > - {t(getSearchTypeRenderer(key).labelKey)} - </Button> - ))} - - <NativeSelect - className="ms-auto" - onChange={e => setSort(e.target.value as SortValue)} - size="sm" - value={sort} - > - {SORT_VALUES.map(value => ( - <NativeSelectOption key={value} value={value}> - {t(`sort.${value}`)} - </NativeSelectOption> - ))} - </NativeSelect> - </div> - - <SearchFeed - initialData={initialData} - params={params} - variant="timeline" - /> - </div> + <SearchControlsContent + defaultParams={defaultParams} + feedQuery={params => + searchFeedQueryOptions({ initialData, locale, params }) + } + LinkComponent={NextSearchFeedLink} + variant="timeline" + /> ); }; diff --git a/packages/vitnode/src/views/search/search-feed.tsx b/packages/vitnode/src/views/search/search-feed.tsx index 5756dfb42..e79703063 100644 --- a/packages/vitnode/src/views/search/search-feed.tsx +++ b/packages/vitnode/src/views/search/search-feed.tsx @@ -21,8 +21,13 @@ export type { SearchFeedParams, SearchFeedVariant }; * * 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. + * + * Exported for `search-controls.tsx`, which renders the shared controls rather + * than this wrapper and so needs the same link: two copies would be two + * component types, and the search page would remount its whole result list on + * every keystroke. */ -const NextSearchFeedLink = ({ +export const NextSearchFeedLink = ({ children, className, href, diff --git a/packages/vitnode/src/views/search/search-params.test.ts b/packages/vitnode/src/views/search/search-params.test.ts new file mode 100644 index 000000000..636c4ea3d --- /dev/null +++ b/packages/vitnode/src/views/search/search-params.test.ts @@ -0,0 +1,298 @@ +import { hashKey } from "@tanstack/react-query"; +import { describe, expect, it } from "vitest"; + +import { searchFeedQueryKey } from "./search-feed-query"; +import { + appliedSearchTerm, + BROWSE_SEARCH_SORT, + defaultSearchSort, + isSearchSort, + MAX_SEARCH_TERM_LENGTH, + MIN_SEARCH_TERM_LENGTH, + normalizeSearchSort, + normalizeSearchTerm, + parseSearchTypes, + SEARCH_SORT_VALUES, + searchFeedParamsFor, + serializeSearchTypes, + sortForAppliedTerm, + TERM_SEARCH_SORT, +} from "./search-params"; + +/** + * What a search request means, asserted where it is decided. + * + * Every one of these is a pure function over data, which is the whole reason + * they were pulled out of the two components and the two route files that used + * to answer these questions inline. Nothing here mounts React, reaches a + * database or builds a URL: the feed's request is `searchFeedRequest`'s job and + * the API's own behaviour is the API's. + */ + +describe("a term from a URL", () => { + it("keeps a term somebody actually typed", () => { + expect(normalizeSearchTerm("hono")).toBe("hono"); + }); + + it("trims it", () => { + expect(normalizeSearchTerm(" hono ")).toBe("hono"); + }); + + it.each([ + ["a bare ?search=", ""], + ["blanks only", " "], + ["a tab and a newline", "\t\n"], + ])("reads %s as no term at all", (_case, value) => { + // `undefined` is the browse feed, which is the right page to render for a + // parameter nobody meant. + expect(normalizeSearchTerm(value)).toBeUndefined(); + }); + + it.each([ + ["repeated, so an array", ["a", "b"]], + ["a number", 42], + ["an object", {}], + ["null", null], + ["absent", undefined], + ])("refuses %s rather than searching for it", (_case, value) => { + expect(normalizeSearchTerm(value)).toBeUndefined(); + }); + + it("caps a term nobody typed by hand", () => { + const term = normalizeSearchTerm("x".repeat(MAX_SEARCH_TERM_LENGTH * 4)); + + expect(term).toHaveLength(MAX_SEARCH_TERM_LENGTH); + }); + + it("does not leave the cut mid-space", () => { + const term = normalizeSearchTerm( + `${"x".repeat(MAX_SEARCH_TERM_LENGTH - 1)} yyy`, + ); + + expect(term).toBe("x".repeat(MAX_SEARCH_TERM_LENGTH - 1)); + }); +}); + +describe("the sort a feed takes when nobody chose one", () => { + it("browses newest-first with no term", () => { + // Relevance is meaningless without a term: every row scores the same. + expect(defaultSearchSort(undefined)).toBe(BROWSE_SEARCH_SORT); + expect(defaultSearchSort("")).toBe(BROWSE_SEARCH_SORT); + }); + + it("ranks by relevance once there is one", () => { + expect(defaultSearchSort("hono")).toBe(TERM_SEARCH_SORT); + }); + + it("accepts every sort the API's enum accepts", () => { + expect(SEARCH_SORT_VALUES.every(isSearchSort)).toBe(true); + }); + + it.each(["relevant", "RELEVANCE", "", 1, null, {}, undefined])( + "refuses %s as a sort", + value => { + expect(isSearchSort(value)).toBe(false); + }, + ); + + it("normalises a sort that is not one back to the default", () => { + expect(normalizeSearchSort("popularity", "hono")).toBe(TERM_SEARCH_SORT); + expect(normalizeSearchSort("popularity")).toBe(BROWSE_SEARCH_SORT); + }); + + it("leaves a real sort alone, term or no term", () => { + expect(normalizeSearchSort("oldest", "hono")).toBe("oldest"); + expect(normalizeSearchSort("oldest")).toBe("oldest"); + }); +}); + +describe("the content types a feed is filtered to", () => { + it("reads the API's comma-separated shape", () => { + expect(parseSearchTypes("blog_post,topic")).toEqual(["blog_post", "topic"]); + }); + + it("reads the shape the controls hold in state", () => { + expect(parseSearchTypes(["blog_post", "topic"])).toEqual([ + "blog_post", + "topic", + ]); + }); + + it("drops the empties a stray comma leaves behind", () => { + // `types=,blog_post,` is three filters, two of which match nothing. + expect(parseSearchTypes(",blog_post,,")).toEqual(["blog_post"]); + }); + + it("drops duplicates, keeping the first", () => { + expect(parseSearchTypes("topic,blog_post,topic")).toEqual([ + "topic", + "blog_post", + ]); + }); + + it("trims each one", () => { + expect(parseSearchTypes(" blog_post , topic ")).toEqual([ + "blog_post", + "topic", + ]); + }); + + it.each([42, null, {}, undefined, true])( + "reads %s as no filter at all", + value => { + expect(parseSearchTypes(value)).toEqual([]); + }, + ); + + it("keeps only the strings out of a mixed list", () => { + expect(parseSearchTypes([1, "topic", null, "blog_post"])).toEqual([ + "topic", + "blog_post", + ]); + }); + + it("keeps a type this build has no renderer for", () => { + // A plugin can index a type the icon registry has never heard of. Dropping + // it would silently ignore a filter the API would have honoured. + expect(parseSearchTypes("something_new")).toEqual(["something_new"]); + }); + + it("serialises back to the query shape, or to nothing", () => { + expect(serializeSearchTypes(["blog_post", "topic"])).toBe( + "blog_post,topic", + ); + expect(serializeSearchTypes([])).toBeUndefined(); + expect(serializeSearchTypes(",,")).toBeUndefined(); + }); + + it("serialises to one canonical string whatever the order", () => { + // The string is part of the query key, so this is what stops one filter + // from being two cache entries. + expect(serializeSearchTypes(["topic", "blog_post"])).toBe( + serializeSearchTypes(["blog_post", "topic"]), + ); + }); +}); + +describe("a search request, as the shared feed's parameters", () => { + it("browses newest-first when asked for nothing", () => { + expect(searchFeedParamsFor()).toEqual({ sort: "newest" }); + }); + + it("is exactly the browse feed with an empty box", () => { + // `/search` with nothing typed and `/discover` are the same request over the + // same documents, so they must be the same cache entry rather than two. + expect(searchFeedParamsFor({ search: "" })).toEqual({ sort: "newest" }); + }); + + it("switches to relevance the moment there is a term", () => { + expect(searchFeedParamsFor({ search: "hono" })).toEqual({ + search: "hono", + sort: "relevance", + }); + }); + + it("keeps a sort the visitor chose", () => { + expect(searchFeedParamsFor({ search: "hono", sort: "oldest" })).toEqual({ + search: "hono", + sort: "oldest", + }); + }); + + it("carries the type filter in the API's shape", () => { + expect(searchFeedParamsFor({ types: ["blog_post"] })).toEqual({ + sort: "newest", + types: "blog_post", + }); + }); + + it("omits what it has no value for, rather than setting it undefined", () => { + // The object is part of the query key. A key holding an explicit + // `undefined` and one holding nothing must not be able to differ. + expect(Object.keys(searchFeedParamsFor({ types: [] }))).toEqual(["sort"]); + }); + + it("normalises rather than throwing on anything a URL can carry", () => { + expect( + searchFeedParamsFor({ + search: ["a", "b"], + sort: "popularity", + types: 42, + }), + ).toEqual({ sort: "newest" }); + }); +}); + +describe("the cache entry a search lands in", () => { + const hashOf = (params: Parameters<typeof searchFeedParamsFor>[0]) => + hashKey( + searchFeedQueryKey({ locale: "en", params: searchFeedParamsFor(params) }), + ); + + it("is one entry however the caller spelled the request", () => { + // A route loader calls this with `{ search }`; the mounted controls call it + // with `{ search, sort, types }` off their own state. Two entries here is + // the bug the whole module exists to prevent: the loader fills one, the + // component reads the other, and an SSR page refetches on hydration. + expect(hashOf({ search: "hono" })).toBe( + hashOf({ search: "hono", sort: "relevance", types: [] }), + ); + }); + + it("is the same entry for a term with stray whitespace", () => { + expect(hashOf({ search: " hono " })).toBe(hashOf({ search: "hono" })); + }); + + it("is the browse entry for every unusable term", () => { + const browse = hashOf({}); + + expect(hashOf({ search: "" })).toBe(browse); + expect(hashOf({ search: " " })).toBe(browse); + expect(hashOf({ search: ["a"] })).toBe(browse); + }); + + it("is a different entry per term, sort, filter and language", () => { + expect(hashOf({ search: "hono" })).not.toBe(hashOf({ search: "drizzle" })); + expect(hashOf({ sort: "oldest" })).not.toBe(hashOf({ sort: "newest" })); + expect(hashOf({ types: ["blog_post"] })).not.toBe(hashOf({})); + expect( + hashKey( + searchFeedQueryKey({ locale: "pl", params: searchFeedParamsFor() }), + ), + ).not.toBe(hashOf({})); + }); + + it("does not depend on the order the filter was toggled in", () => { + // Two visitors who ticked the same two boxes are looking at one feed. + expect(hashOf({ types: ["a", "b"] })).toBe(hashOf({ types: ["b", "a"] })); + }); +}); + +describe("what a keystroke does to the feed", () => { + it("searches once the term is long enough", () => { + expect(appliedSearchTerm("x".repeat(MIN_SEARCH_TERM_LENGTH))).toBe( + "x".repeat(MIN_SEARCH_TERM_LENGTH), + ); + }); + + it("goes back to browsing when the box is cleared", () => { + expect(appliedSearchTerm("")).toBe(""); + }); + + it("leaves the feed alone on the way there", () => { + // `null` is "not yet", and it is the case that matters: backspacing through + // a word must not flash a page of unrelated results. + for (let length = 1; length < MIN_SEARCH_TERM_LENGTH; length += 1) { + expect(appliedSearchTerm("x".repeat(length)), `${length}`).toBeNull(); + } + }); + + it("moves the browse default onto relevance", () => { + expect(sortForAppliedTerm(BROWSE_SEARCH_SORT)).toBe(TERM_SEARCH_SORT); + }); + + it("never overrides a sort the visitor picked", () => { + expect(sortForAppliedTerm("oldest")).toBe("oldest"); + expect(sortForAppliedTerm("relevance")).toBe("relevance"); + }); +}); diff --git a/packages/vitnode/src/views/search/search-params.ts b/packages/vitnode/src/views/search/search-params.ts new file mode 100644 index 000000000..b0f1a7abe --- /dev/null +++ b/packages/vitnode/src/views/search/search-params.ts @@ -0,0 +1,224 @@ +import type { SearchFeedParams } from "./search-feed-query"; + +/** + * What a search *request* is, decided once, as pure functions. + * + * The search page has three inputs - a term, a set of content types, a sort - + * and three places that have to agree on what they mean: a Next.js Server + * Component reading `searchParams`, a TanStack Start route reading its own + * search schema, and the controls the visitor drives in the browser. Every one + * of them used to answer separately, in a line or two of inline logic, and the + * answers were only accidentally the same: `search-view.tsx` decided the default + * sort with `search ? "relevance" : "newest"` while `search-controls.tsx` + * decided it with `defaultParams.sort ?? "newest"`. + * + * So the decisions live here, and nowhere else. Nothing in this module renders, + * fetches or knows which framework is asking - which is also what makes it the + * part of the search slice that is worth testing directly. + * + * {@link SearchFeedParams} - what these produce - is the shared feed's own + * parameter type, and `searchFeedRequest` in `./search-feed-query` is the only + * thing that turns one into a URL. + */ + +/** + * The shortest term the controls will search on. + * + * Two characters is almost always a prefix of something the visitor is still + * typing, and searching on it costs a full-text query per keystroke for a page + * of results nobody reads. + */ +export const MIN_SEARCH_TERM_LENGTH = 3; + +/** + * The longest term that reaches the API. + * + * A term arrives from a URL, so its length is whatever somebody put there. The + * API takes `search` as an unbounded string and hands it to the full-text + * engine, so this is the one place a 40KB query string stops being one. + */ +export const MAX_SEARCH_TERM_LENGTH = 256; + +/** How long the controls wait after a keystroke before searching. */ +export const SEARCH_TERM_DEBOUNCE_MS = 500; + +/** + * Every order results can come back in - the same three the API's `sort` enum + * accepts, in the order the sort control lists them. + */ +export const SEARCH_SORT_VALUES = ["relevance", "newest", "oldest"] as const; + +export type SearchSort = (typeof SEARCH_SORT_VALUES)[number]; + +/** The order a feed with no term comes back in: there is nothing to rank by. */ +export const BROWSE_SEARCH_SORT: SearchSort = "newest"; + +/** The order a feed *with* a term comes back in. */ +export const TERM_SEARCH_SORT: SearchSort = "relevance"; + +/** Narrows a URL parameter, a `<select>` value or a stored preference. */ +export const isSearchSort = (value: unknown): value is SearchSort => + typeof value === "string" && + (SEARCH_SORT_VALUES as readonly string[]).includes(value); + +/** + * A term worth searching for, or nothing at all. + * + * The input is deliberately `unknown`: `?search=a&search=b` parses to an array, + * a bare `?search` to an empty string, and both reach this from a URL nobody + * here wrote. Anything that is not a usable string becomes `undefined`, which is + * the browse feed - a malformed parameter should render the page, not break the + * route. + * + * Trimmed twice on purpose. The first trim is the term itself; the cut at + * {@link MAX_SEARCH_TERM_LENGTH} can land mid-space, so the second tidies the + * tail rather than sending a term ending in a run of blanks. + */ +export const normalizeSearchTerm = (value: unknown): string | undefined => { + if (typeof value !== "string") return undefined; + + const term = value.trim().slice(0, MAX_SEARCH_TERM_LENGTH).trim(); + + return term.length > 0 ? term : undefined; +}; + +/** + * The sort a feed takes when nobody has chosen one. + * + * Relevance is meaningless without a term - every row scores the same - so a + * browse feed is newest-first and a search is relevance-first. This is the rule + * `search-view.tsx` has always applied; it is stated here so the controls and a + * route loader apply the same one. + */ +export const defaultSearchSort = (search?: string): SearchSort => + search ? TERM_SEARCH_SORT : BROWSE_SEARCH_SORT; + +/** A chosen sort if it is one, and {@link defaultSearchSort} if it is not. */ +export const normalizeSearchSort = ( + value: unknown, + search?: string, +): SearchSort => (isSearchSort(value) ? value : defaultSearchSort(search)); + +/** + * The content types a feed is filtered to, as a list. + * + * Accepts either shape the filter is held in - the API's comma-separated string + * or the array the controls keep in state - because both ends of the round trip + * come through here. Blank entries and duplicates are dropped rather than sent: + * `types=,blog_post,` reaches the API as three filters, two of which match + * nothing, and `blog_post,blog_post` is a filter applied twice. + * + * Unknown types are *not* dropped. A type is only unknown to the renderer + * registry - a plugin can index one this build has no icon for - and refusing it + * here would silently ignore a filter the API would have honoured. + */ +export const parseSearchTypes = (value: unknown): string[] => { + const raw = + typeof value === "string" + ? value.split(",") + : Array.isArray(value) + ? value + : []; + const types = new Set<string>(); + + for (const entry of raw) { + if (typeof entry !== "string") continue; + + const type = entry.trim(); + if (type.length > 0) types.add(type); + } + + return [...types]; +}; + +/** + * {@link parseSearchTypes}, back in the shape the API's query takes. + * + * Sorted, and that is the load-bearing part. This string ends up in the query + * key, so `blog_post,topic` and `topic,blog_post` would otherwise be two cache + * entries holding the same results - the API applies the filter as a set and + * does not care what order it arrived in, but Query hashes the string. Two + * visitors who ticked the same two boxes in a different order are looking at one + * feed. + * + * The *display* order is unaffected: the controls hold their filter as an array + * and only ever ask it whether a type is in it. + */ +export const serializeSearchTypes = (value: unknown): string | undefined => { + const types = parseSearchTypes(value).sort((a, b) => a.localeCompare(b)); + + return types.length > 0 ? types.join(",") : undefined; +}; + +/** + * A term, a sort and a set of types, as the shared feed's parameters. + * + * The one function that builds a {@link SearchFeedParams}, and therefore the one + * thing that decides which cache entry a search lands in: the object is part of + * the query key. Every key it omits is omitted *entirely* rather than set to + * `undefined`, so a route loader calling this with `{ search }` and a component + * calling it with `{ search, sort, types }` produce the same object - and Query, + * which hashes keys structurally, reads them as one entry. + * + * With no term at all this returns `{ sort: "newest" }`, which is exactly the + * browse feed `/discover` asks for. That is not a coincidence to be designed + * away: they are the same request over the same documents, so they share a cache + * entry. + */ +export const searchFeedParamsFor = ({ + search, + sort, + types, +}: { + search?: unknown; + sort?: unknown; + types?: unknown; +} = {}): SearchFeedParams => { + const term = normalizeSearchTerm(search); + const params: SearchFeedParams = { + sort: normalizeSearchSort(sort, term), + }; + + if (term !== undefined) params.search = term; + + const filter = serializeSearchTypes(types); + if (filter !== undefined) params.types = filter; + + return params; +}; + +/** + * The term the feed should search on after a keystroke - or `null` for "not + * yet", which is the answer that matters. + * + * Three cases, and the middle one is the reason this is a function rather than a + * comparison at the call site: + * + * "wid" -> "wid" long enough to search + * "" -> "" cleared, so go back to browsing + * "wi" -> null keep showing what is on screen + * + * A one- or two-character term leaves the feed alone rather than resetting it to + * the browse feed, so backspacing through a word does not flash a page of + * unrelated results on the way. + * + * Length is measured on the raw value, not on a trimmed one: that is the + * behaviour the control has always had, and a term of blanks is normalised away + * later by {@link normalizeSearchTerm} anyway. + */ +export const appliedSearchTerm = (value: string): null | string => { + if (value.length >= MIN_SEARCH_TERM_LENGTH) return value; + if (value.length === 0) return ""; + + return null; +}; + +/** + * The sort to use once a term has been typed. + * + * Only the browse default moves. A visitor who explicitly picked "oldest" keeps + * it - overriding a deliberate choice because they then typed something is the + * kind of helpfulness that reads as a bug. + */ +export const sortForAppliedTerm = (sort: SearchSort): SearchSort => + sort === BROWSE_SEARCH_SORT ? TERM_SEARCH_SORT : sort; diff --git a/packages/vitnode/src/views/search/search-view.tsx b/packages/vitnode/src/views/search/search-view.tsx index 8eea48779..e368ed643 100644 --- a/packages/vitnode/src/views/search/search-view.tsx +++ b/packages/vitnode/src/views/search/search-view.tsx @@ -1,10 +1,18 @@ import { getLocale } from "next-intl/server"; -import type { SearchFeedParams } from "./search-feed"; - import { fetchSearchFeed } from "./fetch-feed"; import { SearchControls } from "./search-controls"; +import { normalizeSearchTerm, searchFeedParamsFor } from "./search-params"; +/** + * The search page, the Next.js way: the first page fetched on the server and + * handed to the controls as `initialData`. + * + * The term and the sort it implies come from `./search-params`, which is also + * what the TanStack Start route at `apps/web/src/routes/search.tsx` reads its + * own search schema through - so `/search?search=hello` means the same request + * in both applications rather than in two hand-written approximations of it. + */ export const SearchView = async ({ searchParams, }: { @@ -12,9 +20,8 @@ export const SearchView = async ({ }) => { const query = await searchParams; const locale = await getLocale(); - const search = typeof query.search === "string" ? query.search : undefined; - const sort = search ? "relevance" : "newest"; - const defaultParams: SearchFeedParams = { search, sort }; + const search = normalizeSearchTerm(query.search); + const defaultParams = searchFeedParamsFor({ search }); const initialData = await fetchSearchFeed({ locale, search });