Skip to content

Refactor PaginatedDependencies to use loader-resolved data #1595

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
+106 −173
Open

Refactor PaginatedDependencies to use loader-resolved data #1595

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0a55a7f
Refactor PaginatedDependencies
Roffenlund Nov 4, 2025
5300285
Update PaginatedDependencies usage
Roffenlund Nov 4, 2025
e914a3c
Fix typo in interface
Roffenlund Nov 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
204 changes: 49 additions & 155 deletions apps/cyberstorm-remix/app/commonComponents/PaginatedDependencies/PaginatedDependencies.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,175 +1,69 @@
import { memo, Suspense, useEffect, useMemo, useRef, useState } from "react";
import "./PaginatedDependencies.css";
import { Heading, NewPagination, SkeletonBox } from "@thunderstore/cyberstorm";
import { Heading, NewPagination } from "@thunderstore/cyberstorm";
import { ListingDependency } from "../ListingDependency/ListingDependency";
import { Await, useNavigationType, useSearchParams } from "react-router";
import { useDebounce } from "use-debounce";
import type { getPackageVersionDetails } from "@thunderstore/dapper-ts/src/methods/packageVersion";
import type { getPackageVersionDependencies } from "@thunderstore/dapper-ts/src/methods/package";
import { setParamsBlobValue } from "cyberstorm/utils/searchParamsUtils";
import { useSearchParams } from "react-router";
import { type PackageVersionDependency } from "@thunderstore/thunderstore-api";

interface DependencyResponse {
results: PackageVersionDependency[];
count: number;
}

interface Props {
version:
| Awaited<ReturnType<typeof getPackageVersionDetails>>
| ReturnType<typeof getPackageVersionDetails>;
dependencies:
| Awaited<ReturnType<typeof getPackageVersionDependencies>>
| ReturnType<typeof getPackageVersionDependencies>;
dependencies: DependencyResponse;
pageSize?: number;
siblingCount?: number;
}

