fix: follow-ups for #1886 and #1912 audit findings#2043
Merged
Conversation
Removes nine keys added in #1886 under `packTemplates` that were never referenced anywhere in the monorepo: dayHike, overnight, weekendCamping, multiDay, winterHiking, winterCamping, tripDuration, environment, intendedUse.
Previously each of the 6 featured pack cards called usePackTemplateDetails, which materialised the full item list and recomputed base/total weight per card. With a few hundred template items in the store that meant each render iterated the items store 6 times for tiny "24 items" labels. Introduces usePackTemplateSummaries, a batched selector that computes itemCount / baseWeight / totalWeight for a set of template ids in a single pass over packTemplateItemsStore. FeaturedPacksSection now computes summaries once at the parent level and passes them as props, so cards no longer subscribe to the full details hook.
#1912 added featureFlags.enableTrips and wired the tab trigger and home tiles to hide when the flag is off, but the underlying route files still rendered their screens. That meant deep links such as packrat://trip/new or packrat://trip/:id rendered trip UI even with the kill switch off. Adds a Redirect fallback to each app-router entry point: - app/(app)/trip/[id]/index.tsx - app/(app)/trip/new.tsx - app/(app)/upcoming-trips.tsx - app/(app)/(tabs)/trips/index.tsx Feature components under features/trips/* are intentionally left alone — the gating happens at the route level.
Contributor
|
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 ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
Contributor
There was a problem hiding this comment.
Pull request overview
Follow-up changes addressing audit findings from Featured Packs (#1886) and Trips feature-flag gating (#1912) in the Expo app.
Changes:
- Removed unused
packTemplates.*i18n keys fromen.json. - Reduced Featured Packs render cost by batching pack-template item summary computations via a new
usePackTemplateSummarieshook and passing summary props into cards. - Gated several trip route entry points behind
featureFlags.enableTripsusing<Redirect href="/" />to prevent deep-link bypass.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| apps/expo/lib/i18n/locales/en.json | Removes unused Featured Packs-related translation keys. |
| apps/expo/features/pack-templates/hooks/usePackTemplateSummary.ts | Adds per-template and batched summary hooks to avoid materializing full item lists for card stats. |
| apps/expo/features/pack-templates/hooks/index.ts | Exports the new summary hook(s) from the hooks barrel. |
| apps/expo/features/pack-templates/components/FeaturedPacksSection.tsx | Switches Featured Packs cards to use batched summaries and pass computed stats as props. |
| apps/expo/app/(app)/upcoming-trips.tsx | Adds feature-flag redirect gate for the Upcoming Trips route. |
| apps/expo/app/(app)/trip/new.tsx | Adds feature-flag redirect gate for the Trip Create route. |
| apps/expo/app/(app)/trip/[id]/index.tsx | Adds feature-flag redirect gate for the Trip Detail route. |
| apps/expo/app/(app)/(tabs)/trips/index.tsx | Adds feature-flag redirect gate for the Trips tab route. |
Comment on lines
5
to
10
| export default function TripDetailScreenRoute() { | ||
| // Gate deep links behind the trips feature flag so e.g. `packrat://trip/:id` | ||
| // cannot bypass the kill switch. | ||
| if (!featureFlags.enableTrips) return <Redirect href="/" />; | ||
| return <TripDetailScreen />; | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up PR addressing the non-critical issues flagged in an audit of #1886 (Featured Packs) and #1912 (Trips feature flag) after they merged.
#1886 follow-ups
packTemplatesthat were added but never referenced (dayHike,overnight,weekendCamping,multiDay,winterHiking,winterCamping,tripDuration,environment,intendedUse).FeaturedPackCardwith a batchedusePackTemplateSummariesselector. Previously each of 6 cards calledusePackTemplateDetails, which materialised the full item list and recomputed weights for every card on every render.FeaturedPacksSectionnow computes item counts and base/total weight for all featured templates in a single pass and hands them to the cards as props.#1912 follow-ups
/trip/[id],/trip/new,/upcoming-trips,/(tabs)/trips) behindfeatureFlags.enableTrips. Implement Trips feature (behind feature flag) #1912 hid the trips tab trigger and home tiles but left the underlying route files rendering, sopackrat://trip/newstill bypassed the kill switch. Each route now returns<Redirect href="/" />when the flag is off. Trip feature components underfeatures/trips/*are intentionally unchanged — the gating lives at the router entry points.Test plan
bun run check-typescleanbun lintclean (exit 0 — only pre-existing warnings)enableTrips: false, tapping a trip deep link redirects home