docs(react/isr): use getResponseHeaders in example
#6265
Conversation
📝 WalkthroughWalkthroughThe documentation for React Start's incremental static regeneration (ISR) guide was updated to demonstrate a refactored approach to header management. The example now imports and uses Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
Pre-merge checks✅ Passed checks (3 passed)
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 |
|
View your CI Pipeline Execution ↗ for commit 335443d
☁️ Nx Cloud last updated this comment at |
![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
🧹 Nitpick comments (1)
docs/start/framework/react/guide/isr.md (1)
117-128: Consider adding clarification about whygetResponseHeaders()is used.The documentation now uses
getResponseHeaders()to set cache headers in the middleware example, but doesn't explain why this approach is necessary instead of directly accessingresult.response.headers. Based on the PR objective,result.responsemay be undefined in certain contexts. Adding a brief comment or explanation would help developers understand the reasoning and avoid similar pitfalls.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/start/framework/react/guide/isr.md
🧰 Additional context used
📓 Path-based instructions (1)
docs/**/*.{md,mdx}
📄 CodeRabbit inference engine (AGENTS.md)
Use relative links to
docs/folder format (e.g.,./guide/data-loading) for internal documentation references
Files:
docs/start/framework/react/guide/isr.md
⏰ 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: Preview
🔇 Additional comments (1)
docs/start/framework/react/guide/isr.md (1)
115-115: Update the etagMiddleware example for consistency with the new pattern.The
cacheMiddlewareexample now usesgetResponseHeaders()to set headers, but theetagMiddlewareexample at line 351 still usesresult.response.headers.set(). For consistency and to avoid confusion, the etagMiddleware example should be updated to use the samegetResponseHeaders()approach.Can you confirm whether the etagMiddleware example (line 339-354) should also be updated to use
getResponseHeaders()instead ofresult.response.headers? If so, please update it to:const etagMiddleware = createMiddleware().server(async ({ next }) => { const result = await next() // Generate ETag from response content const etag = crypto .createHash('md5') .update(JSON.stringify(result.data)) .digest('hex') - result.response.headers.set('ETag', `"${etag}"`) + const headers = getResponseHeaders() + headers.set('ETag', `"${etag}"`) return result })(Note: You may also need to add
import { getResponseHeaders } from '@tanstack/react-start/server'at the top of that example if not already imported.)Also applies to: 121-125
3 participants
I've faced an issue when trying to use cache middleware from ISR article.
Currently it uses
result.response.headersto access response headers.However it turns out that
result.responseis undefined, which causes, even though type annotations say that it is not. It seems like a bug, since TypeScript allows a code that leads to a runtime error.See commit 6c6ec1b and screenshot below.
Then I tried to use
result.headers, which is not present inRequestServerResult. Setting them did not work either. See commit 3b417f6In the end I've tried using
getResponseHeadersand it worked! See commit 52fc400I've updated the ISR article in the docs to use
getResponseHeaders. Not sure what to withresult.responsebeing undefinedSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.