-
Notifications
You must be signed in to change notification settings - Fork 8
feat: Console SDK update for version 2.1.0 #51
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThis is a version bump release (2.0.0 to 2.1.0) that introduces two new public methods to the Client class for self-signed TLS and cookie-based authentication configuration. The changes include adding Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/client.ts (1)
375-385: Consider enhancing documentation and clarifying usage context.The
setSelfSignedmethod implementation follows the established pattern for setter methods. However, the documentation could be more descriptive about when and why this should be used, and in which environments (Node.js vs browser) it applies.📝 Suggested documentation enhancement
/** * Set self-signed + * + * Configure whether to allow self-signed SSL certificates. + * Note: This setting may only apply in Node.js environments with custom HTTPS agents. * * @param {boolean} selfSigned * * @returns {this} */
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
CHANGELOG.mdREADME.mddocs/examples/account/update-payment-method.mdpackage.jsonsrc/client.ts
🔇 Additional comments (5)
package.json (1)
5-5: ✓ Version bump looks good.The minor version bump from 2.0.0 to 2.1.0 is appropriate for introducing new public methods without breaking changes.
docs/examples/account/update-payment-method.md (1)
12-12: ✓ Documentation update is appropriate.The expiryYear value is updated to reflect the current year (2026), keeping the example realistic and current.
CHANGELOG.md (1)
3-5: ✓ Changelog entry is well-formatted.The 2.1.0 section properly documents the two new Client class methods and follows the existing changelog style.
src/client.ts (1)
317-317: LGTM!The
cookieconfig default and SDK version header update are correctly implemented and consistent with the version 2.1.0 release.Also applies to: 327-327
README.md (1)
36-36: LGTM!The CDN URL version update correctly reflects the 2.1.0 release.
| selfSigned: boolean; | ||
| [key: string]: string | boolean | undefined; | ||
| } = { | ||
| endpoint: 'https://cloud.appwrite.io/v1', | ||
| endpointRealtime: '', | ||
| selfSigned: false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
rg -n --type=ts -C3 'selfSigned' -g '!CHANGELOG.md'Repository: appwrite/sdk-for-console
Length of output: 1061
🏁 Script executed:
rg -n --type=ts 'config\.selfSigned|this\.config\.selfSigned' -C2Repository: appwrite/sdk-for-console
Length of output: 284
🏁 Script executed:
rg -n --type=ts 'prepareRequest|call\(' src/client.ts | head -20Repository: appwrite/sdk-for-console
Length of output: 651
🏁 Script executed:
sed -n '200,450p' src/client.ts | grep -n -A 30 'prepareRequest'Repository: appwrite/sdk-for-console
Length of output: 50
🏁 Script executed:
sed -n '688,750p' src/client.tsRepository: appwrite/sdk-for-console
Length of output: 2434
🏁 Script executed:
sed -n '795,850p' src/client.tsRepository: appwrite/sdk-for-console
Length of output: 2320
Remove unused selfSigned configuration or implement its functionality.
The selfSigned config property is declared and has a setter method, but it is never actually read or used anywhere in the codebase. The prepareRequest and call methods do not reference it, so calling setSelfSigned() has no effect.
If this is intended for Node.js environments using custom HTTPS agents, the implementation is incomplete. If not needed, remove the property, the setter, and the initialization to eliminate dead code.
| /** | ||
| * Set Cookie | ||
| * | ||
| * The user cookie to authenticate with | ||
| * | ||
| * @param value string | ||
| * | ||
| * @return {this} | ||
| */ | ||
| setCookie(value: string): this { | ||
| this.headers['Cookie'] = value; | ||
| this.config.cookie = value; | ||
| return this; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the setCookie method and surrounding context in src/client.ts
cat -n src/client.ts | sed -n '440,480p'Repository: appwrite/sdk-for-console
Length of output: 1198
🏁 Script executed:
# Check file size to understand if we can read more context
wc -l src/client.tsRepository: appwrite/sdk-for-console
Length of output: 85
🏁 Script executed:
# Search for environment detection or browser-specific code
rg -n "typeof window|process\.env|NODE_ENV|isBrowser|isNode" src/client.ts | head -20Repository: appwrite/sdk-for-console
Length of output: 229
🏁 Script executed:
# Search for other cookie-related code or authentication patterns
rg -n -i "cookie|auth|credentials" src/client.ts | head -30Repository: appwrite/sdk-for-console
Length of output: 1784
🏁 Script executed:
# Look for any documentation or comments about intended environments
rg -n "browser|server|node\.js|environment" src/client.ts | head -20Repository: appwrite/sdk-for-console
Length of output: 126
🌐 Web query:
Can JavaScript set the Cookie HTTP header in browser fetch API
💡 Result:
Short answer: No — browsers forbid JavaScript from setting a request "Cookie" HTTP header in fetch. The Fetch spec and browser implementations treat "Cookie" (and "Set‑Cookie" on responses) as forbidden; the browser controls sending cookies. [1][2]
What to use instead:
- To send existing cookies with fetch to the request origin (or cross‑origin when allowed), set fetch(..., { credentials: 'same-origin' | 'include' }). [2]
- To create/modify cookies from page script for same‑origin (non‑HttpOnly) cookies, use document.cookie. HttpOnly cookies must be set by the server via Set‑Cookie. [1][2]
Sources:
- MDN: Headers / Set-Cookie / forbidden header behavior. [1]
- Explanation & examples (including credentials) on Stack Overflow. [2]
🏁 Script executed:
# Search for setCookie usage throughout the codebase
rg -n "setCookie" --type ts --type jsRepository: appwrite/sdk-for-console
Length of output: 122
🏁 Script executed:
# Check how config.cookie is used, especially in request execution
rg -n "config\.cookie|this\.config\.cookie" src/client.ts -A 2 -B 2Repository: appwrite/sdk-for-console
Length of output: 227
🏁 Script executed:
# Look at the request execution logic to see how headers are processed
cat -n src/client.ts | sed -n '700,750p'Repository: appwrite/sdk-for-console
Length of output: 2286
🏁 Script executed:
# Search for any comments or documentation about server vs browser usage
rg -n "server|node|browser|environment" src/ -B 2 -A 2 | head -40Repository: appwrite/sdk-for-console
Length of output: 4317
🏁 Script executed:
# Let's check if config.cookie is actually used anywhere in the request building or execution
rg -n "config\.cookie" src/ -B 2 -A 2Repository: appwrite/sdk-for-console
Length of output: 297
🏁 Script executed:
# Check the headers object usage in the actual request
cat -n src/client.ts | sed -n '670,710p'Repository: appwrite/sdk-for-console
Length of output: 1737
🏁 Script executed:
# Look for README or documentation that explains browser vs server usage
find . -maxdepth 2 -type f \( -name "README*" -o -name "*.md" \) | head -10Repository: appwrite/sdk-for-console
Length of output: 94
🏁 Script executed:
# Check if there's any initialization or documentation in index.ts or main entry point
find . -maxdepth 2 -type f -name "index.ts" -o -name "package.json" | xargs ls -laRepository: appwrite/sdk-for-console
Length of output: 195
🏁 Script executed:
# Check package.json for project purpose and target environments
cat package.jsonRepository: appwrite/sdk-for-console
Length of output: 1203
🏁 Script executed:
# Check README for documentation about usage
head -100 README.mdRepository: appwrite/sdk-for-console
Length of output: 3890
🏁 Script executed:
# Check how headers are passed to fetch, and if Cookie header is actually sent
cat -n src/client.ts | sed -n '755,800p'Repository: appwrite/sdk-for-console
Length of output: 2007
🏁 Script executed:
# Search for any Node.js specific imports or features
rg -n "require|import.*node|globalThis|Buffer" src/client.ts | head -20Repository: appwrite/sdk-for-console
Length of output: 345
🏁 Script executed:
# Let's confirm if config.cookie is ever used - search the entire codebase
rg -n "config\.cookie" --type ts --type jsRepository: appwrite/sdk-for-console
Length of output: 121
🏁 Script executed:
# Check the call method to see if headers are actually used in the fetch call
cat -n src/client.ts | sed -n '795,825p'Repository: appwrite/sdk-for-console
Length of output: 1509
🏁 Script executed:
# Let's verify what gets passed to fetch as headers
cat -n src/client.ts | sed -n '688,708p'Repository: appwrite/sdk-for-console
Length of output: 924
Document that setCookie is for server-side/Node.js usage only.
The setCookie method sets the Cookie HTTP header, which cannot be set by JavaScript in browser environments—it's a forbidden header name in the Fetch API. The browser handles cookies automatically via the credentials: 'include' option (already implemented at line 706), so this method won't function in browser contexts.
Since this SDK supports both browser and server-side usage, clarify either in documentation or with an environment check that setCookie is for Node.js usage only. Alternatively, consider removing the unused config.cookie property (line 464) if this method is deprecated.
🤖 Prompt for AI Agents
In src/client.ts around lines 453 to 466, document or enforce that setCookie is
for server-side/Node.js only because browsers forbid setting the Cookie header;
either (a) add a runtime environment check (e.g., detect window/fetch
environment) that throws or logs a clear error/warning when called in a browser
and preserve behavior for Node, or (b) mark the method with a JSDoc comment
indicating server-side only and remove the unused config.cookie property if you
choose to deprecate the method; implement one of these fixes and update
tests/docs accordingly.
This PR contains updates to the Console SDK for version 2.1.0.
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.