-
Notifications
You must be signed in to change notification settings - Fork 5
Add suspense usage #1515
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.
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
Add suspense usage #1515
Conversation
WalkthroughClient-side Suspense/Await rendering replaces server-side meta generation and eager data fetching. Community route loaders now return Promises consumed in the UI. PackageSearch accepts a Promise for listings and renders within Suspense. PageHeader CSS removes a flex rule. Root switches from useRouteLoaderData to useLoaderData. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Router as Remix Router
participant Loader as community.loader
participant UI as Community Route Component
participant PS as PackageSearch
User->>Router: Navigate to /c/:community
Router->>Loader: Invoke loader
note over Loader: Return Promises { community, listings }
Loader-->>Router: { community: Promise, listings: Promise }
Router->>UI: Render with data (Promises)
par Suspense boundaries
UI->>UI: Suspense for meta/hero/header
UI->>Loader: Await community
Loader-->>UI: community data
UI->>UI: Construct meta, hero image, breadcrumbs
and
UI->>PS: Render PackageSearch(listings: Promise)
PS->>Loader: Await listings
Loader-->>PS: listings data
PS->>PS: Render count, grid, pagination
end
sequenceDiagram
autonumber
participant Root as Root Route
participant Hook as useLoaderData
participant Ctx as App Context
Root->>Hook: Read loader data
Hook-->>Root: RootLoadersType data
Root->>Ctx: Provide outlet context (sections, config, currentUser, dapper)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
<Suspense> | ||
<Await resolve={community}> | ||
{(resolvedValue) => { | ||
return ( | ||
<> | ||
<meta | ||
title={`Thunderstore - The ${resolvedValue.name} Mod Database`} | ||
/> | ||
<meta | ||
name="description" | ||
content={`Mods for ${resolvedValue.name}`} | ||
/> | ||
<meta property="og:type" content="website" /> | ||
<meta | ||
property="og:url" | ||
content={`${ | ||
getPublicEnvVariables(["VITE_SITE_URL"]).VITE_SITE_URL | ||
}${location.pathname}`} | ||
/> | ||
<meta | ||
property="og:title" | ||
content={`Thunderstore - The ${resolvedValue.name} Mod Database`} | ||
/> | ||
<meta | ||
property="og:description" | ||
content={`Thunderstore is a mod database and API for downloading ${resolvedValue.name} mods`} | ||
/> | ||
<meta property="og:image:width" content="360" /> | ||
<meta property="og:image:height" content="480" /> | ||
<meta | ||
property="og:image" | ||
content={resolvedValue.cover_image_url ?? undefined} | ||
/> | ||
<meta property="og:site_name" content="Thunderstore" /> | ||
</> | ||
); | ||
}} | ||
</Await> | ||
</Suspense> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Meta tags implementation issue
The current implementation places meta tags inside Suspense/Await blocks within the component body, which is problematic for SEO and metadata functionality. Meta tags need to be in the document <head>
to be properly recognized by search engines and social media platforms.
The original approach using the meta
function was more appropriate for setting document metadata, as React Router handles placing these tags in the document head.
Consider reverting to the previous pattern:
export const meta: MetaFunction<typeof loader> = ({ data, location }) => {
return [
{ title: `Thunderstore - The ${data?.community.name} Mod Database` },
// other meta tags...
];
};
If you need to handle the asynchronous nature of the data, consider alternative approaches that ensure meta tags are properly placed in the document head while still supporting the suspense pattern.
<Suspense> | |
<Await resolve={community}> | |
{(resolvedValue) => { | |
return ( | |
<> | |
<meta | |
title={`Thunderstore - The ${resolvedValue.name} Mod Database`} | |
/> | |
<meta | |
name="description" | |
content={`Mods for ${resolvedValue.name}`} | |
/> | |
<meta property="og:type" content="website" /> | |
<meta | |
property="og:url" | |
content={`${ | |
getPublicEnvVariables(["VITE_SITE_URL"]).VITE_SITE_URL | |
}${location.pathname}`} | |
/> | |
<meta | |
property="og:title" | |
content={`Thunderstore - The ${resolvedValue.name} Mod Database`} | |
/> | |
<meta | |
property="og:description" | |
content={`Thunderstore is a mod database and API for downloading ${resolvedValue.name} mods`} | |
/> | |
<meta property="og:image:width" content="360" /> | |
<meta property="og:image:height" content="480" /> | |
<meta | |
property="og:image" | |
content={resolvedValue.cover_image_url ?? undefined} | |
/> | |
<meta property="og:site_name" content="Thunderstore" /> | |
</> | |
); | |
}} | |
</Await> | |
</Suspense> | |
export const meta: MetaFunction<typeof loader> = ({ data, location }) => { | |
if (!data?.community) { | |
return []; | |
} | |
const resolvedValue = data.community; | |
const siteUrl = getPublicEnvVariables(["VITE_SITE_URL"]).VITE_SITE_URL; | |
return [ | |
{ | |
title: `Thunderstore - The ${resolvedValue.name} Mod Database` | |
}, | |
{ | |
name: "description", | |
content: `Mods for ${resolvedValue.name}` | |
}, | |
{ property: "og:type", content: "website" }, | |
{ | |
property: "og:url", | |
content: `${siteUrl}${location.pathname}` | |
}, | |
{ | |
property: "og:title", | |
content: `Thunderstore - The ${resolvedValue.name} Mod Database` | |
}, | |
{ | |
property: "og:description", | |
content: `Thunderstore is a mod database and API for downloading ${resolvedValue.name} mods` | |
}, | |
{ property: "og:image:width", content: "360" }, | |
{ property: "og:image:height", content: "480" }, | |
{ | |
property: "og:image", | |
content: resolvedValue.cover_image_url ?? undefined | |
}, | |
{ property: "og:site_name", content: "Thunderstore" } | |
]; | |
}; |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/cyberstorm-remix/app/root.tsx (1)
221-226
: Sanitize window.ENV injection to prevent XSS.Unescaped JSON inside a <script> can break out via </script> or U+2028/2029. Escape before injecting.
- <script - dangerouslySetInnerHTML={{ - __html: `window.ENV = ${JSON.stringify( - data.publicEnvVariables.ENV - )}`, - }} - /> + <script + dangerouslySetInnerHTML={{ + __html: (() => { + const json = JSON.stringify(data.publicEnvVariables.ENV) + .replace(/</g, "\\u003C") + .replace(/\u2028/g, "\\u2028") + .replace(/\u2029/g, "\\u2029"); + return `window.ENV = ${json};`; + })(), + }} + />apps/cyberstorm-remix/app/commonComponents/PackageSearch/PackageSearch.tsx (1)
483-490
: Rated packages never load on initial render when user is already logged in.
useRef(currentUser)
prevents the effect from firing on mount. Fetch once on mount whencurrentUser
exists.const currentUserRef = useRef(currentUser); useEffect(() => { + // initial load + if (currentUser?.username) { + fetchAndSetRatedPackages(); + } + }, []); + + useEffect(() => { if (currentUserRef.current !== currentUser && currentUser?.username) { fetchAndSetRatedPackages(); currentUserRef.current = currentUser; } }, [currentUser]);
🧹 Nitpick comments (9)
apps/cyberstorm-remix/app/root.tsx (2)
15-16
: Remove dead import comment.Drop the commented
// useRouteLoaderData,
to avoid confusion.- // useRouteLoaderData,
271-279
: Switch to useLoaderData looks good; verify typing.
useLoaderData<RootLoadersType>()
assumes loaders return the same shape. If they ever diverge, preferuseLoaderData<typeof loader | typeof clientLoader>()
for tighter inference.apps/cyberstorm-remix/app/commonComponents/PackageSearch/PackageSearch.tsx (5)
299-310
: Effect uses navigationType but doesn’t list it as a dependency.Add it to avoid stale reads on back/forward navigations.
- }, [searchParams]); + }, [searchParams, navigationType]);
311-475
: Same here: include navigationType in deps.Prevents logic running with an outdated navigation mode.
- }, [debouncedSearchParamsBlob]); + }, [debouncedSearchParamsBlob, navigationType]);
586-600
: Consolidate Await/Suspense to reduce triple fallback flashes.You’re awaiting the same promise three times. Consider one boundary higher up and pass the resolved data down to Count, Grid, and Pagination to avoid repeated “Loading...” flicker.
Also applies to: 612-683, 684-699
586-600
: Surface an error state instead of empty fragments.
errorElement={<></>}
swallows errors silently. Show a minimal failure message and/or retry.- <Await resolve={listings} errorElement={<></>}> + <Await resolve={listings} errorElement={<div className="package-search__error">Failed to load results.</div>}>Also applies to: 612-683, 684-699
114-123
: Avoid empty string entries when parsing categories.Filter falsy IDs after
split
to be safe.- const iCArr = includedCategories.split(","); - const eCArr = excludedCategories.split(","); + const iCArr = includedCategories.split(",").filter(Boolean); + const eCArr = excludedCategories.split(",").filter(Boolean);apps/cyberstorm-remix/app/c/community.tsx (2)
186-204
: Hero rendering via Suspense is fine. Consider using defer for clarity.Using React Router
defer
communicates intent and streams better. Not mandatory, but it simplifies loaders and typing.// Example import { defer } from "react-router"; return defer({ community: dapper.getCommunity(params.communityId), listings: dapper.getPackageListings(/* ... */), filters, // awaited before sortedSections, });
214-223
: Minor: “Loading” breadcrumb fallback.Use a consistent fallback (e.g., “Loading…” with ellipsis) or a skeleton to avoid layout shift.
- <Suspense fallback={<>Loading</>}> + <Suspense fallback={<>Loading…</>}>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
apps/cyberstorm-remix/app/c/community.tsx
(6 hunks)apps/cyberstorm-remix/app/commonComponents/PackageSearch/PackageSearch.tsx
(5 hunks)apps/cyberstorm-remix/app/commonComponents/PageHeader/PageHeader.css
(0 hunks)apps/cyberstorm-remix/app/root.tsx
(2 hunks)
💤 Files with no reviewable changes (1)
- apps/cyberstorm-remix/app/commonComponents/PageHeader/PageHeader.css
🧰 Additional context used
🧬 Code graph analysis (2)
apps/cyberstorm-remix/app/commonComponents/PackageSearch/PackageSearch.tsx (3)
packages/dapper/src/types/package.ts (1)
PackageListings
(20-20)apps/cyberstorm-remix/app/commonComponents/PackageSearch/components/PackageCount/PackageCount.tsx (1)
PackageCount
(19-36)packages/cyberstorm/src/newComponents/Card/CardPackage/CardPackage.tsx (1)
CardPackage
(40-271)
apps/cyberstorm-remix/app/c/community.tsx (3)
apps/cyberstorm-remix/app/p/packageListing.tsx (1)
meta
(85-131)apps/cyberstorm-remix/cyberstorm/security/publicEnvVariables.ts (1)
getPublicEnvVariables
(17-35)apps/cyberstorm-remix/app/commonComponents/PageHeader/PageHeader.tsx (1)
PageHeader
(17-81)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Generate visual diffs
- GitHub Check: Test
- GitHub Check: CodeQL
const filters = await dapper.getCommunityFilters(params.communityId); | ||
const sortedSections = filters.sections.sort( | ||
(a, b) => b.priority - a.priority | ||
); | ||
return { | ||
community: community, | ||
filters: filters, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Await community on the server loader to guarantee meta data.
Keep the UI using Suspense if desired, but resolve community
on the server so the meta
export has the data during SSR. Client loader can keep returning a Promise.
- const community = dapper.getCommunity(params.communityId);
+ const community = await dapper.getCommunity(params.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.
const filters = await dapper.getCommunityFilters(params.communityId); | |
const sortedSections = filters.sections.sort( | |
(a, b) => b.priority - a.priority | |
); | |
return { | |
community: community, | |
filters: filters, | |
const community = await dapper.getCommunity(params.communityId); | |
const filters = await dapper.getCommunityFilters(params.communityId); | |
const sortedSections = filters.sections.sort( | |
(a, b) => b.priority - a.priority | |
); | |
return { | |
community: community, | |
filters: filters, | |
}; |
🤖 Prompt for AI Agents
In apps/cyberstorm-remix/app/c/community.tsx around lines 50 to 56, the loader
returns a Promise for `community` which prevents `meta` from having
server-resolved data during SSR; change the loader to await the community fetch
before returning (i.e., await dapper.getCommunity(params.communityId) and assign
the resolved object to `community`) so the returned object contains concrete
community data for meta generation, while keeping the client-side code able to
return a Promise if desired.
<Await resolve={community}> | ||
{(resolvedValue) => { | ||
return ( | ||
<> | ||
<meta | ||
title={`Thunderstore - The ${resolvedValue.name} Mod Database`} | ||
/> | ||
<meta | ||
name="description" | ||
content={`Mods for ${resolvedValue.name}`} | ||
/> | ||
<meta property="og:type" content="website" /> | ||
<meta | ||
property="og:url" | ||
content={`${ | ||
getPublicEnvVariables(["VITE_SITE_URL"]).VITE_SITE_URL | ||
}${location.pathname}`} | ||
/> | ||
<meta | ||
property="og:title" | ||
content={`Thunderstore - The ${resolvedValue.name} Mod Database`} | ||
/> | ||
<meta | ||
property="og:description" | ||
content={`Thunderstore is a mod database and API for downloading ${resolvedValue.name} mods`} | ||
/> | ||
<meta property="og:image:width" content="360" /> | ||
<meta property="og:image:height" content="480" /> | ||
<meta | ||
property="og:image" | ||
content={resolvedValue.cover_image_url ?? undefined} | ||
/> | ||
<meta property="og:site_name" content="Thunderstore" /> | ||
</> | ||
); | ||
}} | ||
</Await> | ||
</Suspense> | ||
|
||
<Suspense> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Meta tags rendered in the body won’t set document metadata. Reintroduce route meta.
Placing (and a “title” attribute) in the body is invalid and breaks SEO/sharing. Use a route meta
export so in root can render these into .
- <Suspense>
- <Await resolve={community}>
- {(resolvedValue) => {
- return (
- <>
- <meta
- title={`Thunderstore - The ${resolvedValue.name} Mod Database`}
- />
- <meta
- name="description"
- content={`Mods for ${resolvedValue.name}`}
- />
- <meta property="og:type" content="website" />
- <meta
- property="og:url"
- content={`${getPublicEnvVariables(["VITE_SITE_URL"]).VITE_SITE_URL}${location.pathname}`}
- />
- <meta
- property="og:title"
- content={`Thunderstore - The ${resolvedValue.name} Mod Database`}
- />
- <meta
- property="og:description"
- content={`Thunderstore is a mod database and API for downloading ${resolvedValue.name} mods`}
- />
- <meta property="og:image:width" content="360" />
- <meta property="og:image:height" content="480" />
- <meta
- property="og:image"
- content={resolvedValue.cover_image_url ?? undefined}
- />
- <meta property="og:site_name" content="Thunderstore" />
- </>
- );
- }}
- </Await>
- </Suspense>
Add a proper meta export (outside this hunk):
import type { MetaFunction } from "react-router";
export const meta: MetaFunction<typeof loader | typeof clientLoader> = ({
data,
location,
}) => {
const site = getPublicEnvVariables(["VITE_SITE_URL"]).VITE_SITE_URL;
const name =
// supports server loader resolving the community or client loader promise
(data as any)?.community?.name ?? undefined;
return [
{
title: name
? `Thunderstore - The ${name} Mod Database`
: "Thunderstore - The Mod Database",
},
{
name: "description",
content: name ? `Mods for ${name}` : "Thunderstore mod database",
},
{ property: "og:type", content: "website" },
{ property: "og:url", content: `${site}${location.pathname}` },
{
property: "og:title",
content: name ? `Thunderstore - The ${name} Mod Database` : "Thunderstore",
},
{
property: "og:description",
content: name
? `Thunderstore is a mod database and API for downloading ${name} mods`
: "Thunderstore is a mod database and API",
},
{ property: "og:site_name", content: "Thunderstore" },
];
};
🤖 Prompt for AI Agents
In apps/cyberstorm-remix/app/c/community.tsx around lines 146–185, the component
is rendering <meta> tags inside the body which won’t set document head metadata;
remove these body meta tags and add a route-level export named meta (using the
MetaFunction type) outside this component file/hunk that builds and returns an
array of meta objects: derive site URL from
getPublicEnvVariables(["VITE_SITE_URL"]).VITE_SITE_URL, resolve the community
name from data (supporting server loader or client promise), and populate title,
description, og:type, og:url (using location.pathname), og:title,
og:description, and og:site_name accordingly so the app’s <Meta /> in root can
inject proper head tags for SEO/sharing.
Summary by CodeRabbit
New Features
Refactor
Style