From 2237c6e336a519229eb3d7950c350ab7265f76d4 Mon Sep 17 00:00:00 2001 From: Scott Dixon Date: Mon, 18 Nov 2024 13:59:12 +1000 Subject: [PATCH 01/21] Add namespace to component and getPaginationVariables utility --- .../hydrogen/src/pagination/Pagination.ts | 47 +++++++++++++------ .../src/pagination/pagination.test.ts | 11 +++++ .../components/PaginatedResourceSection.tsx | 2 +- 3 files changed, 44 insertions(+), 16 deletions(-) diff --git a/packages/hydrogen/src/pagination/Pagination.ts b/packages/hydrogen/src/pagination/Pagination.ts index 2303374e85..233259a51f 100644 --- a/packages/hydrogen/src/pagination/Pagination.ts +++ b/packages/hydrogen/src/pagination/Pagination.ts @@ -78,6 +78,8 @@ type PaginationProps = { connection: Connection; /** A render prop that includes pagination data and helpers. */ children: PaginationRenderProp; + /** A namespace for the pagination component to avoid URL param conflicts. */ + namespace?: string; }; type PaginationRenderProp = FC>; @@ -96,6 +98,7 @@ export function Pagination({ console.warn(' requires children to work properly'); return null; }, + namespace, }: PaginationProps): ReturnType { const transition = useNavigation(); const isLoading = transition.state === 'loading'; @@ -107,7 +110,7 @@ export function Pagination({ nodes, previousPageUrl, startCursor, - } = usePagination(connection); + } = usePagination(connection, namespace); const state = useMemo( () => ({ @@ -175,10 +178,13 @@ export function Pagination({ }); } -function getParamsWithoutPagination(paramsString?: string) { +function getParamsWithoutPagination(paramsString?: string, namespace?: string) { const params = new URLSearchParams(paramsString); - params.delete('cursor'); - params.delete('direction'); + const cursorParam = namespace ? `${namespace}_cursor` : 'cursor'; + const directionParam = namespace ? `${namespace}_direction` : 'direction'; + + params.delete(cursorParam); + params.delete(directionParam); return params.toString(); } @@ -197,6 +203,7 @@ function makeError(prop: string) { */ export function usePagination( connection: Connection, + namespace?: string, ): Omit< PaginationInfo, 'isLoading' | 'state' | 'NextLink' | 'PreviousLink' @@ -231,8 +238,11 @@ export function usePagination( pathname?: string; }; + const cursorParam = namespace ? `${namespace}_cursor` : 'cursor'; + const directionParam = namespace ? `${namespace}_direction` : 'direction'; + const params = new URLSearchParams(search); - const direction = params.get('direction'); + const direction = params.get(directionParam); const isPrevious = direction === 'previous'; const nodes = useMemo(() => { @@ -298,7 +308,7 @@ export function usePagination( // Keep track of the current URL state, to compare whenever the URL changes const urlRef = useRef({ - params: getParamsWithoutPagination(search), + params: getParamsWithoutPagination(search, namespace), pathname, }); @@ -315,12 +325,12 @@ export function usePagination( if ( // If the URL changes (independent of pagination params) // then reset the pagination params in the URL - getParamsWithoutPagination(search) !== urlRef.current.params || + getParamsWithoutPagination(search, namespace) !== urlRef.current.params || pathname !== urlRef.current.pathname ) { urlRef.current = { pathname, - params: getParamsWithoutPagination(search), + params: getParamsWithoutPagination(search, namespace), }; navigate(`${pathname}?${getParamsWithoutPagination(search)}`, { replace: true, @@ -332,17 +342,17 @@ export function usePagination( const previousPageUrl = useMemo(() => { const params = new URLSearchParams(search); - params.set('direction', 'previous'); + params.set(directionParam, 'previous'); currentPageInfo.startCursor && - params.set('cursor', currentPageInfo.startCursor); + params.set(cursorParam, currentPageInfo.startCursor); return `?${params.toString()}`; }, [search, currentPageInfo.startCursor]); const nextPageUrl = useMemo(() => { const params = new URLSearchParams(search); - params.set('direction', 'next'); + params.set(directionParam, 'next'); currentPageInfo.endCursor && - params.set('cursor', currentPageInfo.endCursor); + params.set(cursorParam, currentPageInfo.endCursor); return `?${params.toString()}`; }, [search, currentPageInfo.endCursor]); @@ -357,7 +367,7 @@ export function usePagination( */ export function getPaginationVariables( request: Request, - options: {pageBy: number} = {pageBy: 20}, + options: {pageBy: number; namespace?: string} = {pageBy: 20}, ) { if (typeof request?.url === 'undefined') { throw new Error( @@ -368,9 +378,16 @@ export function getPaginationVariables( const {pageBy} = options; const searchParams = new URLSearchParams(new URL(request.url).search); - const cursor = searchParams.get('cursor') ?? undefined; + const cursorParam = options.namespace + ? `${options.namespace}_cursor` + : 'cursor'; + const directionParam = options.namespace + ? `${options.namespace}_direction` + : 'direction'; + + const cursor = searchParams.get(cursorParam) ?? undefined; const direction = - searchParams.get('direction') === 'previous' ? 'previous' : 'next'; + searchParams.get(directionParam) === 'previous' ? 'previous' : 'next'; const isPrevious = direction === 'previous'; const prevPage = { diff --git a/packages/hydrogen/src/pagination/pagination.test.ts b/packages/hydrogen/src/pagination/pagination.test.ts index ae5152571a..750f3fef2c 100644 --- a/packages/hydrogen/src/pagination/pagination.test.ts +++ b/packages/hydrogen/src/pagination/pagination.test.ts @@ -63,6 +63,17 @@ describe('getPaginationVariables', () => { ), ).toEqual({startCursor: 'abc', last: 10}); }); + + it('returns cursor from search params with namespace', () => { + expect( + getPaginationVariables( + new Request( + 'https://localhost:3000?products_cursor=abc&products_direction=previous', + ), + {pageBy: 20, namespace: 'products'}, + ), + ).toEqual({startCursor: 'abc', last: 20}); + }); }); describe('', () => { diff --git a/templates/skeleton/app/components/PaginatedResourceSection.tsx b/templates/skeleton/app/components/PaginatedResourceSection.tsx index 599f1ac8f2..6925e7ab06 100644 --- a/templates/skeleton/app/components/PaginatedResourceSection.tsx +++ b/templates/skeleton/app/components/PaginatedResourceSection.tsx @@ -15,7 +15,7 @@ export function PaginatedResourceSection({ resourcesClassName?: string; }) { return ( - + {({nodes, isLoading, PreviousLink, NextLink}) => { const resoucesMarkup = nodes.map((node, index) => children({node, index}), From 4c243f4bdf4774fc6f3735de7c584c3b48b1d207 Mon Sep 17 00:00:00 2001 From: Scott Dixon Date: Mon, 18 Nov 2024 14:18:32 +1000 Subject: [PATCH 02/21] Tests --- .../src/pagination/pagination.test.ts | 178 ++++++++++++++++++ 1 file changed, 178 insertions(+) diff --git a/packages/hydrogen/src/pagination/pagination.test.ts b/packages/hydrogen/src/pagination/pagination.test.ts index 750f3fef2c..573ff5606c 100644 --- a/packages/hydrogen/src/pagination/pagination.test.ts +++ b/packages/hydrogen/src/pagination/pagination.test.ts @@ -348,6 +348,184 @@ describe('', () => { `); }); + + it('allows multiple Pagination components with unique namespaces', () => { + const {asFragment} = render( + createElement( + Fragment, + null, + createElement(Pagination, { + connection: { + nodes: [1, 2, 3], + pageInfo: { + endCursor: 'abc', + startCursor: 'cde', + hasNextPage: true, + hasPreviousPage: false, + }, + }, + namespace: 'products', + children: ({nodes}) => + createElement( + Fragment, + null, + nodes.map((node) => + createElement( + 'div', + {key: node as string}, + `Product: ${node as string}`, + ), + ), + ), + }), + createElement(Pagination, { + connection: { + nodes: [4, 5, 6], + pageInfo: { + endCursor: 'def', + startCursor: 'ghi', + hasNextPage: false, + hasPreviousPage: true, + }, + }, + namespace: 'orders', + children: ({nodes}) => + createElement( + Fragment, + null, + nodes.map((node) => + createElement( + 'div', + {key: node as string}, + `Order: ${node as string}`, + ), + ), + ), + }), + ), + ); + + expect(asFragment()).toMatchInlineSnapshot(` + +
+ Product: 1 +
+
+ Product: 2 +
+
+ Product: 3 +
+
+ Order: 4 +
+
+ Order: 5 +
+
+ Order: 6 +
+ + `); + }); + + it('renders multiple Pagination components with unique namespaces correctly', () => { + const {asFragment} = render( + createElement( + Fragment, + null, + createElement(Pagination, { + connection: { + nodes: [1, 2, 3], + pageInfo: { + endCursor: 'abc', + startCursor: 'cde', + hasNextPage: true, + hasPreviousPage: false, + }, + }, + namespace: 'products', + children: ({NextLink, PreviousLink, nodes}) => + createElement( + 'div', + null, + nodes.map((node) => + createElement( + 'div', + {key: node as string}, + `Order: ${node as string}`, + ), + ), + createElement(NextLink, null, 'Next'), + createElement(PreviousLink, null, 'Previous'), + ), + }), + createElement(Pagination, { + connection: { + nodes: [4, 5, 6], + pageInfo: { + endCursor: 'def', + startCursor: 'ghi', + hasNextPage: false, + hasPreviousPage: true, + }, + }, + namespace: 'orders', + children: ({NextLink, PreviousLink, nodes}) => + createElement( + 'div', + null, + nodes.map((node) => + createElement( + 'div', + {key: node as string}, + `Order: ${node as string}`, + ), + ), + createElement(NextLink, null, 'Next'), + createElement(PreviousLink, null, 'Previous'), + ), + }), + ), + ); + + expect(asFragment()).toMatchInlineSnapshot(` + +
+
+ Order: 1 +
+
+ Order: 2 +
+
+ Order: 3 +
+ +
+ + + `); + }); }); function fillLocation(partial: Partial = {}) { From 6284c8cb3031e5174df6ded76823afd9723e02b1 Mon Sep 17 00:00:00 2001 From: Scott Dixon Date: Wed, 20 Nov 2024 15:24:30 +1000 Subject: [PATCH 03/21] Debug --- .../hydrogen/src/pagination/Pagination.ts | 24 +++++--- .../components/PaginatedResourceSection.tsx | 4 +- .../skeleton/app/routes/collections.all.tsx | 61 ++++++++++++++++--- 3 files changed, 69 insertions(+), 20 deletions(-) diff --git a/packages/hydrogen/src/pagination/Pagination.ts b/packages/hydrogen/src/pagination/Pagination.ts index 233259a51f..df40d37d07 100644 --- a/packages/hydrogen/src/pagination/Pagination.ts +++ b/packages/hydrogen/src/pagination/Pagination.ts @@ -250,6 +250,8 @@ export function usePagination( return flattenConnection(connection); } + console.log('STATE', namespace, flattenConnection(connection)); + if (isPrevious) { return [...flattenConnection(connection), ...state.nodes]; } else { @@ -328,15 +330,19 @@ export function usePagination( getParamsWithoutPagination(search, namespace) !== urlRef.current.params || pathname !== urlRef.current.pathname ) { - urlRef.current = { - pathname, - params: getParamsWithoutPagination(search, namespace), - }; - navigate(`${pathname}?${getParamsWithoutPagination(search)}`, { - replace: true, - preventScrollReset: true, - state: {nodes: undefined, pageInfo: undefined}, - }); + // urlRef.current = { + // pathname, + // params: getParamsWithoutPagination(search, namespace), + // }; + // console.log('navigating, ', { + // pathname, + // params: getParamsWithoutPagination(search, namespace), + // }); + // navigate(`${pathname}?${getParamsWithoutPagination(search, namespace)}`, { + // replace: true, + // preventScrollReset: true, + // state: {nodes: undefined, pageInfo: undefined}, + // }); } }, [pathname, search]); diff --git a/templates/skeleton/app/components/PaginatedResourceSection.tsx b/templates/skeleton/app/components/PaginatedResourceSection.tsx index 6925e7ab06..fd31594b77 100644 --- a/templates/skeleton/app/components/PaginatedResourceSection.tsx +++ b/templates/skeleton/app/components/PaginatedResourceSection.tsx @@ -9,13 +9,15 @@ export function PaginatedResourceSection({ connection, children, resourcesClassName, + namespace, }: { connection: React.ComponentProps>['connection']; children: React.FunctionComponent<{node: NodesType; index: number}>; resourcesClassName?: string; + namespace?: string; }) { return ( - + {({nodes, isLoading, PreviousLink, NextLink}) => { const resoucesMarkup = nodes.map((node, index) => children({node, index}), diff --git a/templates/skeleton/app/routes/collections.all.tsx b/templates/skeleton/app/routes/collections.all.tsx index 4b5cd0e02f..198f0b9906 100644 --- a/templates/skeleton/app/routes/collections.all.tsx +++ b/templates/skeleton/app/routes/collections.all.tsx @@ -26,16 +26,25 @@ export async function loader(args: LoaderFunctionArgs) { async function loadCriticalData({context, request}: LoaderFunctionArgs) { const {storefront} = context; const paginationVariables = getPaginationVariables(request, { - pageBy: 8, + pageBy: 3, + namespace: 'products', }); - const [{products}] = await Promise.all([ + const paginationVariables2 = getPaginationVariables(request, { + pageBy: 3, + namespace: 'products2', + }); + + const [{products}, {products: products2}] = await Promise.all([ storefront.query(CATALOG_QUERY, { variables: {...paginationVariables}, }), + storefront.query(CATALOG_QUERY_2, { + variables: {...paginationVariables2}, + }), // Add other queries here, so that they are loaded in parallel ]); - return {products}; + return {products, products2}; } /** @@ -48,21 +57,29 @@ function loadDeferredData({context}: LoaderFunctionArgs) { } export default function Collection() { - const {products} = useLoaderData(); + const {products, products2} = useLoaderData(); return (
-

Products

+

Products 1

+ {({node: product, index}) => ( + {product.title} + )} + + +

Products 2

+ {({node: product, index}) => ( - + {product.title} )}
@@ -161,3 +178,27 @@ const CATALOG_QUERY = `#graphql } ${PRODUCT_ITEM_FRAGMENT} ` as const; + +const CATALOG_QUERY_2 = `#graphql + query Catalog2( + $country: CountryCode + $language: LanguageCode + $first: Int + $last: Int + $startCursor: String + $endCursor: String + ) @inContext(country: $country, language: $language) { + products(first: $first, last: $last, before: $startCursor, after: $endCursor) { + nodes { + ...ProductItem + } + pageInfo { + hasPreviousPage + hasNextPage + startCursor + endCursor + } + } + } + ${PRODUCT_ITEM_FRAGMENT} +` as const; From cdaca5935deb3e5bacf695ff2abe8d010d2fc80e Mon Sep 17 00:00:00 2001 From: Scott Dixon Date: Thu, 21 Nov 2024 10:30:51 +1000 Subject: [PATCH 04/21] namespace the state too --- .../hydrogen/src/pagination/Pagination.ts | 120 ++++++++++-------- .../skeleton/app/routes/collections.all.tsx | 4 +- 2 files changed, 69 insertions(+), 55 deletions(-) diff --git a/packages/hydrogen/src/pagination/Pagination.ts b/packages/hydrogen/src/pagination/Pagination.ts index df40d37d07..144816d0b7 100644 --- a/packages/hydrogen/src/pagination/Pagination.ts +++ b/packages/hydrogen/src/pagination/Pagination.ts @@ -40,9 +40,15 @@ type Connection = pageInfo: PageInfo; }; -type PaginationState = { - nodes?: Array; - pageInfo?: PageInfo | null; +type PaginationState = { + [key: string]: { + nodes: Array; + pageInfo: { + endCursor: Maybe | undefined; + startCursor: Maybe | undefined; + hasPreviousPage: boolean; + }; + }; }; interface PaginationInfo { @@ -98,7 +104,7 @@ export function Pagination({ console.warn(' requires children to work properly'); return null; }, - namespace, + namespace = 'default', }: PaginationProps): ReturnType { const transition = useNavigation(); const isLoading = transition.state === 'loading'; @@ -111,18 +117,22 @@ export function Pagination({ previousPageUrl, startCursor, } = usePagination(connection, namespace); + const location = useLocation(); const state = useMemo( () => ({ - pageInfo: { - endCursor, - hasPreviousPage, - hasNextPage, - startCursor, + ...location.state, + [namespace]: { + pageInfo: { + endCursor, + hasPreviousPage, + hasNextPage, + startCursor, + }, + nodes, }, - nodes, }), - [endCursor, hasNextPage, hasPreviousPage, startCursor, nodes], + [endCursor, hasNextPage, hasPreviousPage, startCursor, nodes, namespace], ); const NextLink = useMemo( @@ -203,7 +213,7 @@ function makeError(prop: string) { */ export function usePagination( connection: Connection, - namespace?: string, + namespace: string = 'default', ): Omit< PaginationInfo, 'isLoading' | 'state' | 'NextLink' | 'PreviousLink' @@ -233,7 +243,7 @@ export function usePagination( const navigate = useNavigate(); const {state, search, pathname} = useLocation() as { - state?: PaginationState; + state?: PaginationState; search?: string; pathname?: string; }; @@ -246,43 +256,53 @@ export function usePagination( const isPrevious = direction === 'previous'; const nodes = useMemo(() => { - if (!globalThis?.window?.__hydrogenHydrated || !state || !state?.nodes) { + if ( + !globalThis?.window?.__hydrogenHydrated || + !state || + !state[namespace]?.nodes + ) { return flattenConnection(connection); } - console.log('STATE', namespace, flattenConnection(connection)); - if (isPrevious) { - return [...flattenConnection(connection), ...state.nodes]; + return [ + ...flattenConnection(connection), + ...(state[namespace]?.nodes || []), + ]; } else { - return [...state.nodes, ...flattenConnection(connection)]; + return [ + ...(state[namespace]?.nodes || []), + ...flattenConnection(connection), + ]; } - }, [state, connection]); + }, [state, connection, namespace]); const currentPageInfo = useMemo(() => { const hydrogenHydrated = globalThis?.window?.__hydrogenHydrated; let pageStartCursor = - !hydrogenHydrated || state?.pageInfo?.startCursor === undefined + !hydrogenHydrated || + state?.[namespace]?.pageInfo?.startCursor === undefined ? connection.pageInfo.startCursor - : state.pageInfo.startCursor; + : state[namespace].pageInfo.startCursor; let pageEndCursor = - !hydrogenHydrated || state?.pageInfo?.endCursor === undefined + !hydrogenHydrated || state?.[namespace]?.pageInfo?.endCursor === undefined ? connection.pageInfo.endCursor - : state.pageInfo.endCursor; + : state[namespace].pageInfo.endCursor; let previousPageExists = - !hydrogenHydrated || state?.pageInfo?.hasPreviousPage === undefined + !hydrogenHydrated || + state?.[namespace]?.pageInfo?.hasPreviousPage === undefined ? connection.pageInfo.hasPreviousPage - : state.pageInfo.hasPreviousPage; + : state[namespace].pageInfo.hasPreviousPage; let nextPageExists = - !hydrogenHydrated || state?.pageInfo?.hasNextPage === undefined + !hydrogenHydrated || + state?.[namespace]?.pageInfo?.hasNextPage === undefined ? connection.pageInfo.hasNextPage - : state.pageInfo.hasNextPage; + : state[namespace].pageInfo.hasNextPage; - // if (!hydrogenHydrated) { - if (state?.nodes) { + if (state?.[namespace]?.nodes) { if (isPrevious) { pageStartCursor = connection.pageInfo.startCursor; previousPageExists = connection.pageInfo.hasPreviousPage; @@ -291,7 +311,6 @@ export function usePagination( nextPageExists = connection.pageInfo.hasNextPage; } } - // } return { startCursor: pageStartCursor, @@ -302,6 +321,7 @@ export function usePagination( }, [ isPrevious, state, + namespace, connection.pageInfo.hasNextPage, connection.pageInfo.hasPreviousPage, connection.pageInfo.startCursor, @@ -323,28 +343,20 @@ export function usePagination( window.__hydrogenHydrated = true; }, []); - useEffect(() => { - if ( - // If the URL changes (independent of pagination params) - // then reset the pagination params in the URL - getParamsWithoutPagination(search, namespace) !== urlRef.current.params || - pathname !== urlRef.current.pathname - ) { - // urlRef.current = { - // pathname, - // params: getParamsWithoutPagination(search, namespace), - // }; - // console.log('navigating, ', { - // pathname, - // params: getParamsWithoutPagination(search, namespace), - // }); - // navigate(`${pathname}?${getParamsWithoutPagination(search, namespace)}`, { - // replace: true, - // preventScrollReset: true, - // state: {nodes: undefined, pageInfo: undefined}, - // }); - } - }, [pathname, search]); + // useEffect(() => { + // if ( + // getParamsWithoutPagination(search, namespace) !== urlRef.current.params || + // pathname !== urlRef.current.pathname + // ) { + // navigate(`${pathname}?${getParamsWithoutPagination(search, namespace)}`, { + // replace: true, + // preventScrollReset: true, + // state: { + // [namespace]: {nodes: undefined, pageInfo: undefined}, + // }, + // }); + // } + // }, [pathname, search, namespace]); const previousPageUrl = useMemo(() => { const params = new URLSearchParams(search); @@ -381,9 +393,11 @@ export function getPaginationVariables( ); } - const {pageBy} = options; + const {pageBy, namespace = 'default'} = options; const searchParams = new URLSearchParams(new URL(request.url).search); + console.log('GET VARIABLES', pageBy, namespace); + const cursorParam = options.namespace ? `${options.namespace}_cursor` : 'cursor'; diff --git a/templates/skeleton/app/routes/collections.all.tsx b/templates/skeleton/app/routes/collections.all.tsx index 198f0b9906..7e502c74e8 100644 --- a/templates/skeleton/app/routes/collections.all.tsx +++ b/templates/skeleton/app/routes/collections.all.tsx @@ -27,7 +27,7 @@ async function loadCriticalData({context, request}: LoaderFunctionArgs) { const {storefront} = context; const paginationVariables = getPaginationVariables(request, { pageBy: 3, - namespace: 'products', + namespace: 'products1', }); const paginationVariables2 = getPaginationVariables(request, { @@ -65,7 +65,7 @@ export default function Collection() { {({node: product, index}) => ( {product.title} From f76b4fe75c65be596033094d7159e0bb5fe743e5 Mon Sep 17 00:00:00 2001 From: Scott Dixon Date: Thu, 21 Nov 2024 13:28:47 +1000 Subject: [PATCH 05/21] Store state inside a pagination key to reduce conflicts and keep record of namespaces --- .../hydrogen/src/pagination/Pagination.ts | 130 +++++++++++------- 1 file changed, 78 insertions(+), 52 deletions(-) diff --git a/packages/hydrogen/src/pagination/Pagination.ts b/packages/hydrogen/src/pagination/Pagination.ts index 144816d0b7..374daa0af5 100644 --- a/packages/hydrogen/src/pagination/Pagination.ts +++ b/packages/hydrogen/src/pagination/Pagination.ts @@ -40,13 +40,16 @@ type Connection = pageInfo: PageInfo; }; -type PaginationState = { - [key: string]: { - nodes: Array; - pageInfo: { - endCursor: Maybe | undefined; - startCursor: Maybe | undefined; - hasPreviousPage: boolean; +type PaginationState = { + pagination?: { + [key: string]: { + nodes: Array; + pageInfo: { + endCursor: Maybe | undefined; + startCursor: Maybe | undefined; + hasPreviousPage: boolean; + hasNextPage: boolean; + }; }; }; }; @@ -122,17 +125,28 @@ export function Pagination({ const state = useMemo( () => ({ ...location.state, - [namespace]: { - pageInfo: { - endCursor, - hasPreviousPage, - hasNextPage, - startCursor, + pagination: { + ...(location.state?.pagination || {}), + [namespace]: { + pageInfo: { + endCursor, + hasPreviousPage, + hasNextPage, + startCursor, + }, + nodes, }, - nodes, }, }), - [endCursor, hasNextPage, hasPreviousPage, startCursor, nodes, namespace], + [ + endCursor, + hasNextPage, + hasPreviousPage, + startCursor, + nodes, + namespace, + location.state, + ], ); const NextLink = useMemo( @@ -188,13 +202,22 @@ export function Pagination({ }); } -function getParamsWithoutPagination(paramsString?: string, namespace?: string) { +function getParamsWithoutPagination( + paramsString?: string, + state?: PaginationState, +) { const params = new URLSearchParams(paramsString); - const cursorParam = namespace ? `${namespace}_cursor` : 'cursor'; - const directionParam = namespace ? `${namespace}_direction` : 'direction'; - params.delete(cursorParam); - params.delete(directionParam); + const activeNamespaces = Object.keys(state?.pagination || {}); + + activeNamespaces.forEach((namespace) => { + const cursorParam = `${namespace}_cursor`; + const directionParam = `${namespace}_direction`; + + params.delete(cursorParam); + params.delete(directionParam); + }); + return params.toString(); } @@ -243,7 +266,7 @@ export function usePagination( const navigate = useNavigate(); const {state, search, pathname} = useLocation() as { - state?: PaginationState; + state?: PaginationState; search?: string; pathname?: string; }; @@ -258,8 +281,7 @@ export function usePagination( const nodes = useMemo(() => { if ( !globalThis?.window?.__hydrogenHydrated || - !state || - !state[namespace]?.nodes + !state?.pagination?.[namespace]?.nodes ) { return flattenConnection(connection); } @@ -267,11 +289,11 @@ export function usePagination( if (isPrevious) { return [ ...flattenConnection(connection), - ...(state[namespace]?.nodes || []), + ...(state.pagination[namespace].nodes || []), ]; } else { return [ - ...(state[namespace]?.nodes || []), + ...(state.pagination[namespace].nodes || []), ...flattenConnection(connection), ]; } @@ -279,30 +301,30 @@ export function usePagination( const currentPageInfo = useMemo(() => { const hydrogenHydrated = globalThis?.window?.__hydrogenHydrated; + const stateInfo = state?.pagination?.[namespace]?.pageInfo; + let pageStartCursor = - !hydrogenHydrated || - state?.[namespace]?.pageInfo?.startCursor === undefined + !hydrogenHydrated || stateInfo?.startCursor === undefined ? connection.pageInfo.startCursor - : state[namespace].pageInfo.startCursor; + : stateInfo.startCursor; let pageEndCursor = - !hydrogenHydrated || state?.[namespace]?.pageInfo?.endCursor === undefined + !hydrogenHydrated || stateInfo?.endCursor === undefined ? connection.pageInfo.endCursor - : state[namespace].pageInfo.endCursor; + : stateInfo.endCursor; let previousPageExists = - !hydrogenHydrated || - state?.[namespace]?.pageInfo?.hasPreviousPage === undefined + !hydrogenHydrated || stateInfo?.hasPreviousPage === undefined ? connection.pageInfo.hasPreviousPage - : state[namespace].pageInfo.hasPreviousPage; + : stateInfo.hasPreviousPage; let nextPageExists = - !hydrogenHydrated || - state?.[namespace]?.pageInfo?.hasNextPage === undefined + !hydrogenHydrated || stateInfo?.hasNextPage === undefined ? connection.pageInfo.hasNextPage - : state[namespace].pageInfo.hasNextPage; + : stateInfo.hasNextPage; - if (state?.[namespace]?.nodes) { + // Update page info based on current connection + if (state?.pagination?.[namespace]?.nodes) { if (isPrevious) { pageStartCursor = connection.pageInfo.startCursor; previousPageExists = connection.pageInfo.hasPreviousPage; @@ -330,7 +352,7 @@ export function usePagination( // Keep track of the current URL state, to compare whenever the URL changes const urlRef = useRef({ - params: getParamsWithoutPagination(search, namespace), + params: getParamsWithoutPagination(search, state), pathname, }); @@ -343,20 +365,24 @@ export function usePagination( window.__hydrogenHydrated = true; }, []); - // useEffect(() => { - // if ( - // getParamsWithoutPagination(search, namespace) !== urlRef.current.params || - // pathname !== urlRef.current.pathname - // ) { - // navigate(`${pathname}?${getParamsWithoutPagination(search, namespace)}`, { - // replace: true, - // preventScrollReset: true, - // state: { - // [namespace]: {nodes: undefined, pageInfo: undefined}, - // }, - // }); - // } - // }, [pathname, search, namespace]); + useEffect(() => { + if ( + // If the URL changes (independent of pagination params) + // then reset the pagination params in the URL + getParamsWithoutPagination(search, state) !== urlRef.current.params || + pathname !== urlRef.current.pathname + ) { + urlRef.current = { + pathname, + params: getParamsWithoutPagination(search, state), + }; + navigate(`${pathname}?${getParamsWithoutPagination(search, state)}`, { + replace: true, + preventScrollReset: true, + state: {nodes: undefined, pageInfo: undefined}, + }); + } + }, [pathname, search, state]); const previousPageUrl = useMemo(() => { const params = new URLSearchParams(search); From 6f733087baa6ca6e754edfcc626908cb1d728a22 Mon Sep 17 00:00:00 2001 From: Scott Dixon Date: Thu, 21 Nov 2024 13:37:44 +1000 Subject: [PATCH 06/21] Clean up params - default namespace should result in 'cursor' and 'direction' params --- .../hydrogen/src/pagination/Pagination.ts | 35 ++++++++++--------- .../skeleton/app/routes/collections.all.tsx | 2 -- 2 files changed, 19 insertions(+), 18 deletions(-) diff --git a/packages/hydrogen/src/pagination/Pagination.ts b/packages/hydrogen/src/pagination/Pagination.ts index 374daa0af5..252547a9ab 100644 --- a/packages/hydrogen/src/pagination/Pagination.ts +++ b/packages/hydrogen/src/pagination/Pagination.ts @@ -107,7 +107,7 @@ export function Pagination({ console.warn(' requires children to work properly'); return null; }, - namespace = 'default', + namespace = '', }: PaginationProps): ReturnType { const transition = useNavigation(); const isLoading = transition.state === 'loading'; @@ -208,15 +208,24 @@ function getParamsWithoutPagination( ) { const params = new URLSearchParams(paramsString); + // Get all namespaces from state const activeNamespaces = Object.keys(state?.pagination || {}); - activeNamespaces.forEach((namespace) => { - const cursorParam = `${namespace}_cursor`; - const directionParam = `${namespace}_direction`; + // Handle non-namespaced params (empty string namespace) + if (activeNamespaces.includes('')) { + params.delete('cursor'); + params.delete('direction'); + } - params.delete(cursorParam); - params.delete(directionParam); - }); + activeNamespaces + .filter((namespace) => namespace !== '') + .forEach((namespace) => { + const cursorParam = `${namespace}_cursor`; + const directionParam = `${namespace}_direction`; + + params.delete(cursorParam); + params.delete(directionParam); + }); return params.toString(); } @@ -419,17 +428,11 @@ export function getPaginationVariables( ); } - const {pageBy, namespace = 'default'} = options; + const {pageBy, namespace = ''} = options; const searchParams = new URLSearchParams(new URL(request.url).search); - console.log('GET VARIABLES', pageBy, namespace); - - const cursorParam = options.namespace - ? `${options.namespace}_cursor` - : 'cursor'; - const directionParam = options.namespace - ? `${options.namespace}_direction` - : 'direction'; + const cursorParam = namespace ? `${namespace}_cursor` : 'cursor'; + const directionParam = namespace ? `${namespace}_direction` : 'direction'; const cursor = searchParams.get(cursorParam) ?? undefined; const direction = diff --git a/templates/skeleton/app/routes/collections.all.tsx b/templates/skeleton/app/routes/collections.all.tsx index 7e502c74e8..412d6c7249 100644 --- a/templates/skeleton/app/routes/collections.all.tsx +++ b/templates/skeleton/app/routes/collections.all.tsx @@ -27,7 +27,6 @@ async function loadCriticalData({context, request}: LoaderFunctionArgs) { const {storefront} = context; const paginationVariables = getPaginationVariables(request, { pageBy: 3, - namespace: 'products1', }); const paginationVariables2 = getPaginationVariables(request, { @@ -65,7 +64,6 @@ export default function Collection() { {({node: product, index}) => ( {product.title} From 4cfab7e6afc8b0004d0351d97779675039ea6b0e Mon Sep 17 00:00:00 2001 From: Scott Dixon Date: Thu, 21 Nov 2024 14:11:33 +1000 Subject: [PATCH 07/21] Restore collections.all --- .../components/PaginatedResourceSection.tsx | 4 +- .../skeleton/app/routes/collections.all.tsx | 59 ++++--------------- 2 files changed, 11 insertions(+), 52 deletions(-) diff --git a/templates/skeleton/app/components/PaginatedResourceSection.tsx b/templates/skeleton/app/components/PaginatedResourceSection.tsx index fd31594b77..599f1ac8f2 100644 --- a/templates/skeleton/app/components/PaginatedResourceSection.tsx +++ b/templates/skeleton/app/components/PaginatedResourceSection.tsx @@ -9,15 +9,13 @@ export function PaginatedResourceSection({ connection, children, resourcesClassName, - namespace, }: { connection: React.ComponentProps>['connection']; children: React.FunctionComponent<{node: NodesType; index: number}>; resourcesClassName?: string; - namespace?: string; }) { return ( - + {({nodes, isLoading, PreviousLink, NextLink}) => { const resoucesMarkup = nodes.map((node, index) => children({node, index}), diff --git a/templates/skeleton/app/routes/collections.all.tsx b/templates/skeleton/app/routes/collections.all.tsx index 412d6c7249..4b5cd0e02f 100644 --- a/templates/skeleton/app/routes/collections.all.tsx +++ b/templates/skeleton/app/routes/collections.all.tsx @@ -26,24 +26,16 @@ export async function loader(args: LoaderFunctionArgs) { async function loadCriticalData({context, request}: LoaderFunctionArgs) { const {storefront} = context; const paginationVariables = getPaginationVariables(request, { - pageBy: 3, + pageBy: 8, }); - const paginationVariables2 = getPaginationVariables(request, { - pageBy: 3, - namespace: 'products2', - }); - - const [{products}, {products: products2}] = await Promise.all([ + const [{products}] = await Promise.all([ storefront.query(CATALOG_QUERY, { variables: {...paginationVariables}, }), - storefront.query(CATALOG_QUERY_2, { - variables: {...paginationVariables2}, - }), // Add other queries here, so that they are loaded in parallel ]); - return {products, products2}; + return {products}; } /** @@ -56,28 +48,21 @@ function loadDeferredData({context}: LoaderFunctionArgs) { } export default function Collection() { - const {products, products2} = useLoaderData(); + const {products} = useLoaderData(); return (
-

Products 1

+

Products

{({node: product, index}) => ( - {product.title} - )} - - -

Products 2

- - {({node: product, index}) => ( - {product.title} + )}
@@ -176,27 +161,3 @@ const CATALOG_QUERY = `#graphql } ${PRODUCT_ITEM_FRAGMENT} ` as const; - -const CATALOG_QUERY_2 = `#graphql - query Catalog2( - $country: CountryCode - $language: LanguageCode - $first: Int - $last: Int - $startCursor: String - $endCursor: String - ) @inContext(country: $country, language: $language) { - products(first: $first, last: $last, before: $startCursor, after: $endCursor) { - nodes { - ...ProductItem - } - pageInfo { - hasPreviousPage - hasNextPage - startCursor - endCursor - } - } - } - ${PRODUCT_ITEM_FRAGMENT} -` as const; From 0ba07255c6bf35331df088d10388cec106b92cac Mon Sep 17 00:00:00 2001 From: Scott Dixon Date: Thu, 21 Nov 2024 14:23:07 +1000 Subject: [PATCH 08/21] Log an error for duplicate namespaces --- packages/hydrogen/src/pagination/Pagination.ts | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/packages/hydrogen/src/pagination/Pagination.ts b/packages/hydrogen/src/pagination/Pagination.ts index 252547a9ab..a3cd8e237d 100644 --- a/packages/hydrogen/src/pagination/Pagination.ts +++ b/packages/hydrogen/src/pagination/Pagination.ts @@ -111,6 +111,8 @@ export function Pagination({ }: PaginationProps): ReturnType { const transition = useNavigation(); const isLoading = transition.state === 'loading'; + const location = useLocation(); + const { endCursor, hasNextPage, @@ -120,7 +122,19 @@ export function Pagination({ previousPageUrl, startCursor, } = usePagination(connection, namespace); - const location = useLocation(); + + // Throw error for duplicate namespace + if ( + location.state?.pagination && + namespace in location.state.pagination && + transition.state === 'idle' + ) { + console.warn( + `Warning: Multiple components are using the same namespace${ + namespace ? `"${namespace}"` : '' + }. This will cause state conflicts. Each component should have a unique namespace.`, + ); + } const state = useMemo( () => ({ From 894cc17964b2a9b2ec9450262407669e07df3529 Mon Sep 17 00:00:00 2001 From: Scott Dixon Date: Thu, 21 Nov 2024 14:26:22 +1000 Subject: [PATCH 09/21] Default namespace to empty string --- packages/hydrogen/src/pagination/Pagination.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/hydrogen/src/pagination/Pagination.ts b/packages/hydrogen/src/pagination/Pagination.ts index a3cd8e237d..7db954349d 100644 --- a/packages/hydrogen/src/pagination/Pagination.ts +++ b/packages/hydrogen/src/pagination/Pagination.ts @@ -259,7 +259,7 @@ function makeError(prop: string) { */ export function usePagination( connection: Connection, - namespace: string = 'default', + namespace: string = '', ): Omit< PaginationInfo, 'isLoading' | 'state' | 'NextLink' | 'PreviousLink' From c4ae96e7e2e83c96229f71fdcf00988a4a47bf98 Mon Sep 17 00:00:00 2001 From: Scott Dixon Date: Thu, 21 Nov 2024 14:30:29 +1000 Subject: [PATCH 10/21] Comments tidy --- packages/hydrogen/src/pagination/Pagination.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/hydrogen/src/pagination/Pagination.ts b/packages/hydrogen/src/pagination/Pagination.ts index 7db954349d..f34de29375 100644 --- a/packages/hydrogen/src/pagination/Pagination.ts +++ b/packages/hydrogen/src/pagination/Pagination.ts @@ -123,7 +123,7 @@ export function Pagination({ startCursor, } = usePagination(connection, namespace); - // Throw error for duplicate namespace + // Warn about non-unique namespace if ( location.state?.pagination && namespace in location.state.pagination && From b79cddeee0be1b6e84229e8646275c65f7069270 Mon Sep 17 00:00:00 2001 From: Scott Dixon Date: Fri, 22 Nov 2024 13:54:59 +1000 Subject: [PATCH 11/21] Update tests with new location state structure --- .../src/pagination/pagination.test.ts | 34 +++++++++++-------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/packages/hydrogen/src/pagination/pagination.test.ts b/packages/hydrogen/src/pagination/pagination.test.ts index 573ff5606c..2b5b1e199a 100644 --- a/packages/hydrogen/src/pagination/pagination.test.ts +++ b/packages/hydrogen/src/pagination/pagination.test.ts @@ -235,7 +235,7 @@ describe('', () => { @@ -269,7 +269,7 @@ describe('', () => { @@ -321,17 +321,21 @@ describe('', () => { From dbfcb4eab99308dbba07ea1f8274da8391be5443 Mon Sep 17 00:00:00 2001 From: Scott Dixon Date: Fri, 22 Nov 2024 15:06:30 +1000 Subject: [PATCH 12/21] Only clean up if the base URL or non-pagination params change and we're not on the initial load --- packages/hydrogen/src/pagination/Pagination.ts | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/packages/hydrogen/src/pagination/Pagination.ts b/packages/hydrogen/src/pagination/Pagination.ts index f34de29375..96f33a2874 100644 --- a/packages/hydrogen/src/pagination/Pagination.ts +++ b/packages/hydrogen/src/pagination/Pagination.ts @@ -287,6 +287,7 @@ export function usePagination( makeError('pageInfo.hasPreviousPage'); } + const transition = useNavigation(); const navigate = useNavigate(); const {state, search, pathname} = useLocation() as { state?: PaginationState; @@ -389,11 +390,16 @@ export function usePagination( }, []); useEffect(() => { + const currentParams = getParamsWithoutPagination(search, state); + const previousParams = urlRef.current.params; + const pathChanged = pathname !== urlRef.current.pathname; + const nonPaginationParamsChanged = currentParams !== previousParams; + if ( - // If the URL changes (independent of pagination params) - // then reset the pagination params in the URL - getParamsWithoutPagination(search, state) !== urlRef.current.params || - pathname !== urlRef.current.pathname + // Only clean up if the base URL or non-pagination params change + (pathChanged || nonPaginationParamsChanged) && + // And we're not on the initial load + !(transition.state === 'idle' && !transition.location) ) { urlRef.current = { pathname, From 031156e566be25d8c4073fdb460c0f62b44795fa Mon Sep 17 00:00:00 2001 From: Scott Dixon Date: Fri, 22 Nov 2024 15:25:01 +1000 Subject: [PATCH 13/21] Add examples to the docs --- .../hydrogen/src/pagination/Pagination.doc.ts | 29 +++++ .../Pagination.multiple.example.jsx | 117 ++++++++++++++++++ .../Pagination.multiple.example.tsx | 117 ++++++++++++++++++ 3 files changed, 263 insertions(+) create mode 100644 packages/hydrogen/src/pagination/Pagination.multiple.example.jsx create mode 100644 packages/hydrogen/src/pagination/Pagination.multiple.example.tsx diff --git a/packages/hydrogen/src/pagination/Pagination.doc.ts b/packages/hydrogen/src/pagination/Pagination.doc.ts index 1151127506..c81a339bf0 100644 --- a/packages/hydrogen/src/pagination/Pagination.doc.ts +++ b/packages/hydrogen/src/pagination/Pagination.doc.ts @@ -38,6 +38,35 @@ const data: ReferenceEntityTemplateSchema = { description: '', }, ], + examples: { + description: 'Other examples using the `Pagination` component.', + exampleGroups: [ + { + title: 'Multiple `Pagination` components on a single page', + examples: [ + { + description: + 'Use the `namespace` prop to differentiate between multiple `Pagination` components on a single page', + codeblock: { + title: 'Example', + tabs: [ + { + title: 'JavaScript', + code: './Pagination.multiple.example.jsx', + language: 'jsx', + }, + { + title: 'TypeScript', + code: './Pagination.multiple.example.tsx', + language: 'tsx', + }, + ], + }, + }, + ], + }, + ], + }, }; export default data; diff --git a/packages/hydrogen/src/pagination/Pagination.multiple.example.jsx b/packages/hydrogen/src/pagination/Pagination.multiple.example.jsx new file mode 100644 index 0000000000..11e0357633 --- /dev/null +++ b/packages/hydrogen/src/pagination/Pagination.multiple.example.jsx @@ -0,0 +1,117 @@ +import {json} from '@shopify/remix-oxygen'; +import {useLoaderData, Link} from '@remix-run/react'; +import {getPaginationVariables, Pagination} from '@shopify/hydrogen'; + +export async function loader({request, context: {storefront}}) { + const womensPaginationVariables = getPaginationVariables(request, { + pageBy: 2, + namespace: 'womens', // Specify a unique namespace for the pagination parameters + }); + const mensPaginationVariables = getPaginationVariables(request, { + pageBy: 2, + namespace: 'mens', // Specify a unique namespace for the pagination parameters + }); + + const [womensProducts, mensProducts] = await Promise.all([ + storefront.query(COLLECTION_PRODUCTS_QUERY, { + variables: {...womensPaginationVariables, handle: 'women'}, + }), + storefront.query(COLLECTION_PRODUCTS_QUERY, { + variables: {...mensPaginationVariables, handle: 'men'}, + }), + ]); + + return json({womensProducts, mensProducts}); +} + +export default function Collection() { + const {womensProducts, mensProducts} = useLoaderData(); + return ( +
+

Womens

+ + + {({nodes, isLoading, PreviousLink, NextLink}) => { + return ( +
+ + {isLoading ? 'Loading...' : ↑ Load previous} + +
+ {nodes.map((product) => ( +
+ + {product.title} + +
+ ))} +
+ + {isLoading ? 'Loading...' : Load more ↓} + +
+ ); + }} + + +

Mens

+ + {({nodes, isLoading, PreviousLink, NextLink}) => { + return ( +
+ + {isLoading ? 'Loading...' : ↑ Load previous} + +
+ {nodes.map((product) => ( +
+ + {product.title} + +
+ ))} +
+ + {isLoading ? 'Loading...' : Load more ↓} + +
+ ); + }} + +
+ ); +} + +const COLLECTION_PRODUCTS_QUERY = `#graphql + query CollectionProducts( + $first: Int + $last: Int + $startCursor: String + $endCursor: String + $handle: String! + ) { + collection(handle: $handle) { + products(first: $first, last: $last, before: $startCursor, after: $endCursor) { + nodes { + id + handle + title + } + pageInfo { + hasPreviousPage + hasNextPage + startCursor + endCursor + } + } + } + } +`; diff --git a/packages/hydrogen/src/pagination/Pagination.multiple.example.tsx b/packages/hydrogen/src/pagination/Pagination.multiple.example.tsx new file mode 100644 index 0000000000..aeed1ee69d --- /dev/null +++ b/packages/hydrogen/src/pagination/Pagination.multiple.example.tsx @@ -0,0 +1,117 @@ +import {json, type LoaderArgs} from '@shopify/remix-oxygen'; +import {useLoaderData, Link} from '@remix-run/react'; +import {getPaginationVariables, Pagination} from '@shopify/hydrogen'; + +export async function loader({request, context: {storefront}}: LoaderArgs) { + const womensPaginationVariables = getPaginationVariables(request, { + pageBy: 2, + namespace: 'womens', // Specify a unique namespace for the pagination parameters + }); + const mensPaginationVariables = getPaginationVariables(request, { + pageBy: 2, + namespace: 'mens', // Specify a unique namespace for the pagination parameters + }); + + const [womensProducts, mensProducts] = await Promise.all([ + storefront.query(COLLECTION_PRODUCTS_QUERY, { + variables: {...womensPaginationVariables, handle: 'women'}, + }), + storefront.query(COLLECTION_PRODUCTS_QUERY, { + variables: {...mensPaginationVariables, handle: 'men'}, + }), + ]); + + return json({womensProducts, mensProducts}); +} + +export default function Collection() { + const {womensProducts, mensProducts} = useLoaderData(); + return ( +
+

Womens

+ + + {({nodes, isLoading, PreviousLink, NextLink}) => { + return ( +
+ + {isLoading ? 'Loading...' : ↑ Load previous} + +
+ {nodes.map((product) => ( +
+ + {product.title} + +
+ ))} +
+ + {isLoading ? 'Loading...' : Load more ↓} + +
+ ); + }} + + +

Mens

+ + {({nodes, isLoading, PreviousLink, NextLink}) => { + return ( +
+ + {isLoading ? 'Loading...' : ↑ Load previous} + +
+ {nodes.map((product) => ( +
+ + {product.title} + +
+ ))} +
+ + {isLoading ? 'Loading...' : Load more ↓} + +
+ ); + }} + +
+ ); +} + +const COLLECTION_PRODUCTS_QUERY = `#graphql + query CollectionProducts( + $first: Int + $last: Int + $startCursor: String + $endCursor: String + $handle: String! + ) { + collection(handle: $handle) { + products(first: $first, last: $last, before: $startCursor, after: $endCursor) { + nodes { + id + handle + title + } + pageInfo { + hasPreviousPage + hasNextPage + startCursor + endCursor + } + } + } + } +` as const; From 325d9286d546cb1f365bcbb4241e5f9bff798b7b Mon Sep 17 00:00:00 2001 From: Scott Dixon Date: Fri, 22 Nov 2024 15:28:11 +1000 Subject: [PATCH 14/21] Docs --- packages/hydrogen/src/pagination/Pagination.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/hydrogen/src/pagination/Pagination.ts b/packages/hydrogen/src/pagination/Pagination.ts index 96f33a2874..c107da3262 100644 --- a/packages/hydrogen/src/pagination/Pagination.ts +++ b/packages/hydrogen/src/pagination/Pagination.ts @@ -87,7 +87,7 @@ type PaginationProps = { connection: Connection; /** A render prop that includes pagination data and helpers. */ children: PaginationRenderProp; - /** A namespace for the pagination component to avoid URL param conflicts. */ + /** A namespace for the pagination component to avoid URL param conflicts when using multiple `Pagination` components on a single page. */ namespace?: string; }; @@ -434,7 +434,7 @@ export function usePagination( /** * @param request The request object passed to your Remix loader function. - * @param options Options for how to configure the pagination variables. Includes the ability to change how many nodes are within each page. + * @param options Options for how to configure the pagination variables. Includes the ability to change how many nodes are within each page as well as a namespace to avoid URL param conflicts when using multiple `Pagination` components on a single page. * * @returns Variables to be used with the `storefront.query` function */ From f6a35c51acb6c318de3e4f20a21121f481532ae0 Mon Sep 17 00:00:00 2001 From: Scott Dixon Date: Fri, 22 Nov 2024 15:33:55 +1000 Subject: [PATCH 15/21] Changeset --- .changeset/metal-wasps-collect.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/metal-wasps-collect.md diff --git a/.changeset/metal-wasps-collect.md b/.changeset/metal-wasps-collect.md new file mode 100644 index 0000000000..9e3821da9b --- /dev/null +++ b/.changeset/metal-wasps-collect.md @@ -0,0 +1,5 @@ +--- +'@shopify/hydrogen': minor +--- + +Added an optional `namespace` prop to the `` component to allow for multiple pagination components on a single page. From 5f739bf97cfba464d01d95c807ad8ee93dbeb051 Mon Sep 17 00:00:00 2001 From: Scott Dixon Date: Fri, 22 Nov 2024 15:39:28 +1000 Subject: [PATCH 16/21] Fix types --- .../src/pagination/Pagination.multiple.example.tsx | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/packages/hydrogen/src/pagination/Pagination.multiple.example.tsx b/packages/hydrogen/src/pagination/Pagination.multiple.example.tsx index aeed1ee69d..e2261f8d2f 100644 --- a/packages/hydrogen/src/pagination/Pagination.multiple.example.tsx +++ b/packages/hydrogen/src/pagination/Pagination.multiple.example.tsx @@ -1,8 +1,12 @@ -import {json, type LoaderArgs} from '@shopify/remix-oxygen'; +import {json, type LoaderFunctionArgs} from '@shopify/remix-oxygen'; import {useLoaderData, Link} from '@remix-run/react'; import {getPaginationVariables, Pagination} from '@shopify/hydrogen'; +import {type Collection} from '@shopify/hydrogen-react/storefront-api-types'; -export async function loader({request, context: {storefront}}: LoaderArgs) { +export async function loader({ + request, + context: {storefront}, +}: LoaderFunctionArgs) { const womensPaginationVariables = getPaginationVariables(request, { pageBy: 2, namespace: 'womens', // Specify a unique namespace for the pagination parameters @@ -13,10 +17,10 @@ export async function loader({request, context: {storefront}}: LoaderArgs) { }); const [womensProducts, mensProducts] = await Promise.all([ - storefront.query(COLLECTION_PRODUCTS_QUERY, { + storefront.query<{collection: Collection}>(COLLECTION_PRODUCTS_QUERY, { variables: {...womensPaginationVariables, handle: 'women'}, }), - storefront.query(COLLECTION_PRODUCTS_QUERY, { + storefront.query<{collection: Collection}>(COLLECTION_PRODUCTS_QUERY, { variables: {...mensPaginationVariables, handle: 'men'}, }), ]); From d1f0013f422913676cd315ec961f16aa387a440e Mon Sep 17 00:00:00 2001 From: Scott Dixon Date: Fri, 22 Nov 2024 15:53:06 +1000 Subject: [PATCH 17/21] Improve changeset message --- .changeset/metal-wasps-collect.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.changeset/metal-wasps-collect.md b/.changeset/metal-wasps-collect.md index 9e3821da9b..14734e46a5 100644 --- a/.changeset/metal-wasps-collect.md +++ b/.changeset/metal-wasps-collect.md @@ -2,4 +2,8 @@ '@shopify/hydrogen': minor --- -Added an optional `namespace` prop to the `` component to allow for multiple pagination components on a single page. +Added namespace support to prevent conflicts when using multiple Pagination components: +- New optional `namespace` prop for the `` component +- New optional `namespace` option for `getPaginationVariables()` utility +- When specified, pagination URL parameters are prefixed with the namespace (e.g., `products_cursor` instead of `cursor`) +- Maintains backwards compatibility when no namespace is provided From 6c416049c40ab33c7235dadb9f26a50f5bce191d Mon Sep 17 00:00:00 2001 From: Scott Dixon Date: Mon, 25 Nov 2024 17:47:30 +1100 Subject: [PATCH 18/21] Incorporate review feedback --- .../hydrogen/src/pagination/Pagination.ts | 30 ++++++------------- 1 file changed, 9 insertions(+), 21 deletions(-) diff --git a/packages/hydrogen/src/pagination/Pagination.ts b/packages/hydrogen/src/pagination/Pagination.ts index c107da3262..6a7e7dd74d 100644 --- a/packages/hydrogen/src/pagination/Pagination.ts +++ b/packages/hydrogen/src/pagination/Pagination.ts @@ -44,12 +44,7 @@ type PaginationState = { pagination?: { [key: string]: { nodes: Array; - pageInfo: { - endCursor: Maybe | undefined; - startCursor: Maybe | undefined; - hasPreviousPage: boolean; - hasNextPage: boolean; - }; + pageInfo?: PageInfo | null; }; }; }; @@ -225,21 +220,14 @@ function getParamsWithoutPagination( // Get all namespaces from state const activeNamespaces = Object.keys(state?.pagination || {}); - // Handle non-namespaced params (empty string namespace) - if (activeNamespaces.includes('')) { - params.delete('cursor'); - params.delete('direction'); - } - - activeNamespaces - .filter((namespace) => namespace !== '') - .forEach((namespace) => { - const cursorParam = `${namespace}_cursor`; - const directionParam = `${namespace}_direction`; - - params.delete(cursorParam); - params.delete(directionParam); - }); + activeNamespaces.forEach((namespace) => { + // Clean up cursor and direction params for both namespaced and non-namespaced pagination + const namespacePrefix = namespace === '' ? '' : `${namespace}_`; + const cursorParam = `${namespacePrefix}cursor`; + const directionParam = `${namespacePrefix}direction`; + params.delete(cursorParam); + params.delete(directionParam); + }); return params.toString(); } From b5ef197b9ed7f68550c0aed65586a2cad985eb2a Mon Sep 17 00:00:00 2001 From: Scott Dixon Date: Tue, 3 Dec 2024 13:31:39 +1000 Subject: [PATCH 19/21] Individual loading state --- .../hydrogen/src/pagination/Pagination.ts | 25 ++++++++----------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/packages/hydrogen/src/pagination/Pagination.ts b/packages/hydrogen/src/pagination/Pagination.ts index 6a7e7dd74d..b82fc42251 100644 --- a/packages/hydrogen/src/pagination/Pagination.ts +++ b/packages/hydrogen/src/pagination/Pagination.ts @@ -4,6 +4,7 @@ import { useMemo, useRef, forwardRef, + useState, type Ref, type FC, } from 'react'; @@ -104,10 +105,17 @@ export function Pagination({ }, namespace = '', }: PaginationProps): ReturnType { + const [isLoading, setIsLoading] = useState(false); const transition = useNavigation(); - const isLoading = transition.state === 'loading'; const location = useLocation(); + // Reset loading state once the transition state is idle + useEffect(() => { + if (transition.state === 'idle') { + setIsLoading(false); + } + }, [transition.state]); + const { endCursor, hasNextPage, @@ -118,19 +126,6 @@ export function Pagination({ startCursor, } = usePagination(connection, namespace); - // Warn about non-unique namespace - if ( - location.state?.pagination && - namespace in location.state.pagination && - transition.state === 'idle' - ) { - console.warn( - `Warning: Multiple components are using the same namespace${ - namespace ? `"${namespace}"` : '' - }. This will cause state conflicts. Each component should have a unique namespace.`, - ); - } - const state = useMemo( () => ({ ...location.state, @@ -172,6 +167,7 @@ export function Pagination({ state, replace: true, ref, + onClick: () => setIsLoading(true), }) : null; }), @@ -192,6 +188,7 @@ export function Pagination({ state, replace: true, ref, + onClick: () => setIsLoading(true), }) : null; }), From f4e0c6a64f6b61b1f8b02ccd49198420ab09818d Mon Sep 17 00:00:00 2001 From: Scott Dixon Date: Fri, 6 Dec 2024 08:39:06 +1000 Subject: [PATCH 20/21] Clean up location state on initial load --- packages/hydrogen/src/pagination/Pagination.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/packages/hydrogen/src/pagination/Pagination.ts b/packages/hydrogen/src/pagination/Pagination.ts index b82fc42251..12ee79e2fc 100644 --- a/packages/hydrogen/src/pagination/Pagination.ts +++ b/packages/hydrogen/src/pagination/Pagination.ts @@ -108,6 +108,7 @@ export function Pagination({ const [isLoading, setIsLoading] = useState(false); const transition = useNavigation(); const location = useLocation(); + const navigate = useNavigate(); // Reset loading state once the transition state is idle useEffect(() => { @@ -116,6 +117,14 @@ export function Pagination({ } }, [transition.state]); + useEffect(() => { + // Clean up location state on initial load + navigate(window.location.toString(), { + replace: true, + state: {nodes: undefined, pageInfo: undefined}, + }); + }, []); + const { endCursor, hasNextPage, From adf2b8156e4dbb4c926a823454f5f2cd08fdf22b Mon Sep 17 00:00:00 2001 From: Helen Lin Date: Mon, 9 Dec 2024 10:03:33 -0800 Subject: [PATCH 21/21] revert the useEffect replace state --- packages/hydrogen/src/pagination/Pagination.ts | 8 -------- 1 file changed, 8 deletions(-) diff --git a/packages/hydrogen/src/pagination/Pagination.ts b/packages/hydrogen/src/pagination/Pagination.ts index 12ee79e2fc..c480022eca 100644 --- a/packages/hydrogen/src/pagination/Pagination.ts +++ b/packages/hydrogen/src/pagination/Pagination.ts @@ -117,14 +117,6 @@ export function Pagination({ } }, [transition.state]); - useEffect(() => { - // Clean up location state on initial load - navigate(window.location.toString(), { - replace: true, - state: {nodes: undefined, pageInfo: undefined}, - }); - }, []); - const { endCursor, hasNextPage,