Merged
Conversation
|
Cursor Agent can help with this pull request. Just |
📝 WalkthroughWalkthroughReplaces static navbar link constants with a hook that derives desktop and mobile links (including optional active-route matching) from loader-provided podcast season data. Adds a server utility that fetches and caches latest podcast season numbers and surfaces them to the root loader. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser
participant RootLoader as Root Loader
participant Cache as Cachified Cache
participant PodcastUtil as getLatestPodcastSeasonLinks
participant Simplecast
participant Transistor
participant App as Client (Navbar)
Browser->>RootLoader: HTTP request
RootLoader->>Cache: read latestPodcastSeasonLinks
alt cache miss
Cache->>PodcastUtil: compute latest links
PodcastUtil->>Simplecast: fetch chats metadata
PodcastUtil->>Transistor: fetch calls metadata
Simplecast-->>PodcastUtil: chats seasons
Transistor-->>PodcastUtil: calls seasons
PodcastUtil-->>Cache: store computed links
Cache-->>RootLoader: return links
else cache hit
Cache-->>RootLoader: return cached links
end
RootLoader-->>Browser: response with latestPodcastSeasonLinks
Browser->>App: render
App->>App: useNavbarLinks(loaderData.latestPodcastSeasonLinks)
App->>App: render NavLink/MobileMenu with links and activeTo
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/root.tsx (1)
111-123: Consider wrappinggetLatestPodcastSeasonLinkswith a fallback in the root loader.Since this runs on every page load, an unexpected error from
cachifieditself (not fromgetFreshValue, which is already guarded) would break the entire root loader and take down the page. A defensive wrapper here would prevent that:🛡️ Suggested defensive wrapper
- getLatestPodcastSeasonLinks({ request, timings }), + getLatestPodcastSeasonLinks({ request, timings }).catch(() => null),The inner helpers already handle fetch failures gracefully, so the risk is low — this is just an extra safety net for cache-infrastructure failures.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/root.tsx` around lines 111 - 123, The root loader currently awaits getLatestPodcastSeasonLinks inside Promise.all which could throw if the cachified/cache infra fails; wrap the call with a defensive fallback so a thrown error doesn't break the whole loader — replace the direct call to getLatestPodcastSeasonLinks({ request, timings }) with a wrapper that catches errors and returns a safe default (e.g., empty links or null) so Promise.all always resolves; reference the getLatestPodcastSeasonLinks call inside the array passed to Promise.all and ensure the fallback shape matches what the rest of the loader expects.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/root.tsx`:
- Around line 111-123: The root loader currently awaits
getLatestPodcastSeasonLinks inside Promise.all which could throw if the
cachified/cache infra fails; wrap the call with a defensive fallback so a thrown
error doesn't break the whole loader — replace the direct call to
getLatestPodcastSeasonLinks({ request, timings }) with a wrapper that catches
errors and returns a safe default (e.g., empty links or null) so Promise.all
always resolves; reference the getLatestPodcastSeasonLinks call inside the array
passed to Promise.all and ensure the fallback shape matches what the rest of the
loader expects.
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
cursor
bot
force-pushed
the
cursor/podcast-latest-season-links-62f6
branch
from
447d516 to
18155bf
Compare
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/utils/podcast-latest-season.server.ts (1)
31-35: Optional: Align defensive coding with the calls version for consistency.Both functions should either both include or both omit the
?? 0guard onseasonNumber. WhilegetLatestCallsSeasonNumberusese.seasonNumber ?? 0,getLatestChatsSeasonNumberpassess.seasonNumberdirectly toMath.max. The Zod schemas defineseasonNumberas a requiredz.number(), so the guard is not strictly necessary; however, aligning the implementations would improve code consistency and defensive coding practices.♻️ Align with the calls version
- const latestSeasonNumber = seasons.reduce( - (max, s) => Math.max(max, s.seasonNumber), - 0, - ) + const latestSeasonNumber = seasons.reduce( + (max, s) => Math.max(max, s.seasonNumber ?? 0), + 0, + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/utils/podcast-latest-season.server.ts` around lines 31 - 35, Align defensive handling of undefined season numbers between getLatestChatsSeasonNumber and getLatestCallsSeasonNumber by changing the reducer in getLatestChatsSeasonNumber to use s.seasonNumber ?? 0 when computing latestSeasonNumber; locate the seasons.reduce call (variable latestSeasonNumber) and update the reducer to mirror the calls implementation so both functions consistently guard against missing seasonNumber values.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/utils/podcast-latest-season.server.ts`:
- Around line 31-35: Align defensive handling of undefined season numbers
between getLatestChatsSeasonNumber and getLatestCallsSeasonNumber by changing
the reducer in getLatestChatsSeasonNumber to use s.seasonNumber ?? 0 when
computing latestSeasonNumber; locate the seasons.reduce call (variable
latestSeasonNumber) and update the reducer to mirror the calls implementation so
both functions consistently guard against missing seasonNumber values.
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/utils/podcast-latest-season.server.ts (1)
5-14: Consider tighteninglatestSeasonNumberschema to integers.
z.number().nullable()accepts floats and negative values. Since season numbers are always positive integers,z.number().int().positive().nullable()would make the schema self-documenting and catch unexpected API responses earlier.♻️ Proposed refinement
const latestPodcastSeasonLinksSchema = z.object({ chats: z.object({ - latestSeasonNumber: z.number().nullable(), + latestSeasonNumber: z.number().int().positive().nullable(), latestSeasonPath: z.string().min(1), }), calls: z.object({ - latestSeasonNumber: z.number().nullable(), + latestSeasonNumber: z.number().int().positive().nullable(), latestSeasonPath: z.string().min(1), }), })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/utils/podcast-latest-season.server.ts` around lines 5 - 14, The schema latestPodcastSeasonLinksSchema currently allows non-integer or negative season numbers because it uses z.number().nullable(); update both chats.latestSeasonNumber and calls.latestSeasonNumber to z.number().int().positive().nullable() so the validator enforces positive integers (or null) and will fail fast on invalid API responses.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/utils/podcast-latest-season.server.ts`:
- Around line 5-14: The schema latestPodcastSeasonLinksSchema currently allows
non-integer or negative season numbers because it uses z.number().nullable();
update both chats.latestSeasonNumber and calls.latestSeasonNumber to
z.number().int().positive().nullable() so the validator enforces positive
integers (or null) and will fail fast on invalid API responses.
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
2 participants
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.
Update podcast navigation links to dynamically point to the latest season.
This ensures users are always directed to the most current podcast content and avoids outdated hardcoded links in the navigation.
Note
Medium Risk
Touches the root loader and adds server-side calls to external podcast providers (even though cached and guarded), so failures/perf regressions could affect all page loads and nav routing.
Overview
Navbar
Chats/Callslinks are now derived at runtime to point to the latest podcast season instead of hardcoded season routes, with mobile and desktop menus sharing this computed link set.Root loader now fetches and exposes
latestPodcastSeasonLinks, backed by a new cached server utility that queries Simplecast/Transistor, validates shape withzod, and falls back to base/chats//callspaths on failure. Active nav highlighting was adjusted via anactiveToprefix so season-specific links still show active for all/chats/*and/calls/*routes.Written by Cursor Bugbot for commit 02aa4f6. This will update automatically on new commits. Configure here.
Summary by CodeRabbit
New Features
Performance