Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughChanges refine TTL and cache-control header handling to treat zero and numeric falsy values as explicit thresholds rather than falsy defaults. Cache expiration and staleness checks now activate only when TTL exceeds zero, and cache-control headers properly distinguish between omitted values and explicit zero values using null-aware checks. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/http.ts`:
- Around line 147-150: The non-SWR branch currently uses a truthy check for
opts.maxAge which ignores explicit 0; change the condition to test for
undefined/null (e.g., if (opts.maxAge !== undefined && opts.maxAge !== null)) so
that opts.maxAge === 0 produces "max-age=0" and push into cacheControl the
`max-age=${opts.maxAge}` value; ensure this mirrors the SWR path's handling of
opts.maxAge so explicit zero values are respected.
| } else if (opts.maxAge) { | ||
| // For non-SWR, set max-age directly | ||
| cacheControl.push(`max-age=${opts.maxAge}`); | ||
| } |
There was a problem hiding this comment.
Inconsistent handling of maxAge: 0 in non-SWR path.
Line 147 still uses a truthy check (opts.maxAge), so maxAge: 0 would be ignored and no max-age header would be set. This is inconsistent with the SWR path (line 139) which now respects zero values.
If a user explicitly sets maxAge: 0 without SWR, they likely expect max-age=0 to be emitted.
Proposed fix for consistency
- } else if (opts.maxAge) {
+ } else if (opts.maxAge != null) {
// For non-SWR, set max-age directly
cacheControl.push(`max-age=${opts.maxAge}`);
}📝 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.
| } else if (opts.maxAge) { | |
| // For non-SWR, set max-age directly | |
| cacheControl.push(`max-age=${opts.maxAge}`); | |
| } | |
| } else if (opts.maxAge != null) { | |
| // For non-SWR, set max-age directly | |
| cacheControl.push(`max-age=${opts.maxAge}`); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/http.ts` around lines 147 - 150, The non-SWR branch currently uses a
truthy check for opts.maxAge which ignores explicit 0; change the condition to
test for undefined/null (e.g., if (opts.maxAge !== undefined && opts.maxAge !==
null)) so that opts.maxAge === 0 produces "max-age=0" and push into cacheControl
the `max-age=${opts.maxAge}` value; ensure this mirrors the SWR path's handling
of opts.maxAge so explicit zero values are respected.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@README.md`:
- Around line 259-263: The README's TypeScript signature for
defineCachedFunction is truncated; restore the complete declaration by setting
the default for the opts parameter and the return type to match the source:
update the signature for function defineCachedFunction<T, ArgsT extends
unknown[] = any[]>(fn: (...args: ArgsT) => T | Promise<T>, opts: CacheOptions<T,
ArgsT> = {},): (...args: ArgsT) => Promise<T> so that CacheOptions has a default
{} and the function returns (...args: ArgsT) => Promise<T>; ensure the restored
tokens match the source declaration around defineCachedFunction, CacheOptions,
opts, and the return type.
…y check The previous fix treated both `maxAge: 0` and unset `maxAge` as always-expired since `ttl === 0` is true for both. Now `opts.maxAge === 0` explicitly checks the user's intent, while `undefined` maxAge preserves cache-indefinitely behavior. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2 participants
🔗 Linked issue
Fixes: nuxt/nuxt#34307
Related: nitrojs/nitro#4126
Related: nitrojs/nitro#4125
❓ Type of change
📚 Description
swr: 0set innuxt.config.ts(see linked issue to Nuxt) it should be handled likemax-age=0- but it will be ignored, because Nitro checksif (swr)what is false with 0, too.Here it is the same.
This fix checks explicitly for 0.
📝 Checklist
Summary by CodeRabbit
Release Notes
Bug Fixes
Tests