Fix package like action usage in package page #1588
Conversation
As the package like action uses a toast, it needs the provided to be present, when it's fired up. This means that it can't be a globally "in the file" defined function, but rather has to be defined within the route function, so that it has access to the provider when it's initializedFix package like action usage in package page As the package like action uses a toast, it needs the provided to be present, when it's fired up. This means that it can't be a globally "in the file" defined function, but rather has to be defined within the route function, so that it has access to the provider when it's initialized.
WalkthroughThe Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1588 +/- ##
======================================
Coverage 9.89% 9.89%
======================================
Files 308 308
Lines 22497 22482 -15
Branches 404 404
======================================
Hits 2225 2225
+ Misses 20272 20257 -15 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/cyberstorm-remix/app/p/packageListing.tsx (1)
293-297: Bug: og:url builds “[object Object]/path” instead of a full URL.getPublicEnvVariables returns an object; access the VITE_BETA_SITE_URL key.
- <meta - property="og:url" - content={`${getPublicEnvVariables(["VITE_BETA_SITE_URL"])}${location.pathname}`} - /> + <meta + property="og:url" + content={`${getPublicEnvVariables(["VITE_BETA_SITE_URL"]).VITE_BETA_SITE_URL}${location.pathname}`} + />
🧹 Nitpick comments (5)
apps/cyberstorm-remix/app/p/packageListing.tsx (5)
193-209: Stabilize dataUpdateTrigger with useCallback.This avoids recreating the trigger each render and reduces stale-closure risk.
- const fetchAndSetRatedPackages = async () => { + const fetchAndSetRatedPackages = useCallback(async () => { const rp = await dapper.getRatedPackages(); if (isPromise(listing)) { listing.then((listingData) => { setIsLiked( rp.rated_packages.includes( `${listingData.namespace}-${listingData.name}` ) ); }); } else { setIsLiked( rp.rated_packages.includes(`${listing.namespace}-${listing.name}`) ); } - }; + }, [dapper, listing]);Also add the import:
-import { - memo, - type ReactElement, - Suspense, - useEffect, - useMemo, - useRef, - useState, -} from "react"; +import { + memo, + type ReactElement, + Suspense, + useEffect, + useMemo, + useRef, + useState, + useCallback, +} from "react";
210-215: Include the stabilized trigger in effect deps.Ensures the effect responds if the trigger changes (e.g., route swap).
- }, [currentUser]); + }, [currentUser, fetchAndSetRatedPackages]);
438-445: Prop pass-through looks correct in both render paths.Both call sites now pass packageLikeAction consistently. If you want to retain memo benefits for Actions, consider ignoring handler identity in a custom comparator (see note on component below).
Also applies to: 601-607
960-972: Actions props: minor polish to reduce re-renders and ambiguity.
- Optional: rename prop to onPackageLike to avoid name clash with the imported factory and improve readability.
- Optional: add a memo comparator to ignore handler identity churn.
Example comparator:
const Actions = memo(function Actions(props: {...}) { // ... }, (prev, next) => prev.isLiked === next.isLiked && prev.listing === next.listing && prev.team === next.team && (prev.currentUser?.username ?? "") === (next.currentUser?.username ?? "") );
1001-1008: Tooltip should reflect toggle state.Show “Unlike” when already liked.
- tooltipText="Like" + tooltipText={isLiked ? "Unlike" : "Like"}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/cyberstorm-remix/app/p/packageListing.tsx(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/cyberstorm-remix/app/p/packageListing.tsx (2)
packages/cyberstorm-forms/src/actions/PackageLikeAction.tsx (1)
PackageLikeAction(9-56)packages/cyberstorm-forms/src/index.ts (1)
PackageLikeAction(2-2)
⏰ 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). (1)
- GitHub Check: Build
🔇 Additional comments (2)
apps/cyberstorm-remix/app/p/packageListing.tsx (2)
273-278: Good fix: create the like action inside the route component.This aligns with the toast/provider requirement and removes the global handler hazard. Keep this call at component top-level (not inside useMemo/conditions) so the internal hook usage remains valid.
960-971: Verified: All Actions call sites updated correctly.The verification script found no remaining usages of the Actions component with the old props (
likeUpdateTriggerorrequestConfig). TherequestConfigmatches found were in unrelated modules (MultipartUpload, upload context extraction, DeleteAccountForm), not the Actions component. ✓
There was a problem hiding this comment.
so that it has access to the provider when it's
initializedFix package like action usage in package page
Had to read the commit couple of times until I realized that initializedFix is not a name of some variable or function 😅 . So feel free to fix the commit message.
2 participants
As the package like action uses a toast, it needs the provided to be
present, when it's fired up. This means that it can't be a globally "in
the file" defined function, but rather has to be defined within the
route function, so that it has access to the provider when it's
initializedFix package like action usage in package page
As the package like action uses a toast, it needs the provided to be
present, when it's fired up. This means that it can't be a globally "in
the file" defined function, but rather has to be defined within the
route function, so that it has access to the provider when it's
initialized.