Update project to async Svelte and remote functions

[nevthereal]

Summary:

• migrate the project setup to follow the new async Svelte patterns and remote function APIs described in the MCP docs
• adjust any related configuration so the new async runtime and remote calls are wired up consistently

Testing:

• Not run (not requested)

[nevthereal]

@coderabbitai review

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

@nevthereal: Hello! I'm here to help. How can I assist you with this PR? I can:

• Review the changes in detail
• Answer questions about the migration to async Svelte patterns
• Help with code changes or refactoring
• Provide guidance on the remote function APIs
• Generate documentation or tests
• Or anything else you need!

Just let me know what you'd like me to do. 🐰

[nevthereal]

@coderabbitai do everything

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

@nevthereal: I'll conduct a comprehensive review of your PR! Let me gather information about all the changes and analyze the migration to async Svelte patterns and remote function APIs.

---

🏁 Script executed:

#!/bin/bash

# Get PR details and list of changed files
echo "=== PR Details ==="
gh pr view 34 --json title,body,files --jq '.title, .body, .files[].path'

echo -e "\n=== Changed Files with Stats ==="
gh pr diff 34 --name-only

echo -e "\n=== Full diff of the PR ==="
gh pr diff 34

Length of output: 142115

---

@nevthereal: I'll conduct a comprehensive review of your migration to async Svelte and remote functions! This is a significant architectural change. Let me analyze the implementation:

🎯 Architecture & Patterns

✅ Strengths

1. Clean separation of concerns - Remote functions are well-organized by domain (auth, challenge, club, profile)
2. Consistent API patterns - Using query, form, and command appropriately
3. Removed unnecessary dependencies - formsnap and sveltekit-superforms are no longer needed

⚠️ Issues & Recommendations

1. Missing Error Boundaries

Your components now use top-level await, but there's no error handling for failed data fetches:

// src/routes/+page.svelte
const data = await getHomePageData(); // What if this fails?

