refactor: new useQueryTable with legit types, returns the query data

[david-crespo]

This is built on top of the queryOptions work in #2566. The types in QueryTable have always bothered me, as does returning a component instead of an element, but I could never figure out how to fix it.

console/app/table/QueryTable.tsx

Lines 65 to 77 in 0f922b2

	const makeQueryTable = <Item extends Record<string, unknown>>(
	query: any,
	params: any,
	options: any
	): ComponentType<QueryTableProps<Item>> =>
	function QueryTable({
	debug,
	pageSize = PAGE_SIZE,
	rowHeight = 'small',
	emptyState,
	columns,
	}: QueryTableProps<Item>) {
	const { currentPage, goToNextPage, goToPrevPage, hasPrev } = usePagination()

This change:

• Fixes the types so we don't use any
• Uses a much more intuitive mechanism to enforce that we can only use QueryTable with list endpoints (simply require TData extends { items: TItem[] })
• Returns the React Query query result containing the data, so in cases where we are currently making a second useApiQuery call where we painstaking ensure the params match (see below), we don't have to do that anymore
• Keeps previous page around while loading next page, preventing blank screen flash, and adds a small loading spinner (see Page goes blank while loading next page #1861)

console/app/pages/ProjectsPage.tsx

Lines 64 to 67 in 0f922b2

	const { Table } = useQueryTable('projectList', {})
	const { data: projects } = usePrefetchedApiQuery('projectList', {
	query: { limit: PAGE_SIZE },
	})

The new file is 80 lines instead of 120 and strictly better across the board. There are only 20 useQueryTable calls in the app. It would be pretty quick to convert them all. We could do that here or merge this and convert in batches.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated (UTC)
console	✅ Ready (Inspect)	Visit Preview	Nov 22, 2024 4:25pm

[david-crespo]

I discovered a bug on main due to the refetch being driven by its own useQuery call instead of using the same data as QueryTable. When you move to a page other than first, QueryTable has that page's instances in scope, but the one looking for transitioning instances is permanently stuck on the first page. So if you're on the second page, and there are transitioning instances but none on the first page, it does not poll as you expect. This bug can be fixed by using the new useQueryTable on the instances page.

2024-11-20-instance-polling-bug.mp4

[david-crespo]

What this does:

[david-crespo]

We can now share the query options between the prefetch in the loader and the query table, and we don't have to specify PAGE_SIZE manually here to make sure it matches the query inside QueryTable.

[david-crespo]

There's something off that I need to figure out about the fact that PAGE_SIZE has a default both here and in getListQueryOptionsFn. It means you can manually override the page size in the loader prefetch call and a) it won't apply to the QueryTable, and b) the only way you find out about the mismatch is that the prefetch check in QueryTable fails. It's not the worst (it's more or less what we have now) but it doesn't feel right — it doesn't feel like it's taking advantage of the query options sharing.

I feel like ideally you would not have to specify page size on useQueryTable at all; instead, you would set it on the query options object shared by the loader and the query table. It's easy to build the page size into the options object — the only real obstacle is that inside useQueryTable we need the pageSize for UI reasons (whether to enable the next button), so if we build pageSize into the query options, we need to be able to pull it back out of there. There is a meta key on query options that we could put pageSize on.

[david-crespo]

Figured out #2567 (comment) in 8cab460. This video shows how the page size for a given page, both loader and useQueryTable call, is controlled in a single spot. There is no need to keep the page size in sync manually. The global default page size is controlled at the data layer by the helper that produces the paginated query config object. useQueryTable doesn't know anything about the default, it just takes a prop.

The error on hot reload when I change the value is an ensurePrefetch failure inside of useQueryTable because that query changes, but the loader does not refire, so what's in the cache no longer matches the page size. We don't have to worry about this in production until.

2024-11-21-page-size-demo.mp4

[david-crespo]

I just remembered the problem with this, which made me reluctant to do it everywhere, though doing it in just a few random tables (what we currently do) is probably even worse. When we don't do the blank flash, we don't scroll to top when the next page loads.

2024-11-21-pag-no-scroll.mp4

[david-crespo]

GOT EM

2024-11-21-fix-table-scroll-to-top.mp4

[david-crespo]

Loading spinner. We're in business.

2024-11-21-table-loading-spinner.mp4