chore: upgrade and unpin h3 in start-server-core#7044
chore: upgrade and unpin h3 in start-server-core#7044ulrichstark wants to merge 2 commits intoTanStack:mainfrom
Conversation
📝 WalkthroughWalkthroughUpdated Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 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)
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 |
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx affected --targets=test:eslint,test:unit,tes... |
❌ Failed | 11m 2s | View ↗ |
nx run-many --target=build --exclude=examples/*... |
✅ Succeeded | 1m 54s | View ↗ |
☁️ Nx Cloud last updated this comment at 2026-03-26 07:55:06 UTC
Bundle Size Benchmarks
Trend sparkline is historical gzip bytes ending with this PR measurement; lower is better. |
![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
🧹 Nitpick comments (1)
packages/start-server-core/package.json (1)
85-85: Dependency is upgraded, but still pinned exactly.Line 85 uses an exact version (
2.0.1-rc.19). If the intent is to also unpin, switch to the repo’s preferred range style (for example^2.0.1-rc.19).Suggested change
- "h3": "2.0.1-rc.19", + "h3": "^2.0.1-rc.19",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/start-server-core/package.json` at line 85, The h3 dependency is pinned to an exact version ("h3": "2.0.1-rc.19"); change that entry in package.json to use the repo's preferred semver range (e.g., "h3": "^2.0.1-rc.19") so the dependency is unpinned and follow-up package installs pick up compatible updates.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/start-server-core/src/request-response.ts`:
- Around line 307-309: The getCookie implementation is converting empty-string
cookie values to undefined by using "cookies[name] || undefined"; update
getCookie to return the indexed value directly (i.e., return cookies[name]) so
that explicit empty values are preserved; locate the getCookie function where
getH3Event() and h3_parseCookies(event) are used and remove the "|| undefined"
fallback.
---
Nitpick comments:
In `@packages/start-server-core/package.json`:
- Line 85: The h3 dependency is pinned to an exact version ("h3":
"2.0.1-rc.19"); change that entry in package.json to use the repo's preferred
semver range (e.g., "h3": "^2.0.1-rc.19") so the dependency is unpinned and
follow-up package installs pick up compatible updates.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e2a7c156-ee4d-481d-be81-705786a19f55
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (2)
packages/start-server-core/package.jsonpackages/start-server-core/src/request-response.ts
| const event = getH3Event() | ||
| const cookies = h3_parseCookies(event) | ||
| return cookies[name] || undefined |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check the file exists and view lines 307-309
wc -l packages/start-server-core/src/request-response.ts
sed -n '305,315p' packages/start-server-core/src/request-response.tsRepository: TanStack/router
Length of output: 407
🏁 Script executed:
# Let's also look for the getCookie function definition and context
sed -n '305,320p' packages/start-server-core/src/request-response.tsRepository: TanStack/router
Length of output: 493
🏁 Script executed:
# Search for where h3_parseCookies is imported from
rg "h3_parseCookies" packages/start-server-core/src/request-response.ts -B 5 -A 5Repository: TanStack/router
Length of output: 1108
🏁 Script executed:
# Check package.json to find h3 version
rg '"h3"' packages/start-server-core/package.jsonRepository: TanStack/router
Length of output: 83
🏁 Script executed:
# Search for any tests or usage examples of getCookie with empty values
rg "getCookie" packages/start-server-core --type ts -A 3 -B 1 | head -50Repository: TanStack/router
Length of output: 1351
🏁 Script executed:
# Check if there are any comments or documentation about cookie behavior
rg "cookie" packages/start-server-core/src/request-response.ts -i -B 2 -A 2Repository: TanStack/router
Length of output: 3291
🏁 Script executed:
# Search for tests related to getCookie function
find packages/start-server-core -name "*.test.*" -o -name "*.spec.*" | xargs rg "getCookie|cookie" 2>/dev/null | head -30Repository: TanStack/router
Length of output: 41
🏁 Script executed:
# Check if there's any TypeScript type definition or JSDoc for parseCookies
rg "parseCookies" --type ts -B 5 -A 10 | head -50Repository: TanStack/router
Length of output: 3939
🌐 Web query:
h3 parseCookies TypeScript return type empty string values
💡 Result:
In the h3 library (unjs/h3), the parseCookies(event) utility parses the Cookie header from the request and returns a Record<string, string> (object mapping cookie names to their values as strings). Cookie values are always strings, including empty strings ("") if the cookie was set with an empty value (e.g., name=; Path=/). This is standard behavior for Cookie header parsing using libraries like cookie (jshttp/cookie), which h3 relies on internally. TypeScript treats Record<string, string> as all values being non-nullable strings, so empty string values are fully compatible—no type errors occur. If no cookies are present, it returns {} (empty object), which is also valid. Examples from h3 docs and source: - parseCookies(event) → { cookieName: "cookieValue" } or { cookieName: "" } - getCookie(event, name) → string | undefined (wrapper around parseCookies) No issues or bugs reported specifically about empty string values in TypeScript context. If experiencing type mismatches, ensure proper typing: const cookies: Record<string, string> = parseCookies(event);. For source confirmation, early h3 versions used parse(req.headers.cookie || '') from 'cookie', returning Record<string, string>. Current versions maintain this signature per docs and utils pages.
Citations:
- 1: https://h3.dev/utils/cookie
- 2: https://github.com/unjs/h3/blob/10eab1b237b0e01eb9337da653faafb9b9616905/src/utils/cookie.ts
- 3: h3js/h3@088f413
- 4: https://www.h3.dev/examples/handle-cookie
- 5: https://h3.unjs.io/utils/advanced
Preserve empty cookie values in getCookie().
Line 309 uses || undefined, which converts valid empty cookie values ('') into undefined. This loses information about whether a cookie was explicitly set to an empty value versus not being set at all. Return the indexed value directly to preserve this distinction.
Suggested fix
export function getCookie(name: string): string | undefined {
const event = getH3Event()
const cookies = h3_parseCookies(event)
- return cookies[name] || undefined
+ return cookies[name]
}📝 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.
| const event = getH3Event() | |
| const cookies = h3_parseCookies(event) | |
| return cookies[name] || undefined | |
| export function getCookie(name: string): string | undefined { | |
| const event = getH3Event() | |
| const cookies = h3_parseCookies(event) | |
| return cookies[name] | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/start-server-core/src/request-response.ts` around lines 307 - 309,
The getCookie implementation is converting empty-string cookie values to
undefined by using "cookies[name] || undefined"; update getCookie to return the
indexed value directly (i.e., return cookies[name]) so that explicit empty
values are preserved; locate the getCookie function where getH3Event() and
h3_parseCookies(event) are used and remove the "|| undefined" fallback.
|
The e2e test "un-matched malformed paths should return not found on direct navigation" is failing probably because of how h3 now handles malformed paths. I'm not sure what's the best way to continue. I will let the other more experienced contributors decide on how to continue with this PR. Also no worries if my approach is incorrect and you want to close the PR. |
1 participant
Closes #7043
The function
h3_parseCookiesnow returnsRecord<string, string | undefined>instead ofRecord<string, string>. I handled that by filtering out theundefinedvalues to keep the function signature unchanged to avoid breaking changes. Also inlined code fromgetCookiesintogetCookieto avoid the unnecessary execution of theforloop.Summary by CodeRabbit
Chores
Bug Fixes