Skip to content

perf: eliminate wasted API call, afterUpdate overhead, and sequential billing checks#3026

Merged
HarshMN2345 merged 3 commits into
mainfrom
perf-console-optimizations
May 11, 2026
Merged

perf: eliminate wasted API call, afterUpdate overhead, and sequential billing checks#3026
HarshMN2345 merged 3 commits into
mainfrom
perf-console-optimizations

Conversation

@HarshMN2345
Copy link
Copy Markdown
Member

… billing checks

  • Remove unused projects.list() call fired on every console load (result was never consumed)
  • Replace afterUpdate with reactive headerAlert subscription to avoid per-render overhead
  • Parallelize billing checks with Promise.all instead of sequential awaits
  • Fix database.subscribe() memory leak by wrapping in onMount for proper cleanup

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.)

… billing checks

- Remove unused projects.list() call fired on every console load (result was never consumed)
- Replace afterUpdate with reactive headerAlert subscription to avoid per-render overhead
- Parallelize billing checks with Promise.all instead of sequential awaits
- Fix database.subscribe() memory leak by wrapping in onMount for proper cleanup
@HarshMN2345 HarshMN2345 changed the title perf: eliminate wasted API call, afterUpdate overhead, and sequential… perf: eliminate wasted API call, afterUpdate overhead, and sequential billing checks May 5, 2026
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 5, 2026

Greptile Summary

This PR removes a never-consumed projects.list() API call from the layout load, wraps database.subscribe in onMount for proper cleanup, and parallelises billing checks with Promise.all. The reactive $: statement replaces the afterUpdate hook for updating the active header alert.

  • The unused allProjectsCount field and its backing API call are cleanly removed from +layout.ts; no consumers remain in the codebase.
  • The database.subscribe is now correctly wrapped in onMount, returning the unsubscribe function to prevent memory leaks on component destroy.
  • The billing-check parallelisation in +layout.svelte accidentally calls checkForNewDevUpgradePro twice — once via an explicit await before the array is built and again inside billingChecks — issuing duplicate backend requests on every organization load.

Confidence Score: 4/5

Safe to merge once the duplicate checkForNewDevUpgradePro call in +layout.svelte is removed.

The +layout.ts change is straightforward and correct. In +layout.svelte, the database.subscribe fix and reactive-statement replacement look good, but the billing-check refactor left checkForNewDevUpgradePro executing twice per organization load — once explicitly awaited and once inside Promise.all — causing two redundant backend round-trips every time the org changes.

src/routes/(console)/+layout.svelte — the billing check section around the billingChecks array

Important Files Changed

Filename Overview
src/routes/(console)/+layout.svelte Parallelizes billing checks and fixes database.subscribe memory leak, but introduces a duplicate call to checkForNewDevUpgradePro (called once directly and once inside billingChecks), causing redundant API requests on every organization load.
src/routes/(console)/+layout.ts Removes unused projects.list() call and the allProjectsCount return value; grep confirms no other file consumes allProjectsCount, so the removal is safe.

Reviews (2): Last reviewed commit: "Merge branch 'main' into perf-console-op..." | Re-trigger Greptile

afterUpdate(() => {
$activeHeaderAlert = headerAlert.getExcluding('impersonation');
});
$: (void $headerAlert, ($activeHeaderAlert = headerAlert.getExcluding('impersonation')));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Unusual comma-expression reactive pattern

The comma expression (void $headerAlert, ($activeHeaderAlert = ...)) is a non-obvious Svelte pattern. Reading $headerAlert via void purely to register it as a reactive dependency, while performing the actual side-effect in the right-hand operand, is hard to parse at a glance and not idiomatic Svelte. A named reactive block or a dedicated $: if ($headerAlert) form would communicate intent more clearly. There is also a subtle semantic shift from afterUpdate: this reactive declaration fires on component initialization (before any update), whereas afterUpdate only fired after re-renders. On first mount $headerAlert is empty, so getExcluding returns a no-op result — that is harmless today, but the distinction is worth noting for future maintainers.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Comment on lines +283 to 289
onMount(() => {
return database.subscribe(async (database) => {
if (!database || !page.params.region || !page.params.project) return;
// the component checks `isCloud` internally.
await checkForDatabaseBackupPolicies(page.params.region, page.params.project, database);
});
});
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Two separate onMount blocks in the same component

There are now two onMount calls in this component (lines 253 and 283). While Svelte allows multiple onMount registrations, having two separate blocks for the same component lifecycle event makes the initialization flow harder to follow. The database.subscribe setup could be placed inside the existing onMount block at line 253 to keep all mounting logic in one place.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Comment on lines 301 to +306
await checkForNewDevUpgradePro(org);

if (org?.billingPlanDetails.requiresPaymentMethod) {
await paymentExpired(org);
await checkPaymentAuthorizationRequired(org);
const billingChecks: Promise<unknown>[] = [
checkForUsageLimit(org),
checkForNewDevUpgradePro(org)
];
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 checkForNewDevUpgradePro is called twice: once on line 301 with an explicit await, and again inside billingChecks on line 305. The first call is awaited before the array is even constructed, so both invocations will hit the backend (two organizations.list() calls and two getCoupon() calls per load). The localStorage.getItem('newDevUpgradePro') guard is only set externally (on user dismissal), so it won't short-circuit the second run. The function should be in billingChecks only — remove the standalone await call.

Suggested change
await checkForNewDevUpgradePro(org);
if (org?.billingPlanDetails.requiresPaymentMethod) {
await paymentExpired(org);
await checkPaymentAuthorizationRequired(org);
const billingChecks: Promise<unknown>[] = [
checkForUsageLimit(org),
checkForNewDevUpgradePro(org)
];
const billingChecks: Promise<unknown>[] = [
checkForUsageLimit(org),
checkForNewDevUpgradePro(org)
];

@HarshMN2345 HarshMN2345 merged commit 736972c into main May 11, 2026
4 of 5 checks passed
@HarshMN2345 HarshMN2345 deleted the perf-console-optimizations branch May 11, 2026 10:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants