Fix: logic in onMount for realtime. #2519
Conversation
|
This PR was not deployed automatically as @ItzNotABug does not have access to the Railway project. In order to get automatic PR deploys, please add @ItzNotABug to your workspace on Railway. |
ConsoleProject ID: Tip Appwrite has a Discord community with over 16 000 members. |
WalkthroughThis PR replaces subscribe-style realtime usage with a closure-based API: realtime.forConsole(...) and realtime.forProject(...). Many components and pages switch their onMount subscription setup/teardown to use the returned unsubscribe closure. The sdk store is reworked: new RealtimeResponse and AppwriteRealtimeResponseEvent types, createRealtimeSubscription helper, forConsole added, and AppwriteRealtimeSubscription and createAdminClient removed. Several pages move from exporting Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Files/areas to pay extra attention:
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧠 Learnings (4)📚 Learning: 2025-10-13T05:13:54.542ZApplied to files:
📚 Learning: 2025-09-08T13:20:47.308ZApplied to files:
📚 Learning: 2025-10-05T09:41:40.439ZApplied to files:
📚 Learning: 2025-10-21T06:07:53.455ZApplied to files:
⏰ 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). (2)
🔇 Additional comments (9)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.svelte (1)
19-43: Critical: onMount is async; teardown won’t registerMake onMount synchronous; chain the subscribe promise and guard close to avoid leaks. (svelte.dev)
-onMount(async () => { - const subscription: AppwriteRealtimeSubscription = await sdk - .forConsoleIn(page.params.region) - .realtime.subscribe('console', async (response) => { - if ( - response.events.includes( - `sites.${data.site.$id}.deployments.${data.deployment.$id}.update` - ) - ) { - deployment = response.payload; - if (response.payload.status === 'ready') { - const resolvedUrl = resolve( - '/(console)/project-[region]-[project]/sites/create-site/finish', - { - region: page.params.region, - project: page.params.project - } - ); - await goto(`${resolvedUrl}?site=${data.site.$id}`); - } - } - }); - - return subscription.close(); -}); +onMount(() => { + let subscription: AppwriteRealtimeSubscription | undefined; + let closed = false; + + sdk.forConsoleIn(page.params.region).realtime + .subscribe('console', async (response) => { + if ( + response.events.includes( + `sites.${data.site.$id}.deployments.${data.deployment.$id}.update` + ) + ) { + deployment = response.payload; + if (response.payload.status === 'ready') { + const resolvedUrl = resolve( + '/(console)/project-[region]-[project]/sites/create-site/finish', + { region: page.params.region, project: page.params.project } + ); + await goto(`${resolvedUrl}?site=${data.site.$id}`); + } + } + }) + .then((s) => (closed ? s.close() : (subscription = s))) + .catch((e) => console.error('Realtime subscribe failed', e)); + + return () => { + if (!closed) { + closed = true; + subscription?.close(); + } + }; +});
🧹 Nitpick comments (4)
src/lib/components/csvImportBox.svelte (1)
161-170: Consider using await for consistency.Since
onMountis now async, consider converting the promise chain to useawaitfor consistency with the subscription pattern below. This also enables better error handling if needed.Apply this diff:
- sdk.forProject(page.params.region, page.params.project) - .migrations.list({ - queries: [ - Query.equal('source', 'CSV'), - Query.equal('status', ['pending', 'processing']) - ] - }) - .then((migrations) => { - migrations.migrations.forEach(updateOrAddItem); - }); + const migrations = await sdk + .forProject(page.params.region, page.params.project) + .migrations.list({ + queries: [ + Query.equal('source', 'CSV'), + Query.equal('status', ['pending', 'processing']) + ] + }); + migrations.migrations.forEach(updateOrAddItem);src/routes/(console)/project-[region]-[project]/sites/site-[site]/deployments/deployment-[deployment]/+page.svelte (1)
33-44: Consider adding error handling for subscription setup.The async subscription setup has no error handling. If the subscription fails to establish, it could lead to unhandled promise rejections.
Consider wrapping the subscription in a try-catch:
onMount(async () => { + try { const subscription: AppwriteRealtimeSubscription = await sdk .forConsoleIn(page.params.region) .realtime.subscribe('console', async (response) => { if ( response.events.includes( `sites.${page.params.site}.deployments.${page.params.deployment}.update` ) ) { await invalidate(Dependencies.DEPLOYMENT); } }); - return subscription?.close(); + return () => subscription?.close(); + } catch (error) { + console.error('Failed to establish realtime subscription:', error); + } });src/routes/(console)/project-[region]-[project]/functions/function-[function]/executions/+page.svelte (1)
20-24: Optional: narrow the subscription channel to reduce noiseSubscribing to
'console'may deliver many unrelated events. If supported by your SDK wrapper, prefer a targeted channel like'functions.*.executions.*'to minimize churn before theincludesfilter.Example:
- subscription = await sdk.forConsole.realtime.subscribe('console', (response) => { + subscription = await sdk.forConsole.realtime.subscribe('functions.*.executions.*', (response) => {src/routes/(console)/project-[region]-[project]/overview/platforms/createFlutter.svelte (1)
174-175: Optional: remove unnecessary async on resetPlatformStore.
resetPlatformStoreis declared async but only callscreatePlatform.reset(). Consider droppingasyncto avoid implying awaited work.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
src/lib/components/csvImportBox.svelte(3 hunks)src/routes/(console)/project-[region]-[project]/functions/function-[function]/executions/+page.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/overview/platforms/createAndroid.svelte(2 hunks)src/routes/(console)/project-[region]-[project]/overview/platforms/createApple.svelte(2 hunks)src/routes/(console)/project-[region]-[project]/overview/platforms/createFlutter.svelte(2 hunks)src/routes/(console)/project-[region]-[project]/overview/platforms/createReactNative.svelte(2 hunks)src/routes/(console)/project-[region]-[project]/overview/platforms/createWeb.svelte(2 hunks)src/routes/(console)/project-[region]-[project]/sites/+page.svelte(2 hunks)src/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.svelte(2 hunks)src/routes/(console)/project-[region]-[project]/sites/site-[site]/+page.svelte(2 hunks)src/routes/(console)/project-[region]-[project]/sites/site-[site]/deployments/+layout.svelte(2 hunks)src/routes/(console)/project-[region]-[project]/sites/site-[site]/deployments/+page.svelte(2 hunks)src/routes/(console)/project-[region]-[project]/sites/site-[site]/deployments/deployment-[deployment]/+page.svelte(2 hunks)src/routes/(console)/project-[region]-[project]/sites/site-[site]/logs/+page.svelte(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-13T05:13:54.542Z
Learnt from: ItzNotABug
PR: appwrite/console#2413
File: src/routes/(console)/project-[region]-[project]/databases/table.svelte:33-39
Timestamp: 2025-10-13T05:13:54.542Z
Learning: In Svelte 5, `import { page } from '$app/state'` provides a reactive state proxy that can be accessed directly (e.g., `page.params`), unlike the older `import { page } from '$app/stores'` which returns a readable store requiring the `$page` syntax for auto-subscription in components.
Applied to files:
src/lib/components/csvImportBox.svelte
⏰ 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). (2)
- GitHub Check: e2e
- GitHub Check: build
🔇 Additional comments (14)
src/lib/components/csvImportBox.svelte (1)
5-5: LGTM! Type import added correctly.The type import for
AppwriteRealtimeSubscriptionfollows TypeScript best practices and aligns with the PR's refactoring pattern.src/routes/(console)/project-[region]-[project]/overview/platforms/createWeb.svelte (1)
220-220: LGTM: Store cleanup on component destroy.The
onDestroyhook properly resets the platform store when the component is destroyed, ensuring clean state management.src/routes/(console)/project-[region]-[project]/sites/site-[site]/deployments/deployment-[deployment]/+page.svelte (1)
3-3: LGTM!The type import is properly declared and used for type annotation.
src/routes/(console)/project-[region]-[project]/functions/function-[function]/executions/+page.svelte (2)
15-15: PageProps import is correct; codebase uses runesThe Svelte ^5.25.3 and SvelteKit ^2.42.1 stack supports runes. The file itself uses
PagePropswith$props()on line 17, confirming the import on line 15 is appropriate.
7-7: Type definition and import are correct and properly aligned.Verification confirms:
AppwriteRealtimeSubscriptionis correctly defined asAwaited<ReturnType<Realtime['subscribe']>>insrc/lib/stores/sdk.ts- The type is properly imported and used in the page file at line 20
- The
subscribe()call is correctly annotated with the type alias- The type derives from the external Appwrite SDK (
@appwrite.io/console), ensuring correct alignmentThe review comment's reference to
.close()usage cannot be verified as there are no.close()calls in the codebase. The type definition itself is sound.src/routes/(console)/project-[region]-[project]/overview/platforms/createAndroid.svelte (1)
19-19: LGTM: imports and onDestroy usageImporting onDestroy and resetting the platform store on teardown looks good.
Also applies to: 99-99
src/routes/(console)/project-[region]-[project]/overview/platforms/createApple.svelte (1)
20-20: LGTM: imports and teardown store resetAlso applies to: 108-108
src/routes/(console)/project-[region]-[project]/overview/platforms/createReactNative.svelte (1)
20-20: LGTM: lifecycle imports and onDestroy(resetPlatformStore)Also applies to: 135-135
src/routes/(console)/project-[region]-[project]/sites/+page.svelte (1)
28-28: LGTM: type import for AppwriteRealtimeSubscriptionsrc/routes/(console)/project-[region]-[project]/overview/platforms/createFlutter.svelte (1)
20-20: Imports look good.Adding onDestroy is appropriate given the separate store reset. No issues here.
src/routes/(console)/project-[region]-[project]/sites/site-[site]/logs/+page.svelte (1)
7-7: Type import addition is fine.Explicit typing improves clarity.
src/routes/(console)/project-[region]-[project]/sites/site-[site]/deployments/+page.svelte (1)
16-16: Type import is good.Keeps subscription handling explicit.
src/routes/(console)/project-[region]-[project]/sites/site-[site]/deployments/+layout.svelte (1)
4-4: Type import looks good.No concerns.
src/routes/(console)/project-[region]-[project]/sites/site-[site]/+page.svelte (1)
10-10: Type import addition LGTM.
...s/(console)/project-[region]-[project]/functions/function-[function]/executions/+page.svelte
Show resolved
Hide resolved
...s/(console)/project-[region]-[project]/functions/function-[function]/executions/+page.svelte
Outdated
Show resolved
Hide resolved
src/routes/(console)/project-[region]-[project]/overview/platforms/createAndroid.svelte
Outdated
Show resolved
Hide resolved
src/routes/(console)/project-[region]-[project]/overview/platforms/createApple.svelte
Outdated
Show resolved
Hide resolved
src/routes/(console)/project-[region]-[project]/sites/site-[site]/+page.svelte
Outdated
Show resolved
Hide resolved
src/routes/(console)/project-[region]-[project]/sites/site-[site]/deployments/+layout.svelte
Outdated
Show resolved
Hide resolved
src/routes/(console)/project-[region]-[project]/sites/site-[site]/deployments/+page.svelte
Outdated
Show resolved
Hide resolved
...roject-[region]-[project]/sites/site-[site]/deployments/deployment-[deployment]/+page.svelte
Outdated
Show resolved
Hide resolved
src/routes/(console)/project-[region]-[project]/sites/site-[site]/logs/+page.svelte
Outdated
Show resolved
Hide resolved
| onMount(() => { | ||
| // fast path: don't subscribe if org is on a free plan or is self-hosted. | ||
| if (isSelfHosted || (isCloud && $organization.billingPlan === BillingPlan.FREE)) return; | ||
| if (isSelfHosted || (isCloud && $organization?.billingPlan === BillingPlan.FREE)) return; |
There was a problem hiding this comment.
randomly gave an error during tests, figured I'd just add a ? for now.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/routes/(console)/project-[region]-[project]/sites/site-[site]/deployments/+layout.svelte (1)
14-40: Critical: Unsafe payload access before event-type check.Lines 17-22 cast and access
payload.statusbefore verifying the event is deployment-related (lines 24-38). Non-deployment console events can have incompatible payload structures, causing runtime errors.Apply this diff to check event type first:
onMount(() => { let previousStatus: string = null; return realtime.forConsole(page.params.region, 'console', (message) => { - const payload = message.payload as Models.Deployment; - if (payload.status !== 'ready' && previousStatus === payload.status) { - return; - } - - previousStatus = payload.status; - if (message.events.includes('sites.*.deployments.*.create')) { invalidate(Dependencies.DEPLOYMENTS); - return; } if (message.events.includes('sites.*.deployments.*.update')) { + const payload = message.payload as Models.Deployment; + if (payload.status !== 'ready' && previousStatus === payload.status) { + return; + } + previousStatus = payload.status; invalidate(Dependencies.DEPLOYMENTS); invalidate(Dependencies.SITE); return; } if (message.events.includes('sites.*.deployments.*.delete')) { invalidate(Dependencies.DEPLOYMENTS); - return; } }); });Note: Only
.updateevents need status checking;.createand.deletedon't require deduplication.
♻️ Duplicate comments (1)
src/routes/(console)/project-[region]-[project]/functions/function-[function]/executions/+page.svelte (1)
18-18: Destructuring $props() breaks reactivity—props won't update on invalidate().Destructuring from
$props()performs a one-time copy, sodatawon't react to prop changes wheninvalidate(Dependencies.EXECUTIONS)is called.Apply this diff to preserve reactivity:
-let { data }: PageProps = $props(); +const props: PageProps = $props();Then update all references from
data.*toprops.data.*throughout the template (lines 31, 35, 36, 47, 48, 52-55, 67, 72, 73).
🧹 Nitpick comments (1)
src/routes/(console)/project-[region]-[project]/sites/site-[site]/deployments/+page.svelte (1)
34-40: Consider adding error handling for subscription setup.While the current implementation may be acceptable depending on your app's error handling strategy, wrapping the subscription in a try-catch could prevent component crashes if the realtime connection fails.
Example with error handling:
onMount(() => { - return realtime.forConsole(page.params.region, 'console', (response) => { - if (response.events.includes('sites.*.deployments.*')) { - invalidate(Dependencies.DEPLOYMENTS); - } - }); + try { + return realtime.forConsole(page.params.region, 'console', (response) => { + if (response.events.includes('sites.*.deployments.*')) { + invalidate(Dependencies.DEPLOYMENTS); + } + }); + } catch (error) { + console.error('Failed to setup realtime subscription:', error); + return () => {}; // Return no-op cleanup function + } });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
src/lib/components/backupRestoreBox.svelte(1 hunks)src/lib/components/csvImportBox.svelte(2 hunks)src/lib/stores/sdk.ts(2 hunks)src/routes/(console)/project-[region]-[project]/functions/function-[function]/executions/+page.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/overview/platforms/createAndroid.svelte(2 hunks)src/routes/(console)/project-[region]-[project]/overview/platforms/createApple.svelte(2 hunks)src/routes/(console)/project-[region]-[project]/overview/platforms/createFlutter.svelte(2 hunks)src/routes/(console)/project-[region]-[project]/overview/platforms/createReactNative.svelte(2 hunks)src/routes/(console)/project-[region]-[project]/overview/platforms/createWeb.svelte(2 hunks)src/routes/(console)/project-[region]-[project]/sites/+page.svelte(2 hunks)src/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/sites/site-[site]/+page.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/sites/site-[site]/deployments/+layout.svelte(2 hunks)src/routes/(console)/project-[region]-[project]/sites/site-[site]/deployments/+page.svelte(2 hunks)src/routes/(console)/project-[region]-[project]/sites/site-[site]/deployments/deployment-[deployment]/+page.svelte(2 hunks)src/routes/(console)/project-[region]-[project]/sites/site-[site]/logs/+page.svelte(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/routes/(console)/project-[region]-[project]/sites/site-[site]/+page.svelte
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-10-13T05:13:54.542Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2413
File: src/routes/(console)/project-[region]-[project]/databases/table.svelte:33-39
Timestamp: 2025-10-13T05:13:54.542Z
Learning: In Svelte 5, `import { page } from '$app/state'` provides a reactive state proxy that can be accessed directly (e.g., `page.params`), unlike the older `import { page } from '$app/stores'` which returns a readable store requiring the `$page` syntax for auto-subscription in components.
Applied to files:
src/routes/(console)/project-[region]-[project]/overview/platforms/createFlutter.sveltesrc/routes/(console)/project-[region]-[project]/overview/platforms/createAndroid.sveltesrc/routes/(console)/project-[region]-[project]/overview/platforms/createReactNative.sveltesrc/routes/(console)/project-[region]-[project]/overview/platforms/createWeb.sveltesrc/lib/components/csvImportBox.sveltesrc/routes/(console)/project-[region]-[project]/overview/platforms/createApple.sveltesrc/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/logs/+page.sveltesrc/routes/(console)/project-[region]-[project]/functions/function-[function]/executions/+page.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/deployments/deployment-[deployment]/+page.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/deployments/+page.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/deployments/+layout.sveltesrc/routes/(console)/project-[region]-[project]/sites/+page.svelte
📚 Learning: 2025-10-05T09:41:40.439Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2398
File: src/routes/(console)/verify-email/+page.svelte:48-51
Timestamp: 2025-10-05T09:41:40.439Z
Learning: In SvelteKit 5, `page` imported from `$app/state` is a reactive state object (using runes), not a store. It should be accessed as `page.data` without the `$` prefix, unlike the store-based `$page` from `$app/stores` in earlier versions.
Applied to files:
src/routes/(console)/project-[region]-[project]/overview/platforms/createFlutter.sveltesrc/routes/(console)/project-[region]-[project]/overview/platforms/createAndroid.sveltesrc/routes/(console)/project-[region]-[project]/overview/platforms/createReactNative.sveltesrc/routes/(console)/project-[region]-[project]/overview/platforms/createWeb.sveltesrc/lib/components/csvImportBox.sveltesrc/routes/(console)/project-[region]-[project]/overview/platforms/createApple.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/logs/+page.sveltesrc/routes/(console)/project-[region]-[project]/functions/function-[function]/executions/+page.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/deployments/+layout.sveltesrc/routes/(console)/project-[region]-[project]/sites/+page.svelte
📚 Learning: 2025-09-08T13:20:47.308Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2316
File: src/routes/(console)/project-[region]-[project]/functions/create-function/deploy/+page.svelte:29-29
Timestamp: 2025-09-08T13:20:47.308Z
Learning: The Form.svelte component in the Appwrite console creates a FormContext with isSubmitting as writable(false) and expects consumers to work with Svelte writable stores, not plain booleans.
Applied to files:
src/routes/(console)/project-[region]-[project]/overview/platforms/createFlutter.sveltesrc/routes/(console)/project-[region]-[project]/overview/platforms/createAndroid.sveltesrc/routes/(console)/project-[region]-[project]/overview/platforms/createReactNative.sveltesrc/routes/(console)/project-[region]-[project]/overview/platforms/createWeb.sveltesrc/routes/(console)/project-[region]-[project]/overview/platforms/createApple.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/logs/+page.svelte
📚 Learning: 2025-09-25T04:21:57.071Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2373
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte:536-552
Timestamp: 2025-09-25T04:21:57.071Z
Learning: In the Appwrite console database suggestions flow, after successfully creating columns via `createColumns()`, the `await invalidate(Dependencies.TABLE)` call causes the view to be destroyed and another view (populated table view) to be rendered, automatically cleaning up component state without needing manual reset.
Applied to files:
src/routes/(console)/project-[region]-[project]/sites/site-[site]/logs/+page.svelte
📚 Learning: 2025-10-13T05:16:07.656Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2413
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/header.svelte:54-58
Timestamp: 2025-10-13T05:16:07.656Z
Learning: In SvelteKit apps, shared layout components (like headers) that use `$derived(page.data.*)` should use optional chaining when accessing properties that may not be present on all routes. During page transitions, reactive statements can briefly evaluate with different page.data structures, so optional chaining prevents runtime errors when navigating between routes with different data shapes (e.g., between `/databases` and `/databases/database-[database]`).
Applied to files:
src/routes/(console)/project-[region]-[project]/sites/site-[site]/logs/+page.sveltesrc/routes/(console)/project-[region]-[project]/functions/function-[function]/executions/+page.sveltesrc/routes/(console)/project-[region]-[project]/sites/+page.svelte
🧬 Code graph analysis (1)
src/lib/stores/sdk.ts (1)
src/lib/system.ts (1)
isDev(28-28)
⏰ 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). (2)
- GitHub Check: build
- GitHub Check: e2e
🔇 Additional comments (15)
src/lib/components/backupRestoreBox.svelte (1)
128-128: Verify the logic when organization data is undefined.The optional chaining prevents the crash, but it changes the behavior when
$organizationis undefined. Currently:
- If
$organizationisundefined, then$organization?.billingPlanevaluates toundefinedundefined === BillingPlan.FREEevaluates tofalse- The condition becomes
isSelfHosted || false, so the subscription will be set upIs this intended? If the organization data hasn't loaded yet, should the subscription be established optimistically, or should we wait until we can confirm the billing plan?
Consider one of these alternatives if the current behavior is problematic:
Option 1: Skip subscription when organization is undefined
-if (isSelfHosted || (isCloud && $organization?.billingPlan === BillingPlan.FREE)) return; +if (isSelfHosted || !$organization || (isCloud && $organization.billingPlan === BillingPlan.FREE)) return;Option 2: Only set up subscription when explicitly allowed
-if (isSelfHosted || (isCloud && $organization?.billingPlan === BillingPlan.FREE)) return; +if (isSelfHosted || (isCloud && (!$organization || $organization.billingPlan === BillingPlan.FREE))) return;Otherwise, if organization data is guaranteed to be available at mount time or optimistic subscription is acceptable, the current implementation is fine.
src/routes/(console)/project-[region]-[project]/overview/platforms/createAndroid.svelte (1)
83-97: LGTM! Realtime subscription correctly refactored.The synchronous onMount pattern now properly registers the cleanup function. The new
realtime.forConsoleAPI elegantly handles the subscription lifecycle, and callingsubscription()on both successful ping and unmount ensures proper cleanup.src/lib/stores/sdk.ts (1)
152-177: LGTM! Clean subscription lifecycle management.The
realtime.forConsoleimplementation properly handles the async subscription lifecycle by returning a synchronous cleanup closure that chains the promise. Theclosedflag prevents double-closing, and error handling in dev mode aids debugging.src/lib/components/csvImportBox.svelte (1)
190-195: LGTM! Clean direct return of cleanup function.The refactored subscription correctly returns the cleanup function from
realtime.forConsoledirectly. This is the cleanest pattern for this use case.src/routes/(console)/project-[region]-[project]/sites/+page.svelte (1)
52-58: LGTM! Proper subscription cleanup.The refactored realtime subscription correctly uses the synchronous onMount pattern and directly returns the cleanup function from
realtime.forConsole.src/routes/(console)/project-[region]-[project]/sites/site-[site]/deployments/deployment-[deployment]/+page.svelte (1)
33-43: LGTM! Correct subscription lifecycle.The refactored subscription properly uses the synchronous onMount pattern and returns the cleanup function from
realtime.forConsole. The async callback with awaited invalidate is fine—realtime handlers support async callbacks.src/routes/(console)/project-[region]-[project]/overview/platforms/createReactNative.svelte (1)
119-133: LGTM: Subscription lifecycle correctly implemented.The new
realtime.forConsoleAPI is used correctly. The subscription function is invoked to close the stream both when the ping event is received and during component cleanup.src/routes/(console)/project-[region]-[project]/functions/function-[function]/executions/+page.svelte (1)
20-26: LGTM: Subscription correctly implemented.The
realtime.forConsolepattern is used correctly—returning the subscription function directly fromonMountensures proper cleanup.src/routes/(console)/project-[region]-[project]/overview/platforms/createApple.svelte (1)
92-106: LGTM: Subscription lifecycle correctly implemented.The new
realtime.forConsoleAPI is used correctly, with proper cleanup.src/routes/(console)/project-[region]-[project]/overview/platforms/createFlutter.svelte (1)
158-172: LGTM: Subscription lifecycle correctly implemented.The new
realtime.forConsoleAPI is used correctly, with proper cleanup.src/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.svelte (1)
20-40: LGTM: Async callback and subscription lifecycle correctly implemented.The async callback is appropriate here since it needs to
await goto(). The subscription function is returned directly fromonMountfor proper cleanup.src/routes/(console)/project-[region]-[project]/overview/platforms/createWeb.svelte (1)
204-218: LGTM: Subscription lifecycle correctly implemented.The new
realtime.forConsoleAPI is used correctly, with proper cleanup.src/routes/(console)/project-[region]-[project]/sites/site-[site]/logs/+page.svelte (1)
16-22: LGTM: Subscription lifecycle correctly implemented.The new
realtime.forConsoleAPI is used correctly—returning the subscription function directly fromonMountensures proper cleanup.src/routes/(console)/project-[region]-[project]/sites/site-[site]/deployments/+page.svelte (2)
16-16: LGTM: Import updated for new realtime API.The addition of the
realtimeimport is necessary for the updated subscription pattern used in theonMountfunction.
34-40: Cleanup pattern verified and correct.The
realtime.forConsolemethod atsrc/lib/stores/sdk.ts:152-157has the return type: () => void, confirming it returns a cleanup function synchronously. TheonMountimplementation correctly returns this cleanup function for proper teardown when the component unmounts. The pattern is sound.
…ne realtime service as well.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/lib/stores/sdk.ts (1)
143-157: Consider caching the Realtime instance.Creating a new
Realtimeinstance on every call toforProjectcould impact performance if this method is invoked frequently. Consider caching the instance and reusing it when the endpoint hasn't changed.Example approach:
+let cachedRealtime: Realtime | null = null; +let cachedEndpoint: string | null = null; + forProject( region: string, channels: string | string[], callback: AppwriteRealtimeResponseEvent ) { const endpoint = getApiEndpoint(region); if (endpoint !== clientRealtime.config.endpoint) { clientRealtime.setEndpoint(endpoint); + cachedRealtime = null; } - // because uses a different client! - const realtime = new Realtime(clientRealtime); + if (!cachedRealtime || cachedEndpoint !== endpoint) { + cachedRealtime = new Realtime(clientRealtime); + cachedEndpoint = endpoint; + } - return createRealtimeSubscription(realtime, channels, callback); + return createRealtimeSubscription(cachedRealtime, channels, callback); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/lib/components/backupRestoreBox.svelte(1 hunks)src/lib/components/migrationBox.svelte(1 hunks)src/lib/stores/sdk.ts(4 hunks)src/routes/(console)/project-[region]-[project]/+layout.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/backups/+page.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.svelte(3 hunks)src/routes/(console)/project-[region]-[project]/functions/function-[function]/+layout.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/functions/function-[function]/executions/+layout.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/settings/migrations/+page.svelte(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lib/components/backupRestoreBox.svelte
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-10-13T05:13:54.542Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2413
File: src/routes/(console)/project-[region]-[project]/databases/table.svelte:33-39
Timestamp: 2025-10-13T05:13:54.542Z
Learning: In Svelte 5, `import { page } from '$app/state'` provides a reactive state proxy that can be accessed directly (e.g., `page.params`), unlike the older `import { page } from '$app/stores'` which returns a readable store requiring the `$page` syntax for auto-subscription in components.
Applied to files:
src/routes/(console)/project-[region]-[project]/functions/function-[function]/executions/+layout.sveltesrc/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.svelte
📚 Learning: 2025-09-25T04:21:57.071Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2373
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte:536-552
Timestamp: 2025-09-25T04:21:57.071Z
Learning: In the Appwrite console database suggestions flow, after successfully creating columns via `createColumns()`, the `await invalidate(Dependencies.TABLE)` call causes the view to be destroyed and another view (populated table view) to be rendered, automatically cleaning up component state without needing manual reset.
Applied to files:
src/routes/(console)/project-[region]-[project]/functions/function-[function]/executions/+layout.sveltesrc/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.svelte
📚 Learning: 2025-09-30T07:41:06.679Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2425
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte:454-468
Timestamp: 2025-09-30T07:41:06.679Z
Learning: In `src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte`, the column suggestions API (console.suggestColumns) has a maximum limit of 7 columns returned, which aligns with the initial placeholder count of 7 in customColumns.
Applied to files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.svelte
🧬 Code graph analysis (1)
src/lib/stores/sdk.ts (1)
src/lib/system.ts (1)
isDev(28-28)
⏰ 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). (2)
- GitHub Check: build
- GitHub Check: e2e
🔇 Additional comments (9)
src/routes/(console)/project-[region]-[project]/settings/migrations/+page.svelte (1)
41-45: Returning the unsubscribe fixes onMount cleanupGood call letting
onMounthand back the newrealtime.forProjectunsubscribe closure—this keeps the subscription scoped to the component lifecycle with the updated API.src/routes/(console)/project-[region]-[project]/functions/function-[function]/executions/+layout.svelte (1)
9-13: Realtime executions subscription now matches the new APIThe streamlined
forProjectcall cleanly ties the executions invalidation to the returned unsubscribe function—looks solid.src/routes/(console)/project-[region]-[project]/databases/database-[database]/backups/+page.svelte (1)
164-171: Scoped invalidation keeps backups refresh preciseThanks for deferring to the new
forProjectsignature and retaining the channel guard—this keeps the BACKUPS cache invalidation tight to the active project.src/routes/(console)/project-[region]-[project]/+layout.svelte (1)
30-36: Stats stream now cleans up via returned unsubscribeThe refactor to return the closure from
realtime.forProjectensures the stats listener is registered and torn down correctly with the layout.src/routes/(console)/project-[region]-[project]/functions/function-[function]/+layout.svelte (1)
24-45: Deployment watcher aligns with the closure-based realtime APIHappy to see the deployment listener now rely on the callback-returned unsubscribe—keeps lifecycle management tidy while preserving the existing status guard logic.
src/lib/components/migrationBox.svelte (1)
53-60: Console channel subscription now lifecycles correctlyReturning the
forProjectcleanup here dovetails with the channel filter to keep migration updates scoped to the active project—looks good.src/lib/stores/sdk.ts (3)
1-1: LGTM!The
isDevimport is appropriately used for conditional error logging in the new subscription handler.
159-169: LGTM!The region-aware logic correctly selects the appropriate realtime instance, and the subscription setup is properly delegated.
213-221: LGTM!The new types provide a clear contract for realtime responses and callbacks. Using
unknownfor payload is appropriate given the varied nature of realtime events.
...nsole)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.svelte
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/routes/(console)/project-[region]-[project]/functions/function-[function]/deployment-[deployment]/+page.svelte (2)
46-59: Good refactor to closure-based subscription; verify type safety of payload.The new realtime.forProject API simplifies subscription management and cleanup. However, the type assertion at line 53 assumes the payload always matches Models.Deployment without runtime validation.
Consider verifying that the realtime response payload typing is enforced server-side, or add a runtime check to ensure the payload shape matches expectations before casting:
const payload = response.payload as Models.Deployment; if (payload && typeof payload.status === 'string' && payload.status === 'ready') { invalidate(Dependencies.DEPLOYMENT); }Alternatively, if the realtime API guarantees type safety, this is acceptable as-is. You may also want to add error handling for subscription failures:
onMount(() => { try { return realtime.forProject(page.params.region, 'console', (response) => { // ... existing logic }); } catch (error) { console.error('Failed to subscribe to deployment updates:', error); } });
39-39: Consider migrating to$props()for consistency with broader refactor.While
export let datais still valid, the AI summary indicates that several pages in this PR are switching to typedPagePropsvia$props(). Consider whether this file should also adopt that pattern for consistency:const { data }: PageProps = $props();If the phased migration is intentional, feel free to ignore this suggestion.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/routes/(console)/project-[region]-[project]/functions/function-[function]/deployment-[deployment]/+page.svelte(2 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-09-08T13:20:47.308Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2316
File: src/routes/(console)/project-[region]-[project]/functions/create-function/deploy/+page.svelte:29-29
Timestamp: 2025-09-08T13:20:47.308Z
Learning: The Form.svelte component in the Appwrite console creates a FormContext with isSubmitting as writable(false) and expects consumers to work with Svelte writable stores, not plain booleans.
Applied to files:
src/routes/(console)/project-[region]-[project]/functions/function-[function]/deployment-[deployment]/+page.svelte
📚 Learning: 2025-10-13T05:13:54.542Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2413
File: src/routes/(console)/project-[region]-[project]/databases/table.svelte:33-39
Timestamp: 2025-10-13T05:13:54.542Z
Learning: In Svelte 5, `import { page } from '$app/state'` provides a reactive state proxy that can be accessed directly (e.g., `page.params`), unlike the older `import { page } from '$app/stores'` which returns a readable store requiring the `$page` syntax for auto-subscription in components.
Applied to files:
src/routes/(console)/project-[region]-[project]/functions/function-[function]/deployment-[deployment]/+page.svelte
📚 Learning: 2025-10-05T09:41:40.439Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2398
File: src/routes/(console)/verify-email/+page.svelte:48-51
Timestamp: 2025-10-05T09:41:40.439Z
Learning: In SvelteKit 5, `page` imported from `$app/state` is a reactive state object (using runes), not a store. It should be accessed as `page.data` without the `$` prefix, unlike the store-based `$page` from `$app/stores` in earlier versions.
Applied to files:
src/routes/(console)/project-[region]-[project]/functions/function-[function]/deployment-[deployment]/+page.svelte
🔇 Additional comments (1)
src/routes/(console)/project-[region]-[project]/functions/function-[function]/deployment-[deployment]/+page.svelte (1)
4-4: LGTM: Clean migration to the new realtime API.The import change correctly reflects the new realtime subscription approach.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.svelte (1)
85-85: Type fix applied correctly.The parentheses correctly wrap the function signature before the null union, as addressed in the previous review.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.svelte(3 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-09-30T07:41:06.679Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2425
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte:454-468
Timestamp: 2025-09-30T07:41:06.679Z
Learning: In `src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte`, the column suggestions API (console.suggestColumns) has a maximum limit of 7 columns returned, which aligns with the initial placeholder count of 7 in customColumns.
Applied to files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.svelte
📚 Learning: 2025-10-07T14:17:11.597Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2413
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts:28-28
Timestamp: 2025-10-07T14:17:11.597Z
Learning: In src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts, the empty `vectordb: {}` entry in baseTerminology is intentional. The vectordb database type is not currently used because the backend API is not finalized, and no database type returns 'vectordb' at the moment.
Applied to files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.svelte
📚 Learning: 2025-10-13T05:13:54.542Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2413
File: src/routes/(console)/project-[region]-[project]/databases/table.svelte:33-39
Timestamp: 2025-10-13T05:13:54.542Z
Learning: In Svelte 5, `import { page } from '$app/state'` provides a reactive state proxy that can be accessed directly (e.g., `page.params`), unlike the older `import { page } from '$app/stores'` which returns a readable store requiring the `$page` syntax for auto-subscription in components.
Applied to files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.svelte
📚 Learning: 2025-09-25T04:21:57.071Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2373
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte:536-552
Timestamp: 2025-09-25T04:21:57.071Z
Learning: In the Appwrite console database suggestions flow, after successfully creating columns via `createColumns()`, the `await invalidate(Dependencies.TABLE)` call causes the view to be destroyed and another view (populated table view) to be rendered, automatically cleaning up component state without needing manual reset.
Applied to files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.svelte
⏰ 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). (2)
- GitHub Check: build
- GitHub Check: e2e
🔇 Additional comments (2)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.svelte (2)
23-23: LGTM!The type import improves type safety for realtime event handlers.
268-268: LGTM!The parameter type annotation is consistent with the handler type definition and improves type safety.
What does this PR do?
(Provide a description of what this PR does.)
Test Plan
(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work.)
Related PRs and Issues
(If this PR is related to any other PR or resolves any issue or related to any issue link all related PR and issues here.)
Have you read the Contributing Guidelines on issues?
(Write your answer here.)
Summary by CodeRabbit