-
Couldn't load subscription status.
- Fork 63
feat: Web SDK update for version 21.3.0 #146
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 PR updates the package to 21.3.0, bumps SDK version strings and docs, deprecates Client.subscribe in favor of a new Realtime service, and refactors Client.config to a stricter typed shape. It changes realtime connection/session resolution to prefer an explicit config.session over localStorage fallback and only includes project in socket queries when present. A new src/services/realtime.ts adds a full WebSocket-based Realtime class with subscription APIs, heartbeat, reconnection/backoff, event dispatching, and lifecycle hooks. The Realtime service is exported from src/index.ts. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Areas to focus review on:
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🧰 Additional context used🧬 Code graph analysis (1)src/client.ts (1)
🪛 ast-grep (0.39.6)src/services/realtime.ts[warning] 156-163: Message event listeners should validate the origin to prevent XSS attacks. Always check the event origin before processing the message. (event-origin-validation) [warning] 156-163: Message event listener without origin validation detected. Always validate the event.origin property in message event handlers to prevent XSS attacks from malicious frames or windows. (message-listener-origin-check) 🪛 Biome (2.1.2)src/client.ts[error] 537-537: Other switch clauses can erroneously access this declaration. The declaration is defined in this switch clause: Safe fix: Wrap the declaration in a block. (lint/correctness/noSwitchDeclarations) [error] 543-543: Other switch clauses can erroneously access this declaration. The declaration is defined in this switch clause: Safe fix: Wrap the declaration in a block. (lint/correctness/noSwitchDeclarations) 🔇 Additional comments (11)
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/client.ts (1)
530-546: Wrap case body in braces to satisfy noSwitchDeclarations.
Biome flags declarations inside switch cases. Enclose the'connected'case body in a block.Apply:
- case 'connected': - let session = this.config.session; - if (!session) { - const cookie = JSON.parse(window.localStorage.getItem('cookieFallback') ?? '{}'); - session = cookie?.[`a_session_${this.config.project}`]; - } - - const messageData = <RealtimeResponseConnected>message.data; + case 'connected': { + let session = this.config.session; + if (!session) { + const cookie = JSON.parse(window.localStorage.getItem('cookieFallback') ?? '{}'); + session = cookie?.[`a_session_${this.config.project}`]; + } + + const messageData = <RealtimeResponseConnected>message.data; if (session && !messageData.user) { this.realtime.socket?.send(JSON.stringify(<RealtimeRequest>{ type: 'authentication', data: { session } })); } - break; + break; + }
🧹 Nitpick comments (4)
CHANGELOG.md (1)
3-10: Add deprecation timeline and migration pointer.
Briefly note expected removal version forclient.subscribeand link to a migration snippet usingRealtime.subscribe(...)to reduce ambiguity.src/index.ts (1)
19-20: Export subscription-related types at top-level.
ExposeRealtimeSubscription(and optionallyRealtimeCode) to avoid deep imports and improve DX.Apply this minimal addition:
export { Realtime } from './services/realtime'; +export type { RealtimeSubscription } from './services/realtime'; +export { RealtimeCode } from './services/realtime';src/client.ts (1)
588-590: Clarify deprecation with link and timeline.
Add a link to the newRealtimeusage and note the planned removal version to set expectations.src/services/realtime.ts (1)
275-312: Simplify subscribe batching using microtasks.
Replacesleep(1)/depth tracking with a microtask batch to reduce timing fragility.Example:
- this.subCallDepth++; - await this.sleep(this.DEBOUNCE_MS); - if (this.subCallDepth === 1) { - await this.createSocket(); - } - this.subCallDepth--; + if (!this._pendingConnect) { + this._pendingConnect = Promise.resolve().then(() => this.createSocket()).finally(() => { this._pendingConnect = undefined; }); + } + await this._pendingConnect;Add a private
_pendingConnect?: Promise<void>;.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
CHANGELOG.md(1 hunks)README.md(1 hunks)package.json(1 hunks)src/client.ts(3 hunks)src/index.ts(1 hunks)src/services/realtime.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/services/realtime.ts (2)
src/client.ts (3)
RealtimeResponseEvent(810-810)Client(807-807)AppwriteException(807-807)src/index.ts (4)
RealtimeResponseEvent(20-20)Realtime(19-19)Client(8-8)AppwriteException(8-8)
🪛 ast-grep (0.39.6)
src/services/realtime.ts
[warning] 143-150: Message event listener without origin validation detected. Always validate the event.origin property in message event handlers to prevent XSS attacks from malicious frames or windows.
Context: this.socket.addEventListener('message', (event: MessageEvent) => {
try {
const message = JSON.parse(event.data) as RealtimeResponse;
this.handleMessage(message);
} catch (error) {
console.error('Failed to parse message:', error);
}
})
Note: [CWE-346] Origin Validation Error [REFERENCES]
- https://developer.mozilla.org/en-US/docs/Web/API/Window/postMessage
- https://cwe.mitre.org/data/definitions/346.html
(message-listener-origin-check)
[warning] 143-150: Message event listeners should validate the origin to prevent XSS attacks. Always check the event origin before processing the message.
Context: this.socket.addEventListener('message', (event: MessageEvent) => {
try {
const message = JSON.parse(event.data) as RealtimeResponse;
this.handleMessage(message);
} catch (error) {
console.error('Failed to parse message:', error);
}
})
Note: [CWE-346] Origin Validation Error [REFERENCES]
- https://cheatsheetseries.owasp.org/cheatsheets/Cross_Site_Scripting_Prevention_Cheat_Sheet.html
- https://developer.mozilla.org/en-US/docs/Web/API/Window/postMessage
(event-origin-validation)
🪛 Biome (2.1.2)
src/client.ts
[error] 531-531: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 537-537: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🔇 Additional comments (4)
README.md (1)
36-36: CDN version bump looks good.package.json (1)
5-5: Version bump to 21.3.0 is consistent.src/client.ts (1)
319-321: Header version update is correct.src/services/realtime.ts (1)
3-10: Confirm intended public surface for types.
If consumers should typeclose()results, keepRealtimeSubscriptionexported and re-export it fromsrc/index.ts(see related comment). Otherwise, make it internal to avoid API bloat.
This PR contains updates to the Web SDK for version 21.3.0.
Summary by CodeRabbit
New Features
Deprecations
client.subscribedeprecated; Realtime service is the recommended replacement (backwards compatibility retained).Bug Fixes
Other