Skip to content

Re-rendering performance improvements #1777

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
Oksamies wants to merge 1 commit into
base: 04-02-refactor_replace_await_and_suspense_with_suspenseifpromise_globally
Choose a base branch
from
Open

Re-rendering performance improvements #1777

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
19 changes: 16 additions & 3 deletions apps/cyberstorm-remix/app/c/community.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { FontAwesomeIcon } from "@fortawesome/react-fontawesome";
import { getSessionTools } from "cyberstorm/security/publicEnvVariables";
import { getApiHostForSsr } from "cyberstorm/utils/env";
import { createSeo } from "cyberstorm/utils/meta";
import React from "react";
import type { ShouldRevalidateFunctionArgs } from "react-router";
import {
Outlet,
Expand Down Expand Up @@ -102,8 +103,13 @@ export default function Community() {

const outletContext = useOutletContext() as OutletContextShape;

return (
<>
const memoizedContext = React.useMemo(
() => ({ ...outletContext, community }),
[outletContext, community]
);

const headerContent = React.useMemo(
() => (
<div className="community__header">
<div
className={classnames(
Expand Down Expand Up @@ -231,7 +237,14 @@ export default function Community() {
</NewButton>
</div>
</div>
<Outlet context={outletContext} />
),
[community, isPackageListingSubPath, isSubPath]
);

return (
<>
{headerContent}
<Outlet context={memoizedContext} />
</>
);
}
27 changes: 22 additions & 5 deletions apps/cyberstorm-remix/app/c/tabs/PackageSearch/PackageSearch.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,10 @@
throw new Response("Community not found", { status: 404 });
}

let cachedFilters: any = null;

Check failure

Code scanning / ESLint

Disallow the `any` type Error

Unexpected any. Specify a different type.
let cachedCommunityId: string | null = null;
let cachedSeo: any = null;

Check failure

Code scanning / ESLint

Disallow the `any` type Error

Unexpected any. Specify a different type.
Comment on lines +67 to +69
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

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

Module-scope caches are typed as any, which defeats type-safety and will likely trip @typescript-eslint/no-explicit-any (enabled via plugin:@typescript-eslint/recommended). Prefer typing these as the concrete loader return types (e.g. CommunityFilters | null and ReturnType<typeof createSeo> | null) so downstream code can rely on filters.sections/etc without implicit any.

Suggested change
let cachedFilters: any = null;
let cachedCommunityId: string | null = null;
let cachedSeo: any = null;
let cachedFilters: Awaited<
ReturnType<DapperTs["getCommunityFilters"]>
> | null = null;
let cachedCommunityId: string | null = null;
let cachedSeo: ReturnType<typeof createSeo> | null = null;

Copilot uses AI. Check for mistakes.

export async function clientLoader({
request,
params,
Expand All @@ -88,10 +92,23 @@
const nsfw = searchParams.get("nsfw");
const deprecated = searchParams.get("deprecated");

// Use the filters already fetched by the server so that React Router
// doesn't send an extra data request during client-side hydration
const serverData = await serverLoader();
const filters = serverData.filters;
let filters;
let seo;
if (
cachedCommunityId === params.communityId &&
cachedFilters &&
cachedSeo
) {
filters = cachedFilters;
seo = cachedSeo;
} else {
const serverData = await serverLoader();
filters = serverData.filters;
seo = serverData.seo;
cachedFilters = filters;
cachedSeo = seo;
cachedCommunityId = params.communityId;
}

const listingsPromise = (async () => {
const finalSection = getSectionDefault(section, filters.sections);
Expand All @@ -115,7 +132,7 @@
return {
filters: filters,
listings: listingsPromise,
seo: serverData.seo,
seo: seo,
};
}
throw new Response("Community not found", { status: 404 });
Expand Down
3 changes: 2 additions & 1 deletion apps/cyberstorm-remix/app/commonComponents/PackageSearch/components/PackageOrder.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,15 @@ import {
interface Props {
order: PackageOrderOptionsType;
setOrder: (val: PackageOrderOptionsType) => void;
id?: string;
}

export const PackageOrder = (props: Props) => (
<NewSelect
options={selectOptions}
value={props.order}
onChange={props.setOrder}
id="packageOrder"
id={props.id ?? "packageOrder"}
/>
);

Expand Down
98 changes: 64 additions & 34 deletions apps/cyberstorm-remix/app/root.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,14 @@ import {
import { LinkLibrary } from "cyberstorm/utils/LinkLibrary";
import { getApiHostForSsr } from "cyberstorm/utils/env";
import { createSeo } from "cyberstorm/utils/meta";
import { type ReactNode, memo, useEffect, useRef, useState } from "react";
import {
type ReactNode,
memo,
useEffect,
useMemo,
useRef,
useState,
} from "react";
import type { LinksFunction } from "react-router";
import {
Links,
Expand Down Expand Up @@ -261,6 +268,17 @@ export function Layout({ children }: { children: React.ReactNode }) {
? false
: true;

const navigationBlock = useMemo(
() => (
<NavigationWrapper
domain={resolvedEnvVars?.VITE_API_URL || ""}
currentUser={data?.currentUser}
communityId={communityId}
/>
),
[resolvedEnvVars?.VITE_API_URL, data?.currentUser, communityId]
);

return (
<html lang="en">
<head>
Expand Down Expand Up @@ -310,11 +328,7 @@ export function Layout({ children }: { children: React.ReactNode }) {
<LinkingProvider value={LinkLibrary}>
<ToastProvider toastDuration={10000}>
<TooltipProvider>
<NavigationWrapper
domain={resolvedEnvVars?.VITE_API_URL || ""}
currentUser={data?.currentUser}
communityId={communityId}
/>
{navigationBlock}
<div className="container container--x container--full island">
<main className="container container--x container--full island-item layout__main">
<section className="container container--y container--full layout__content">
Expand All @@ -323,15 +337,7 @@ export function Layout({ children }: { children: React.ReactNode }) {
{children}
</section>
</main>
{shouldShowAds ? (
<div className="container container--y island-item layout__ads">
<div className="container container--y layout__ads-inner">
{adContainerIds.map((cid, k_i) => (
<AdContainer key={k_i} containerId={cid} />
))}
</div>
</div>
) : null}
<AdsColumn shouldShow={shouldShowAds} />
</div>
<Footer />
{shouldShowAds ? <AdsInit /> : null}
Expand All @@ -352,30 +358,54 @@ const TooltipProvider = memo(function TooltipProvider({
}: {
children: ReactNode;
}) {
return <RadixTooltip delayDuration={80}>{children}</RadixTooltip>;
return (
<RadixTooltip delayDuration={80} skipDelayDuration={200}>
{children}
</RadixTooltip>
);
});

function App() {
const data = useLoaderData<RootLoadersType>();
const sessionTools = getSessionTools();
const dapper = new DapperTs(
() => data.config,
() =>
sessionTools.clearInvalidSession(
data?.publicEnvVariables.VITE_COOKIE_DOMAIN
)
);
const AdsColumn = memo(function AdsColumn({
shouldShow,
}: {
shouldShow: boolean;
}) {
if (!shouldShow) return null;

return (
<Outlet
context={{
currentUser: data?.currentUser,
requestConfig: dapper.config,
domain: data?.publicEnvVariables.VITE_API_URL,
dapper: dapper,
}}
/>
<div className="container container--y island-item layout__ads">
<div className="container container--y layout__ads-inner">
{adContainerIds.map((cid, k_i) => (
<AdContainer key={k_i} containerId={cid} />
Comment on lines +378 to +379
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

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

key={k_i} uses the array index, which can cause unnecessary remounts if the list ever changes. Since adContainerIds is an array of stable IDs, prefer using key={cid} for stable reconciliation.

Suggested change
{adContainerIds.map((cid, k_i) => (
<AdContainer key={k_i} containerId={cid} />
{adContainerIds.map((cid) => (
<AdContainer key={cid} containerId={cid} />

Copilot uses AI. Check for mistakes.
))}
</div>
</div>
);
});

AdsColumn.displayName = "AdsColumn";

function App() {
const data = useLoaderData<RootLoadersType>();

const appOutletContext = useMemo(() => {
const sessionTools = getSessionTools();
const dapper = new DapperTs(
() => data.config,
() =>
sessionTools.clearInvalidSession(
data?.publicEnvVariables.VITE_COOKIE_DOMAIN
)
);
return {
currentUser: data?.currentUser,
requestConfig: dapper.config,
domain: data?.publicEnvVariables.VITE_API_URL,
dapper: dapper,
};
}, [data?.config, data?.currentUser, data?.publicEnvVariables]);

return <Outlet context={appOutletContext} />;
}

export default withSentry(App);
Expand Down
10 changes: 10 additions & 0 deletions apps/cyberstorm-remix/cyberstorm/utils/searchParamsUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,16 @@
export function setParamsBlobValue<
SearchParamsType,
K extends keyof SearchParamsType,
>(setter: React.Dispatch<React.SetStateAction<SearchParamsType>>, key: K) {
return (v: SearchParamsType[K]) =>
setter((prevBlob) => ({ ...prevBlob, [key]: v }));
}

// Keep the old signature wrapper with `oldBlob` around for backward compatibility,
// although it should be refactored eventually.
export function setParamsBlobValueLegacy<
SearchParamsType,
K extends keyof SearchParamsType,
>(setter: (v: SearchParamsType) => void, oldBlob: SearchParamsType, key: K) {
return (v: SearchParamsType[K]) => setter({ ...oldBlob, [key]: v });
Comment on lines 1 to 15
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

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

setParamsBlobValue now takes (setter, key) and uses React.Dispatch/React.SetStateAction, but this file has no React type imports and existing call sites in the app currently call setParamsBlobValue(setter, oldBlob, key). As written this will fail typechecking/build; either restore the original signature under setParamsBlobValue (and export the new setter-based helper under a new name), or update all callers to use setParamsBlobValueLegacy and add the necessary react type imports.

Suggested change
export function setParamsBlobValue<
SearchParamsType,
K extends keyof SearchParamsType,
>(setter: React.Dispatch<React.SetStateAction<SearchParamsType>>, key: K) {
return (v: SearchParamsType[K]) =>
setter((prevBlob) => ({ ...prevBlob, [key]: v }));
}
// Keep the old signature wrapper with `oldBlob` around for backward compatibility,
// although it should be refactored eventually.
export function setParamsBlobValueLegacy<
SearchParamsType,
K extends keyof SearchParamsType,
>(setter: (v: SearchParamsType) => void, oldBlob: SearchParamsType, key: K) {
return (v: SearchParamsType[K]) => setter({ ...oldBlob, [key]: v });
import type { Dispatch, SetStateAction } from 'react';
export function setParamsBlobValue<
SearchParamsType,
K extends keyof SearchParamsType,
>(setter: (v: SearchParamsType) => void, oldBlob: SearchParamsType, key: K) {
return (v: SearchParamsType[K]) => setter({ ...oldBlob, [key]: v });
}
export function setParamsBlobValueFromStateSetter<
SearchParamsType,
K extends keyof SearchParamsType,
>(setter: Dispatch<SetStateAction<SearchParamsType>>, key: K) {
return (v: SearchParamsType[K]) =>
setter((prevBlob) => ({ ...prevBlob, [key]: v }));

Copilot uses AI. Check for mistakes.
}
5 changes: 3 additions & 2 deletions packages/cyberstorm/src/newComponents/AdContainer/AdContainer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { faHeart } from "@fortawesome/free-solid-svg-icons";
import { FontAwesomeIcon } from "@fortawesome/react-fontawesome";
import { memo } from "react";

import { Icon as NewIcon } from "../Icon/Icon";
import "./AdContainer.css";
Expand All @@ -8,7 +9,7 @@ interface AdContainerProps {
containerId: string;
}

export function AdContainer(props: AdContainerProps) {
export const AdContainer = memo(function AdContainer(props: AdContainerProps) {
const { containerId } = props;

return (
Expand All @@ -28,6 +29,6 @@ export function AdContainer(props: AdContainerProps) {
<div className="ad-container__content" id={containerId} />
</div>
);
}
});

AdContainer.displayName = "AdContainer";
6 changes: 3 additions & 3 deletions packages/cyberstorm/src/newComponents/Card/CardPackage/CardPackage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {
} from "@fortawesome/free-solid-svg-icons";
import { faLips } from "@fortawesome/pro-solid-svg-icons";
import { FontAwesomeIcon } from "@fortawesome/react-fontawesome";
import { useEffect, useState } from "react";
import { memo, useEffect, useState } from "react";
import ago from "s-ago";

import { type CardPackageVariants } from "@thunderstore/cyberstorm-theme";
Expand Down Expand Up @@ -43,7 +43,7 @@ interface Props {
rootClasses?: string;
}

export function CardPackage(props: Props) {
export const CardPackage = memo(function CardPackage(props: Props) {
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Mar 24, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify all usages of CardPackage to check for inline function props
rg -n -A 10 '<CardPackage' --type ts --type tsx -g '!*.test.*' -g '!*.spec.*'

Repository: thunderstore-io/thunderstore-ui

Length of output: 102

🏁 Script executed:

#!/bin/bash
# Search for CardPackage component usage (corrected file type flags)
rg -n -A 10 '<CardPackage' --type ts --type jsx

Repository: thunderstore-io/thunderstore-ui

Length of output: 102

🏁 Script executed:

#!/bin/bash
# Also check CardPackage.tsx file directly to confirm memo wrapper exists
wc -l packages/cyberstorm/src/newComponents/Card/CardPackage/CardPackage.tsx

Repository: thunderstore-io/thunderstore-ui

Length of output: 149

🏁 Script executed:

#!/bin/bash
# Check the actual CardPackage.tsx around line 46
head -n 50 packages/cyberstorm/src/newComponents/Card/CardPackage/CardPackage.tsx | tail -n 15

Repository: thunderstore-io/thunderstore-ui

Length of output: 438

🏁 Script executed:

#!/bin/bash
# Search for CardPackage usage without file type flags
rg '<CardPackage' -A 10 --max-count 20

Repository: thunderstore-io/thunderstore-ui

Length of output: 2425

🏁 Script executed:

#!/bin/bash
# Check PackageSearch.tsx specifically around lines mentioned in review (406-416)
head -n 420 packages/cyberstorm/src/newComponents/PackageSearch/PackageSearch.tsx | tail -n 30

Repository: thunderstore-io/thunderstore-ui

Length of output: 203

🏁 Script executed:

#!/bin/bash
# Get full context of the CardPackage usage in PackageSearch
rg -B 5 -A 20 'packageLikeAction=.*={() =>' apps/cyberstorm-remix/app/commonComponents/PackageSearch/PackageSearch.tsx

Repository: thunderstore-io/thunderstore-ui

Length of output: 206

🏁 Script executed:

#!/bin/bash
# Check if useCallback is used elsewhere in PackageSearch
rg 'useCallback' apps/cyberstorm-remix/app/commonComponents/PackageSearch/PackageSearch.tsx

Repository: thunderstore-io/thunderstore-ui

Length of output: 121

🏁 Script executed:

#!/bin/bash
# Search for all CardPackage usages to see if there are other call sites
rg -B 3 -A 15 '<CardPackage' --max-count 50

Repository: thunderstore-io/thunderstore-ui

Length of output: 4070

Inline callback defeats memoization effectiveness.

The memo() wrapper is correctly applied, but in PackageSearch.tsx, packageLikeAction is passed as an inline arrow function inside the map loop. This creates a new reference on every render, causing CardPackage to re-render despite memoization.

Since the file already uses useCallback for other callbacks, wrap packageLikeAction in useCallback to stabilize the prop reference and realize the performance benefit of memoization.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@packages/cyberstorm/src/newComponents/Card/CardPackage/CardPackage.tsx` at
line 46, The prop passed to CardPackage from PackageSearch.tsx
(packageLikeAction) is created inline inside a map, causing a new function
reference each render and defeating memo(); wrap the callback in
React.useCallback in PackageSearch.tsx (import useCallback if missing) to return
a stable function reference and pass that to CardPackage instead of an inline
arrow, and ensure the useCallback dependency array includes only the minimal
reactive values used by the callback so the reference stays stable when
appropriate.

const {
packageData,
isLiked,
Expand Down Expand Up @@ -290,6 +290,6 @@ export function CardPackage(props: Props) {
{csVariant === "fullWidth" ? cardFooter : null}
</div>
);
}
});

CardPackage.displayName = "CardPackage";
Loading