feat: add isomorphic getCookie/setCookie to react-start, solid-start, vue-start#7724
Open
eranpenso wants to merge 12 commits into
Open
feat: add isomorphic getCookie/setCookie to react-start, solid-start, vue-start#7724eranpenso wants to merge 12 commits into
eranpenso wants to merge 12 commits into
Conversation
Spec for exposing createIsomorphicFn-based getCookie/setCookie from the root export of react-start, solid-start, and vue-start.
Task-by-task plan for docs/superpowers/specs/2026-07-01-isomorphic-cookies-design.md.
Contributor
|
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:
📝 WalkthroughWalkthroughAdds shared isomorphic cookie helpers for the start packages, re-exports them from the package entrypoints, adds tests and build script updates, and documents the new APIs in the start guides. ChangesIsomorphic Cookie Helpers
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
…for isomorphic cookies
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/react-start/src/cookies.ts (1)
9-27: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDrop the trailing
as GetCookieFncast. The.server()/.client()chain already composes to() => string | undefined, so the assertion is redundant here.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/react-start/src/cookies.ts` around lines 9 - 27, Remove the redundant type assertion from the getCookie export: the createIsomorphicFn().server(getServerCookie).client(...) chain already resolves to the correct GetCookieFn shape, so drop the trailing cast and let TypeScript infer the type. Keep the getCookie symbol and its server/client composition unchanged; only simplify the export to rely on the existing inference.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/solid-start/src/cookies.ts`:
- Around line 39-43: The client-side setCookie implementation currently accepts
the full CookieSerializeOptions even though document.cookie cannot honor
httpOnly, so update createIsomorphicFn().client(setCookie) to explicitly reject
unsupported flags or use a client-specific option type. Make the check in the
setCookie client callback before calling serialize, and ensure the SetCookieFn
shape reflects the server/client asymmetry so callers cannot assume httpOnly
works in the browser.
---
Nitpick comments:
In `@packages/react-start/src/cookies.ts`:
- Around line 9-27: Remove the redundant type assertion from the getCookie
export: the createIsomorphicFn().server(getServerCookie).client(...) chain
already resolves to the correct GetCookieFn shape, so drop the trailing cast and
let TypeScript infer the type. Keep the getCookie symbol and its server/client
composition unchanged; only simplify the export to rely on the existing
inference.
🪄 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: ba0627a6-e6c1-4f69-a0b4-19401194ef9d
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
packages/react-start/package.jsonpackages/react-start/src/cookies.tspackages/react-start/src/index.tspackages/solid-start/package.jsonpackages/solid-start/src/cookies.tspackages/solid-start/src/index.tspackages/vue-start/package.jsonpackages/vue-start/src/cookies.tspackages/vue-start/src/index.ts
eranpenso
force-pushed
the
isomorphic-cookies
branch
from
af9fddd to
eeb141b
Compare
document.cookie cannot honor HttpOnly; a client-side setCookie call with httpOnly: true silently produces a non-HttpOnly cookie. Warn in dev so callers don't mistake this for a protected cookie.
Document the new isomorphic getCookie/setCookie exported from the package root (vs the server-only versions under /server), including the HttpOnly caveat in the browser. Cross-link from Hydration Errors.
Add tests/cookies.test.ts to react-start, solid-start, and vue-start, and wire up a test:unit script (vitest) matching start-client-core's convention. Since createIsomorphicFn's uncompiled runtime fallback always resolves to the .server() implementation once one is registered, the .client() branch is unreachable through the exported getCookie/setCookie outside of a Start-compiled bundle. Extract the client implementations into named getClientCookie/setClientCookie functions (not part of the package's public entry) so they can be tested directly. Also corrects the httpOnly warning message: per the WHATWG cookie spec, browsers discard the entire cookie (not just the HttpOnly attribute) when set via document.cookie with HttpOnly present, which the new tests caught.
Extract the client-side cookie logic (parsing/serializing document.cookie, the httpOnly warning) and the createIsomorphicFn wiring into a new createCookieFns(server) factory in start-client-core, exposed via a new ./cookies subpath (not re-exported from the package root, so it doesn't leak into react-start/solid-start/vue-start's public API). The server implementation is injected as a parameter rather than imported directly, since start-server-core already depends on start-client-core and a static import the other way would be a circular workspace dependency. Each framework's cookies.ts shrinks to a ~25-line file that calls createCookieFns with its own start-server-core getCookie/setCookie. The cookie-es dependency and the client-logic unit tests move to start-client-core accordingly; each framework's remaining test just verifies its own getCookie/setCookie delegate to the right start-server-core functions.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/start-client-core/src/cookies.ts (1)
12-19: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winComment claims these aren't "part of any package's public entry," but they are actually exported from
./cookies.
getClientCookie/setClientCookieare top-level named exports of this module, andpackage.jsonnow maps"./cookies"to this file's build output. Any consumer can thereforeimport { getClientCookie } from '@tanstack/start-client-core/cookies', contradicting the comment's stated intent that these are test-only exports not part of the public surface. If they're meant to stay internal, consider marking them@internalin JSDoc, or exposing them for tests via a separate non-exported test-only entry (e.g. re-export only from a.internal.tsfile not covered by the public export map) instead of relying on a comment.Also applies to: 21-32
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/start-client-core/src/cookies.ts` around lines 12 - 19, The comment above getClientCookie/setClientCookie is incorrect because these functions are publicly exported through the cookies entrypoint, so update the module documentation to reflect their real availability or change the export surface if they should remain internal. If they are intended to be test-only, move the implementations behind a non-public/internal entry and re-export them only for tests; otherwise, keep them as public exports and remove the “not part of any package’s public entry” claim.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/start-client-core/src/cookies.ts`:
- Around line 12-19: The comment above getClientCookie/setClientCookie is
incorrect because these functions are publicly exported through the cookies
entrypoint, so update the module documentation to reflect their real
availability or change the export surface if they should remain internal. If
they are intended to be test-only, move the implementations behind a
non-public/internal entry and re-export them only for tests; otherwise, keep
them as public exports and remove the “not part of any package’s public entry”
claim.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c04c32f2-fb1f-41db-a39f-38d425c645df
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (13)
packages/react-start/package.jsonpackages/react-start/src/cookies.tspackages/react-start/tests/cookies.test.tspackages/solid-start/package.jsonpackages/solid-start/src/cookies.tspackages/solid-start/tests/cookies.test.tspackages/start-client-core/package.jsonpackages/start-client-core/src/cookies.tspackages/start-client-core/tests/cookies.test.tspackages/start-client-core/vite.config.tspackages/vue-start/package.jsonpackages/vue-start/src/cookies.tspackages/vue-start/tests/cookies.test.ts
✅ Files skipped from review due to trivial changes (3)
- packages/vue-start/tests/cookies.test.ts
- packages/solid-start/tests/cookies.test.ts
- packages/react-start/tests/cookies.test.ts
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
1 participant
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
getCookie(name)/setCookie(name, value, options?)exported from the root of@tanstack/react-start,@tanstack/solid-start, and@tanstack/vue-start(previously these names only existed server-only under/server).createIsomorphicFn: the server branch delegates to the existing@tanstack/start-server-coregetCookie/setCookie; the client branch usescookie-es'sparse/serializeagainstdocument.cookie.start-client-core) becausestart-server-corealready depends onstart-client-core, and the reverse would create a circular workspace dependency.Test plan
pnpm --filter @tanstack/react-start run build/test:build(publint + attw) passpnpm --filter @tanstack/solid-start run build/test:buildpasspnpm --filter @tanstack/vue-start run build/test:buildpasscookies.tsverified byte-identical across all three packagesSummary by CodeRabbit
Summary
getCookieandsetCookiehelper APIs for React, Solid, and Vue, available via each package’s top-level exports.httpOnlycookies now warn in development (and won’t set such cookies from the browser).