export const PaginatedDependencies = memo(function PaginatedDependencies(
props: Props
) {
const navigationType = useNavigationType();

export function PaginatedDependencies({
dependencies,
pageSize = 20, // Default page size from backend
siblingCount = 4,
}: Props) {
const [searchParams, setSearchParams] = useSearchParams();
const page = Number(searchParams.get("page") ?? 1);

const initialParams = searchParamsToBlob(searchParams);

const [searchParamsBlob, setSearchParamsBlob] =
useState<SearchParamsType>(initialParams);

const [currentPage, setCurrentPage] = useState(
searchParams.get("page") ? Number(searchParams.get("page")) : 1
);

const [debouncedSearchParamsBlob] = useDebounce(searchParamsBlob, 300, {
maxWait: 300,
});

const searchParamsBlobRef = useRef(debouncedSearchParamsBlob);
const handlePageChange = (nextPage: number) => {
const next = new URLSearchParams(searchParams);

const searchParamsRef = useRef(searchParams);
useEffect(() => {
if (navigationType === "POP") {
if (searchParamsRef.current !== searchParams) {
const spb = searchParamsToBlob(searchParams);
setSearchParamsBlob(spb);
setCurrentPage(spb.page);
searchParamsRef.current = searchParams;
}
searchParamsBlobRef.current = searchParamsToBlob(searchParams);
if (nextPage === 1) {
next.delete("page");
} else {
next.set("page", String(nextPage));
}
}, [searchParams]);

useEffect(() => {
if (
navigationType !== "POP" ||
(navigationType === "POP" &&
searchParamsBlobRef.current !== debouncedSearchParamsBlob)
) {
if (searchParamsBlobRef.current !== debouncedSearchParamsBlob) {
const oldPage = searchParams.get("page")
? Number(searchParams.get("page"))
: 1;
// Page number
if (oldPage !== debouncedSearchParamsBlob.page) {
if (debouncedSearchParamsBlob.page === 1) {
searchParams.delete("page");
setCurrentPage(1);
} else {
searchParams.set("page", String(debouncedSearchParamsBlob.page));
setCurrentPage(debouncedSearchParamsBlob.page);
}
}
const uncommittedSearchParams = searchParamsToBlob(searchParams);

if (
navigationType !== "POP" ||
(navigationType === "POP" &&
!compareSearchParamBlobs(
uncommittedSearchParams,
searchParamsBlobRef.current
) &&
compareSearchParamBlobs(
uncommittedSearchParams,
debouncedSearchParamsBlob
))
) {
setSearchParams(searchParams, { preventScrollReset: true });
}
searchParamsBlobRef.current = debouncedSearchParamsBlob;
}
}
}, [debouncedSearchParamsBlob]);

const versionAndDependencies = useMemo(
() => Promise.all([props.version, props.dependencies]),
[currentPage]
);
setSearchParams(next, { preventScrollReset: true });
};

return (
<div className="paginated-dependencies">
<Suspense
fallback={<SkeletonBox className="paginated-dependencies__skeleton" />}
>
<Await
resolve={versionAndDependencies}
errorElement={
<div>Error occurred while loading required dependencies</div>
}
>
{(resolvedValue) => {
return (
<>
<div className="paginated-dependencies__title">
<Heading csLevel="3" csSize="3">
Required mods ({resolvedValue[0].dependency_count})
</Heading>
<span className="paginated-dependencies__description">
This package requires the following packages to work.
</span>
</div>
<div className="paginated-dependencies__body">
{resolvedValue[1].results.map((dep, key) => {
return <ListingDependency key={key} dependency={dep} />;
})}
</div>
<NewPagination
currentPage={currentPage}
pageSize={props.pageSize ?? 20}
totalCount={resolvedValue[0].dependency_count}
onPageChange={setParamsBlobValue(
setSearchParamsBlob,
searchParamsBlob,
"page"
)}
siblingCount={props.siblingCount ?? 4}
/>
</>
);
}}
</Await>
</Suspense>
<div className="paginated-dependencies__title">
<Heading csLevel="3" csSize="3">
Required mods ({dependencies.count})
</Heading>
<span className="paginated-dependencies__description">
This package requires the following packages to work.
</span>
</div>

<div className="paginated-dependencies__body">
{dependencies.results.map((dep, idx: number) => (
<ListingDependency
key={`${dep.name}-${dep.version_number}-${idx}`}
dependency={dep}
/>
Comment on lines +51 to +54
Copy link

@coderabbitai coderabbitai bot Nov 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify if communityId should be passed to ListingDependency.

The ListingDependency component accepts an optional communityId prop to determine which link variant to use. Currently, no communityId is passed, so it will always use the "WithoutCommunity" links. Confirm whether this is intentional based on where this component is used, or if communityId should be added to the props and passed through.

Run the following script to check how this component is consumed:

🏁 Script executed:

#!/bin/bash
# Description: Check how PaginatedDependencies is used in the codebase

# Search for PaginatedDependencies usage
rg -n -C5 '<PaginatedDependencies' --type=tsx --type=ts

Length of output: 27

🏁 Script executed:

#!/bin/bash
# Description: Search for PaginatedDependencies usage and ListingDependency prop definition
# Using glob patterns instead of file types

# Find PaginatedDependencies usage
echo "=== PaginatedDependencies Usage ==="
rg -n -C5 '<PaginatedDependencies' -g '*.tsx' -g '*.ts'

# Find ListingDependency component definition
echo -e "\n=== ListingDependency Component Definition ==="
rg -n -B2 -A10 'interface.*ListingDependency|type.*ListingDependency|function ListingDependency|const ListingDependency.*=' -g '*.tsx' -g '*.ts' | head -40

# Find ListingDependency prop interface/type
echo -e "\n=== ListingDependency Props ==="
rg -n -B2 -A15 'interface.*ListingDependencyProps|type.*ListingDependencyProps' -g '*.tsx' -g '*.ts'

Length of output: 8678

🏁 Script executed:

#!/bin/bash
# Description: Check PaginatedDependencies component definition and parent context

# Find PaginatedDependencies component definition
echo "=== PaginatedDependencies Definition ==="
rg -n -B2 -A20 'interface.*PaginatedDependenciesProps|type.*PaginatedDependenciesProps|function PaginatedDependencies|export.*PaginatedDependencies' -g '*.tsx' -g '*.ts' apps/cyberstorm-remix/app/commonComponents/PaginatedDependencies/

# Check parent component for communityId availability
echo -e "\n=== Required.tsx context (first 50 lines) ==="
head -50 apps/cyberstorm-remix/app/p/tabs/Required/Required.tsx

# Check PackageVersionWithoutCommunityRequired for comparison
echo -e "\n=== PackageVersionWithoutCommunityRequired.tsx context (first 50 lines) ==="
head -50 apps/cyberstorm-remix/app/p/tabs/Required/PackageVersionWithoutCommunityRequired.tsx

Length of output: 6471

🏁 Script executed:

#!/bin/bash
# Description: Check how communityId is used in ListingDependency and find other usages

# Look at ListingDependency implementation to see how communityId is used
echo "=== ListingDependency Implementation (full file) ==="
cat apps/cyberstorm-remix/app/commonComponents/ListingDependency/ListingDependency.tsx

# Search for other usages of ListingDependency with communityId passed
echo -e "\n=== Other ListingDependency usages in codebase ==="
rg -n -B2 -A2 '<ListingDependency' -g '*.tsx' -g '*.ts'

Length of output: 4188

🏁 Script executed:

#!/bin/bash
# Description: Confirm PackageVersionRequired has communityId

echo "=== PackageVersionRequired.tsx loader (first 40 lines) ==="
head -40 apps/cyberstorm-remix/app/p/tabs/Required/PackageVersionRequired.tsx | grep -A 10 "export async function loader"

Length of output: 530

Add communityId prop to PaginatedDependencies and pass it to ListingDependency; update Required.tsx to provide it.

The review comment is valid. ListingDependency uses communityId to select between link variants ("PackageVersion" with community vs "PackageVersionWithoutCommunity" without). Currently, PaginatedDependencies doesn't accept or forward this prop, so Required.tsx—which has access to communityId—cannot use it. This causes dependency links in Required.tsx to render with the wrong link type.

Changes needed:

  1. Add communityId?: string to PaginatedDependencies props
  2. Pass it to the ListingDependency component (line 51)
  3. Update Required.tsx to pass communityId from route params to PaginatedDependencies

Note: PackageVersionRequired.tsx and PackageVersionWithoutCommunityRequired.tsx have no communityId in their route params, so they remain unchanged.

🤖 Prompt for AI Agents 
In
apps/cyberstorm-remix/app/commonComponents/PaginatedDependencies/PaginatedDependencies.tsx
around lines 51–54, PaginatedDependencies does not accept or forward the
communityId prop so ListingDependency renders the wrong link variant; add
communityId?: string to PaginatedDependencies props, pass communityId into each
<ListingDependency ... /> (preserve current key/props) and then update
Required.tsx to extract communityId from route params and pass it into
PaginatedDependencies when rendering it.

))}
</div>

<NewPagination
currentPage={page}
pageSize={pageSize}
totalCount={dependencies.count}
onPageChange={handlePageChange}
siblingCount={siblingCount}
/>
</div>
);
});
}

PaginatedDependencies.displayName = "PaginatedDependencies";

export type SearchParamsType = {
page: number;
};

export const compareSearchParamBlobs = (
b1: SearchParamsType,
b2: SearchParamsType
) => {
if (b1.page !== b2.page) return false;
return true;
};

export const searchParamsToBlob = (searchParams: URLSearchParams) => {
const initialPage = searchParams.get("page");

return {
page:
initialPage &&
!Number.isNaN(Number.parseInt(initialPage)) &&
Number.isSafeInteger(Number.parseInt(initialPage))
? Number.parseInt(initialPage)
: 1,
};
};
25 changes: 19 additions & 6 deletions apps/cyberstorm-remix/app/p/tabs/Required/PackageVersionRequired.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
import { Suspense } from "react";
import { type LoaderFunctionArgs } from "react-router";
import { useLoaderData } from "react-router";
import { useLoaderData, Await } from "react-router";
import { DapperTs } from "@thunderstore/dapper-ts";
import { SkeletonBox } from "@thunderstore/cyberstorm";
import { PaginatedDependencies } from "~/commonComponents/PaginatedDependencies/PaginatedDependencies";
import {
getPublicEnvVariables,
getSessionTools,
} from "cyberstorm/security/publicEnvVariables";
import { PaginatedDependencies } from "~/commonComponents/PaginatedDependencies/PaginatedDependencies";

export async function loader({ params, request }: LoaderFunctionArgs) {
if (params.namespaceId && params.packageId && params.packageVersion) {
Expand Down Expand Up @@ -66,11 +68,22 @@ export async function clientLoader({ params, request }: LoaderFunctionArgs) {
}

export default function PackageVersionRequired() {
const { version, dependencies } = useLoaderData<
typeof loader | typeof clientLoader
>();
const { dependencies } = useLoaderData();

return (
<PaginatedDependencies version={version} dependencies={dependencies} />
<Suspense
fallback={<SkeletonBox className="paginated-dependencies__skeleton" />}
>
<Await
resolve={dependencies}
errorElement={
<div>Error occurred while loading required dependencies</div>
}
>
{(resolvedDependencies) => (
<PaginatedDependencies dependencies={resolvedDependencies} />
)}
</Await>
</Suspense>
);
}
25 changes: 19 additions & 6 deletions apps/cyberstorm-remix/app/p/tabs/Required/PackageVersionWithoutCommunityRequired.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
import { Suspense } from "react";
import { type LoaderFunctionArgs } from "react-router";
import { useLoaderData } from "react-router";
import { useLoaderData, Await } from "react-router";
import { DapperTs } from "@thunderstore/dapper-ts";
import { SkeletonBox } from "@thunderstore/cyberstorm";
import { PaginatedDependencies } from "~/commonComponents/PaginatedDependencies/PaginatedDependencies";
import {
getPublicEnvVariables,
getSessionTools,
} from "cyberstorm/security/publicEnvVariables";
import { PaginatedDependencies } from "~/commonComponents/PaginatedDependencies/PaginatedDependencies";

export async function loader({ params, request }: LoaderFunctionArgs) {
if (params.namespaceId && params.packageId && params.packageVersion) {
Expand Down Expand Up @@ -66,11 +68,22 @@ export async function clientLoader({ params, request }: LoaderFunctionArgs) {
}

export default function PackageVersionWithoutCommunityRequired() {
const { version, dependencies } = useLoaderData<
typeof loader | typeof clientLoader
>();
const { dependencies } = useLoaderData();

return (
<PaginatedDependencies version={version} dependencies={dependencies} />
<Suspense
fallback={<SkeletonBox className="paginated-dependencies__skeleton" />}
>
<Await
resolve={dependencies}
errorElement={
<div>Error occurred while loading required dependencies</div>
}
>
{(resolvedDependencies) => (
<PaginatedDependencies dependencies={resolvedDependencies} />
)}
</Await>
</Suspense>
);
}
25 changes: 19 additions & 6 deletions apps/cyberstorm-remix/app/p/tabs/Required/Required.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
import { Suspense } from "react";
import { type LoaderFunctionArgs } from "react-router";
import { useLoaderData } from "react-router";
import { useLoaderData, Await } from "react-router";
import { DapperTs } from "@thunderstore/dapper-ts";
import { SkeletonBox } from "@thunderstore/cyberstorm";
import { PaginatedDependencies } from "~/commonComponents/PaginatedDependencies/PaginatedDependencies";
import {
getPublicEnvVariables,
getSessionTools,
} from "cyberstorm/security/publicEnvVariables";
import { PaginatedDependencies } from "~/commonComponents/PaginatedDependencies/PaginatedDependencies";

export async function loader({ params, request }: LoaderFunctionArgs) {
if (params.communityId && params.namespaceId && params.packageId) {
Expand Down Expand Up @@ -76,11 +78,22 @@ export async function clientLoader({ params, request }: LoaderFunctionArgs) {
}

export default function PackageVersionRequired() {
const { version, dependencies } = useLoaderData<
typeof loader | typeof clientLoader
>();
const { dependencies } = useLoaderData();

return (
<PaginatedDependencies version={version} dependencies={dependencies} />
<Suspense
fallback={<SkeletonBox className="paginated-dependencies__skeleton" />}
>
<Await
resolve={dependencies}
errorElement={
<div>Error occurred while loading required dependencies</div>
}
>
{(resolvedDependencies) => (
<PaginatedDependencies dependencies={resolvedDependencies} />
)}
</Await>
Comment on lines +93 to +96
Copy link

@coderabbitai coderabbitai bot Nov 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Pass communityId through to PaginatedDependencies

We need the community context so ListingDependency emits the community-specific links. Right now we drop it, so Required → dependency links omit the community segment. Please pull it from useParams() (or loader data) and forward it:

-import { useLoaderData, Await } from "react-router";
+import { useLoaderData, Await, useParams } from "react-router";
...
-  const { dependencies } = useLoaderData();
+  const { dependencies } = useLoaderData();
+  const { communityId } = useParams();
...
-          <PaginatedDependencies dependencies={resolvedDependencies} />
+          <PaginatedDependencies
+            dependencies={resolvedDependencies}
+            communityId={communityId}
+          />
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
{(resolvedDependencies) => (
<PaginatedDependencies dependencies={resolvedDependencies} />
)}
</Await>
{(resolvedDependencies) => (
<PaginatedDependencies
dependencies={resolvedDependencies}
communityId={communityId}
/>
)}
</Await>
🤖 Prompt for AI Agents 
In apps/cyberstorm-remix/app/p/tabs/Required/Required.tsx around lines 93 to 96,
the communityId from route params (or loader data) is not being passed into
PaginatedDependencies so ListingDependency cannot build community-specific
links; retrieve communityId via useParams() (or from the loader data already
available in this component), add it as a prop when rendering
<PaginatedDependencies />, and update PaginatedDependencies prop types/signature
if needed to accept and forward communityId down to ListingDependency so links
include the community segment.

</Suspense>
);
}
Loading