Recommendation: Wrap data fetches in try-catch or use {#await} blocks:

{`#await` getHomePageData()}
  <p>Loading...</p>
{:then data}
  <!-- Your content -->
{:catch error}
  <p>Error: {error.message}</p>
{/await}

2. Inconsistent location.reload() Usage

Multiple places use location.reload() after mutations:

// src/routes/clubs/[clubId]/challenge/[challengeId]/+layout.svelte (lines 113, 165, 184)
await joinChallenge({ clubId, challengeId });
location.reload(); // Hard reload loses app state

Recommendation: Use SvelteKit's invalidate() or invalidateAll() for reactive updates without full page reload, or consider using stores to trigger reactive updates.

3. Race Conditions in $effect

Several components have $effect blocks that set form values, which could cause race conditions:

// src/lib/components/DisciplineForm.svelte
$effect(() => {
  disciplineForm.fields.challengeId.set(challengeId); // Runs on every reactive change
});

Recommendation: Use $effect.pre() or add dependency tracking to prevent unnecessary updates.

4. Validation Schema Inconsistency

Your validation schemas are split between src/lib/zod.ts and inline in remote functions:

// Inline in challenge.remote.ts
const addEntrySchema = z.object({
  challengeId: z.string(),
  disciplineId: z.string(),
  amount: z.number().min(0.01),
  date: z.string()
});

// In zod.ts
export const newEntry = z.object({
  disciplineId: z.string(),
  amount: z.coerce.number().multipleOf(0.01).min(0.01),
  date: z.iso.date()
});

Recommendation: Consolidate all validation schemas in one place for consistency and reusability.

5. Missing Authentication Checks

Some remote functions don't verify user permissions early:

// src/lib/remote/challenge.remote.ts - addEntry function
export const addEntry = form(addEntrySchema, async ({ challengeId, disciplineId, amount, date }) => {
  const user = requireUser(); // Good
  
  const qDiscipline = await db.query.discipline.findFirst({ where: { id: disciplineId } });
  if (!qDiscipline) error(404, 'Disziplin nicht gefunden');
  
  // Should check if user is a challenge member BEFORE querying discipline
});

Recommendation: Check user permissions as early as possible to avoid unnecessary database queries.

🔒 Security Concerns

1. No CSRF Protection on Commands

Your command functions (like deleteEntry, leaveChallenge) don't have CSRF protection. The old form actions had built-in CSRF tokens.

Recommendation: Verify that remote functions have CSRF protection or implement your own token verification.

2. Client-Side Authorization Logic

Authorization checks happen server-side (good), but UI elements are conditionally rendered client-side:

<ClubAdmin {isAdmin}>
  <!-- Admin-only content -->
</ClubAdmin>

While server-side validation exists, ensure all admin actions re-verify permissions server-side.

🐛 Potential Bugs

1. Date Validation Issues

The date handling in addEntry could be problematic:

// src/lib/remote/challenge.remote.ts line 406
const entryDate = new Date(date); // 'date' is a string
const challengeStart = new Date(qChallenge.startsAt);
challengeStart.setUTCHours(0, 0, 0, 0);

If date is in ISO format without timezone, this could lead to timezone-related bugs.

Recommendation: Use a consistent timezone library or ensure all dates are in UTC.

2. Memory Leaks in Derived State

Multiple files derive state in loops without proper cleanup:

// src/routes/clubs/[clubId]/+page.svelte
const activeChallenges = $derived(club.challenges.filter(...));
const pastChallenges = $derived(club.challenges.filter(...));

This is fine, but if club changes frequently, ensure there are no memory leaks.

3. Empty String vs Undefined

Field components might send empty strings instead of undefined/null:

<Input id="club-name" {...editClubDetails.fields.name.as('text')} />

Ensure your validation schemas handle empty strings appropriately (e.g., .min(1) will catch them).

🚀 Performance Considerations

1. Sequential Queries

Some functions perform sequential database queries that could be parallelized:

// src/lib/remote/challenge.remote.ts - getChallengeContext
const qChallenge = await db.query.challenge.findFirst(...);
const currentUserChallenge = await db.query.challengeMember.findFirst(...);
const currentUserClub = await db.query.clubMember.findFirst(...);

Recommendation:

const [qChallenge, currentUserChallenge, currentUserClub] = await Promise.all([
  db.query.challenge.findFirst(...),
  db.query.challengeMember.findFirst(...),
  db.query.clubMember.findFirst(...)
]);

2. Unnecessary Data Fetching

Components fetch data on every navigation:

const data = await getChallengeLayoutData({ clubId, challengeId });

Consider implementing a caching strategy for frequently accessed data.

📝 Code Quality

1. Magic Numbers

Hard-coded values like grace periods:

// src/lib/remote/challenge.remote.ts line 157
gte(challenge.endsAt, new Date(now.getTime() - 2 * 24 * 60 * 60 * 1000))

Recommendation: Extract to named constants:

const GRACE_PERIOD_DAYS = 2;
const GRACE_PERIOD_MS = GRACE_PERIOD_DAYS * 24 * 60 * 60 * 1000;

2. Inconsistent Null Handling

Some functions use optional chaining, others use explicit null checks:

const winner = items[0]?.name ?? 'Offen'; // Good
if (!qChallenge) error(404); // Also good, but inconsistent pattern

Recommendation: Choose one pattern and stick with it.

✅ Testing Recommendations

Since testing wasn't run, I recommend testing:

1. Form submissions with invalid data
2. Concurrent form submissions (race conditions)
3. Navigation during data loading
4. Error states when API calls fail
5. Permission boundaries (non-admin accessing admin features)
6. Date boundary cases (challenge start/end dates)

📦 Summary

This is a well-executed migration! The code is clean and follows good patterns. My main concerns are:

1. Add error boundaries for failed data fetches
2. Replace location.reload() with reactive patterns
3. Consolidate validation schemas
4. Parallelize database queries where possible
5. Add CSRF protection verification
6. Test thoroughly, especially error cases

Would you like me to help implement any of these improvements? I can generate code changes for specific issues.