-
Notifications
You must be signed in to change notification settings - Fork 5
Package Version pages #1547
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
base: 09-17-update_thunderstore-api_to_match_up_coming_cyberstorm_api_changes
Are you sure you want to change the base?
Package Version pages #1547
Conversation
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing touches🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal). Please share your feedback with us on this Discord post. 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. Comment |
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
895c5a8
to
4582600
Compare
bd91070
to
e6fba84
Compare
4582600
to
b1e3d73
Compare
e6fba84
to
0ef1a10
Compare
b1e3d73
to
e6817ad
Compare
0ef1a10
to
c9789f0
Compare
e6817ad
to
e6956ff
Compare
e6956ff
to
73cc06b
Compare
Add function for using the endpoint and related schemas
Add packageVersion method for Dapper and DapperFake
This enables componens like NewLink to auto resolve links and it's used props, with the linkId prop. (the method name in LinkLibrary interface)
These pages are more or less the same as PackageListing pages/tabs. The biggest difference being the ommited Team and Package Listing related information as it's not relevant when viewing a package. The "Required" is mostly commented out, because the endpoint isn't yet available. (even for local development testing)
These pages are and should be exactly the same, not including community related features and functionalities. The reason these have to be their own files is that, react-router doesn't support using the same files for two routes at the same time. The "Required" is mostly commented out, because the endpoint isn't yet available. (even for local development testing)
Note that the ones that do not require a community to fetch the package version data, are not under the /c/ path
So that no-community-needing package pages are accessible through the proxy
73cc06b
to
7a9bbdb
Compare
useEffect(() => { | ||
if (!startsHydrated.current && isHydrated) return; | ||
if (isPromise(version)) { | ||
version.then((versionData) => { | ||
setFirstUploaded( | ||
<RelativeTime | ||
time={versionData.datetime_created} | ||
suppressHydrationWarning | ||
/> | ||
); | ||
}); | ||
} else { | ||
setFirstUploaded( | ||
<RelativeTime | ||
time={version.datetime_created} | ||
suppressHydrationWarning | ||
/> | ||
); | ||
} | ||
}, []); |
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.
The useEffect
hook depends on the version
variable but it's not included in the dependency array. This could lead to stale closures if version
changes after the initial render. Consider adding version
to the dependency array:
}, [version]);
This ensures the effect runs when version
changes, keeping the displayed upload time in sync with the current version data.
useEffect(() => { | |
if (!startsHydrated.current && isHydrated) return; | |
if (isPromise(version)) { | |
version.then((versionData) => { | |
setFirstUploaded( | |
<RelativeTime | |
time={versionData.datetime_created} | |
suppressHydrationWarning | |
/> | |
); | |
}); | |
} else { | |
setFirstUploaded( | |
<RelativeTime | |
time={version.datetime_created} | |
suppressHydrationWarning | |
/> | |
); | |
} | |
}, []); | |
useEffect(() => { | |
if (!startsHydrated.current && isHydrated) return; | |
if (isPromise(version)) { | |
version.then((versionData) => { | |
setFirstUploaded( | |
<RelativeTime | |
time={versionData.datetime_created} | |
suppressHydrationWarning | |
/> | |
); | |
}); | |
} else { | |
setFirstUploaded( | |
<RelativeTime | |
time={version.datetime_created} | |
suppressHydrationWarning | |
/> | |
); | |
} | |
}, [version]); |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
<meta | ||
title={`${formatToDisplayName( | ||
resolvedValue[0].full_version_name | ||
)} | Thunderstore - The ${resolvedValue[1].name} Mod Database`} | ||
/> |
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.
The title
attribute is being used incorrectly on the meta
tag. For page titles, use the standard <title>
element instead. If using a meta tag for title information, the proper attribute would be name="title"
.
Consider replacing:
<meta
title={`${formatToDisplayName(
resolvedValue[0].full_version_name
)} | Thunderstore - The ${resolvedValue[1].name} Mod Database`}
/>
with:
<title>
{formatToDisplayName(resolvedValue[0].full_version_name)} | Thunderstore - The {resolvedValue[1].name} Mod Database
</title>
This ensures proper SEO and browser display of the page title.
<meta | |
title={`${formatToDisplayName( | |
resolvedValue[0].full_version_name | |
)} | Thunderstore - The ${resolvedValue[1].name} Mod Database`} | |
/> | |
<title>{`${formatToDisplayName( | |
resolvedValue[0].full_version_name | |
)} | Thunderstore - The ${resolvedValue[1].name} Mod Database`}</title> | |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
Look at the commits for more information