Feature/migrate quizzes to Svelte 5 #888
Conversation
- Replaces run() with effect() for reactivity - Uses $state and $derived for all dynamic values - Handles audio playback, quiz progression, and result saving - Ensures full client-side functionality without server code
WalkthroughThe Svelte quiz page component was refactored to use the new Svelte 5 reactive state management APIs, replacing traditional reactive declarations and variable exports with Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SvelteComponent
participant State
participant Derived
participant Effect
User->>SvelteComponent: Loads quiz page
SvelteComponent->>State: Initializes state variables via $state
SvelteComponent->>Derived: Computes derived values via $derived
SvelteComponent->>Effect: Sets up effect to monitor data changes
Effect->>State: Updates quiz state, resets as needed
User->>SvelteComponent: Interacts (clicks, answers)
SvelteComponent->>State: Updates state variables
SvelteComponent->>Derived: Updates derived values
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Possibly related PRs
Suggested reviewers
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/routes/quiz/[collection]/[id]/+page.svelte (3)
20-21: Consider using$effectinstead of legacyrunThe code imports
runfromsvelte/legacybut actually useseffectin the code. Since this is a migration to Svelte 5, consider importing$effectfrom 'svelte' instead of using the legacy import.-import { run } from 'svelte/legacy'; +import { $effect } from 'svelte';Then update line 283 to use
$effectinstead ofeffect:-effect(() => { +$effect(() => {
283-312: Optimize effect dependency trackingThe effect currently tracks all reactive values which may cause unnecessary re-runs. Consider tracking only the specific values that should trigger quiz reinitialization.
Consider using
$effect.preor tracking specific dependencies:-effect(() => { +$effect(() => { + // Only reinitialize when quiz data actually changes + if (!quiz && !quizQuestions.length) return; + if (quiz) {This prevents the effect from running when unrelated reactive values change.
36-36: Consider using $derived for currentQuizQuestion
currentQuizQuestionshould be a derived value rather than a separate state variable to ensure it's always in sync withquestionNumandquizQuestions.Replace the state declaration:
-let currentQuizQuestion = $state(); +let currentQuizQuestion = $derived(quizQuestions[questionNum]);Then remove the manual assignment on line 141:
-currentQuizQuestion = quizQuestions[questionNum];Also applies to: 141-141
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
-
src/routes/quiz/[collection]/[id]/+page.svelte(8 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: judah-sotomayor
PR: sillsdev/appbuilder-pwa#840
File: src/lib/components/NoteDialog.svelte:94-95
Timestamp: 2025-06-16T18:49:02.188Z
Learning: The appbuilder-pwa project uses Svelte 5, as evidenced by "svelte": "^5" in package.json and the use of Svelte 5 runes syntax like $props() and $derived(). In Svelte 5, onclick is the preferred event handler syntax over on:click.
Learnt from: judah-sotomayor
PR: sillsdev/appbuilder-pwa#840
File: src/lib/components/NoteDialog.svelte:68-70
Timestamp: 2025-06-16T18:48:21.767Z
Learning: In Svelte 5, the event handler syntax has changed from Svelte 4. The `onclick` attribute is valid in Svelte 5, unlike Svelte 4 where `on:click` was required.
Learnt from: chrisvire
PR: sillsdev/appbuilder-pwa#845
File: src/routes/text/+page.svelte:3-3
Timestamp: 2025-07-04T18:51:27.821Z
Learning: In SvelteKit 2.12 and Svelte 5, `$app/stores` was deprecated in favor of `$app/state`. When migrating from Svelte 4 to Svelte 5, imports should be changed from `import { page } from '$app/stores';` to `import { page } from '$app/state';`. This change aligns with Svelte 5's reactivity model and provides more granular control over state updates.
Learnt from: chrisvire
PR: sillsdev/appbuilder-pwa#845
File: src/routes/text/+page.svelte:3-3
Timestamp: 2025-07-04T18:51:27.821Z
Learning: In Svelte 5, the `page` store is imported from `$app/state` instead of `$app/stores`. This is part of the migration from Svelte 4 to Svelte 5 where `$app/stores` is being deprecated in favor of `$app/state`.
Learnt from: chrisvire
PR: sillsdev/appbuilder-pwa#845
File: src/routes/text/+page.svelte:201-209
Timestamp: 2025-07-04T18:53:08.369Z
Learning: In Svelte 5, `$state` creates a reactive rune, not a store. Stores are created with functions like `writable()`, `readable()`, and `derived()`. When using `bind:this` to bind DOM elements, use plain variables rather than reactive runes to ensure direct DOM property access works correctly.
Learnt from: judah-sotomayor
PR: sillsdev/appbuilder-pwa#840
File: src/lib/components/NoteDialog.svelte:68-70
Timestamp: 2025-06-16T18:48:21.767Z
Learning: In Svelte 5, event handler syntax changed from Svelte 4. Use `onclick={handler}` instead of `on:click={handler}`. Event modifiers like `|preventDefault` are no longer supported and must be handled inside the handler function. Only one handler per event is allowed per element.
Learnt from: chrisvire
PR: sillsdev/appbuilder-pwa#845
File: src/routes/text/+page.svelte:322-324
Timestamp: 2025-07-04T18:50:06.709Z
Learning: In Svelte 5, variables that hold the result of $derived runes should be accessed directly without the $ prefix. The $ prefix is only needed for traditional Svelte stores (writable(), readable(), derived()), not for variables that hold the result of runes like $state() or $derived().
Learnt from: chrisvire
PR: sillsdev/appbuilder-pwa#845
File: src/routes/text/+page.svelte:380-381
Timestamp: 2025-07-04T18:48:12.143Z
Learning: In Svelte 5, variables declared with $state() are state runes and should be accessed directly without the $ prefix. The $ prefix is only needed for stores (writable(), readable(), derived()), not for state runes. State runes are inherently reactive without requiring the $ prefix.
Learnt from: chrisvire
PR: sillsdev/appbuilder-pwa#839
File: src/lib/components/BottomNavigationBar.svelte:12-13
Timestamp: 2025-06-18T17:13:27.707Z
Learning: The $derived rune in Svelte 5 is specifically for creating computed reactive states that depend on other reactive variables, not for accessing component props. Using $derived to access props is incorrect and goes against Svelte 5 idioms.
Learnt from: judah-sotomayor
PR: sillsdev/appbuilder-pwa#820
File: src/lib/components/Modal.svelte:28-34
Timestamp: 2025-06-05T17:25:45.457Z
Learning: In the sillsdev/appbuilder-pwa repository, the Modal.svelte component has an existing bug where `{{ id }.showModal()}` syntax was used instead of calling the exported `showModal` function directly. This bug predates Svelte 5 migration efforts.
src/routes/quiz/[collection]/[id]/+page.svelte (12)
Learnt from: chrisvire
PR: #845
File: src/routes/text/+page.svelte:3-3
Timestamp: 2025-07-04T18:51:27.821Z
Learning: In Svelte 5, the page store is imported from $app/state instead of $app/stores. This is part of the migration from Svelte 4 to Svelte 5 where $app/stores is being deprecated in favor of $app/state.
Learnt from: chrisvire
PR: #845
File: src/routes/text/+page.svelte:3-3
Timestamp: 2025-07-04T18:51:27.821Z
Learning: In SvelteKit 2.12 and Svelte 5, $app/stores was deprecated in favor of $app/state. When migrating from Svelte 4 to Svelte 5, imports should be changed from import { page } from '$app/stores'; to import { page } from '$app/state';. This change aligns with Svelte 5's reactivity model and provides more granular control over state updates.
Learnt from: chrisvire
PR: #845
File: src/routes/text/+page.svelte:201-209
Timestamp: 2025-07-04T18:53:08.369Z
Learning: In Svelte 5, $state creates a reactive rune, not a store. Stores are created with functions like writable(), readable(), and derived(). When using bind:this to bind DOM elements, use plain variables rather than reactive runes to ensure direct DOM property access works correctly.
Learnt from: chrisvire
PR: #839
File: src/lib/components/BottomNavigationBar.svelte:12-13
Timestamp: 2025-06-18T17:13:27.707Z
Learning: The $derived rune in Svelte 5 is specifically for creating computed reactive states that depend on other reactive variables, not for accessing component props. Using $derived to access props is incorrect and goes against Svelte 5 idioms.
Learnt from: chrisvire
PR: #845
File: src/routes/text/+page.svelte:322-324
Timestamp: 2025-07-04T18:50:06.709Z
Learning: In Svelte 5, variables that hold the result of $derived runes should be accessed directly without the $ prefix. The $ prefix is only needed for traditional Svelte stores (writable(), readable(), derived()), not for variables that hold the result of runes like $state() or $derived().
Learnt from: chrisvire
PR: #845
File: src/routes/text/+page.svelte:380-381
Timestamp: 2025-07-04T18:48:12.143Z
Learning: In Svelte 5, variables declared with $state() are state runes and should be accessed directly without the $ prefix. The $ prefix is only needed for stores (writable(), readable(), derived()), not for state runes. State runes are inherently reactive without requiring the $ prefix.
Learnt from: judah-sotomayor
PR: #820
File: src/lib/components/Modal.svelte:28-34
Timestamp: 2025-06-05T17:25:45.457Z
Learning: In the sillsdev/appbuilder-pwa repository, the Modal.svelte component has an existing bug where {{ id }.showModal()} syntax was used instead of calling the exported showModal function directly. This bug predates Svelte 5 migration efforts.
Learnt from: chrisvire
PR: #839
File: src/lib/components/BottomNavigationBar.svelte:12-13
Timestamp: 2025-06-18T17:13:27.707Z
Learning: In Svelte 5, destructuring props using let { propName = defaultValue } = $props(); is the correct and idiomatic way to handle component props. This pattern maintains reactivity and is the recommended approach, not using $derived() for prop access.
Learnt from: chrisvire
PR: #839
File: src/lib/components/BottomNavigationBar.svelte:12-13
Timestamp: 2025-06-18T17:13:27.707Z
Learning: The standard Svelte 5 migration pattern converts export let propName = defaultValue; to let { propName = defaultValue } = $props(); while maintaining proper reactivity without requiring additional reactive constructs.
Learnt from: chrisvire
PR: #839
File: src/lib/components/BottomNavigationBar.svelte:12-13
Timestamp: 2025-06-18T17:13:27.707Z
Learning: In Svelte 5, destructuring props using let { propName = defaultValue } = $props(); is the correct and idiomatic way to handle component props. Props destructured from $props() maintain full reactivity and update automatically when parent components change the prop values.
Learnt from: judah-sotomayor
PR: #840
File: src/lib/components/TextAppearanceSelector.svelte:165-197
Timestamp: 2025-06-16T18:47:15.122Z
Learning: In src/lib/components/TextAppearanceSelector.svelte, the user judah-sotomayor considers theme selection buttons not relevant for screen-reader users and prefers not to add accessibility labels to theme buttons.
Learnt from: judah-sotomayor
PR: #840
File: src/lib/components/NoteDialog.svelte:68-70
Timestamp: 2025-06-16T18:48:21.767Z
Learning: In Svelte 5, event handler syntax changed from Svelte 4. Use onclick={handler} instead of on:click={handler}. Event modifiers like |preventDefault are no longer supported and must be handled inside the handler function. Only one handler per event is allowed per element.
🔇 Additional comments (5)
src/routes/quiz/[collection]/[id]/+page.svelte (5)
22-29: Props destructuring follows Svelte 5 best practicesThe migration correctly uses
$props()for destructuring props with proper TypeScript/JSDoc type annotations.
30-42: State variable declarations are correctly migratedAll reactive variables have been properly converted to use
$statewith appropriate initial values, following Svelte 5's reactive state management pattern.
326-327: Event handler syntax correctly migrated to Svelte 5All event handlers have been properly updated from
on:clicktoonclickfollowing Svelte 5's event handler syntax.Also applies to: 342-342, 409-411, 464-466, 487-489
337-339: Accessibility comments updated to Svelte 5 syntaxThe a11y ignore comments have been correctly updated to use underscores instead of hyphens.
Also applies to: 394-394, 458-459
136-146: No reactivity issues with nested mutations under Svelte 5’s Proxy-based $stateSvelte 5’s
$staterune uses Proxy-based deep reactivity, so mutating nested properties (e.g.answer.clicked = false) will automatically trigger updates in the UI. You don’t need to replace the entire object or reassign the array to ensure reactivity—this pattern is valid and functions as intended.Likely an incorrect or invalid review comment.
| let { locked, quiz, quizId, quizName, passScore } = $derived(data); | ||
| let book = $derived( | ||
| config.bookCollections | ||
| .find((x) => x.id === $refs.collection) | ||
| .books.find((x) => x.id === quizId) | ||
| ); | ||
| let displayLabel = $derived(quizName || 'Quiz'); | ||
| effect(() => { | ||
| const { locked, quiz, quizId, quizName, passScore } = data; | ||
| setBook( | ||
| config.bookCollections | ||
| .find((x) => x.id === $refs.collection) | ||
| .books.find((x) => x.id === quizId) | ||
| ); | ||
| setDisplayLabel(quizName || 'Quiz'); | ||
| if (quiz) { | ||
| resetQuizState(); | ||
| setScore(0); | ||
| setQuestionNum(0); | ||
| setShuffledAnswers([]); | ||
| const q = book.quizFeatures['shuffle-questions'] | ||
| ? shuffleArray([...quiz.questions]) | ||
| : [...quiz.questions]; | ||
| setQuizQuestions(q); | ||
| handleQuestionChange(q, 0); | ||
| playQuizQuestionAudio(); | ||
| setQuizSaved(false); | ||
| } else { | ||
| stopAudioPlayback(); | ||
| setScore(0); | ||
| setQuestionNum(0); | ||
| setShuffledAnswers([]); | ||
| setQuizQuestions([]); | ||
| setQuizSaved(false); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Critical issues in reactive state management
The effect implementation has several problems:
- Duplicate destructuring of
data(already handled by$derived) - Calling non-existent setter functions - Svelte 5 uses direct assignment to state variables
handleQuestionChangecalled with wrong parameters
Apply this fix:
let { locked, quiz, quizId, quizName, passScore } = $derived(data);
let book = $derived(
config.bookCollections
.find((x) => x.id === $refs.collection)
.books.find((x) => x.id === quizId)
);
let displayLabel = $derived(quizName || 'Quiz');
effect(() => {
- const { locked, quiz, quizId, quizName, passScore } = data;
- setBook(
- config.bookCollections
- .find((x) => x.id === $refs.collection)
- .books.find((x) => x.id === quizId)
- );
- setDisplayLabel(quizName || 'Quiz');
-
if (quiz) {
resetQuizState();
- setScore(0);
- setQuestionNum(0);
- setShuffledAnswers([]);
+ score = 0;
+ questionNum = 0;
+ shuffledAnswers = [];
const q = book.quizFeatures['shuffle-questions']
? shuffleArray([...quiz.questions])
: [...quiz.questions];
- setQuizQuestions(q);
- handleQuestionChange(q, 0);
+ quizQuestions = q;
+ handleQuestionChange();
playQuizQuestionAudio();
- setQuizSaved(false);
+ quizSaved = false;
} else {
stopAudioPlayback();
- setScore(0);
- setQuestionNum(0);
- setShuffledAnswers([]);
- setQuizQuestions([]);
- setQuizSaved(false);
+ score = 0;
+ questionNum = 0;
+ shuffledAnswers = [];
+ quizQuestions = [];
+ quizSaved = false;
}
});🤖 Prompt for AI Agents
In src/routes/quiz/[collection]/[id]/+page.svelte lines 276 to 312, remove the
duplicate destructuring of data inside the effect since $derived already
provides reactive values. Replace all calls to non-existent setter functions
like setBook, setDisplayLabel, setScore, setQuestionNum, setShuffledAnswers,
setQuizQuestions, setQuizSaved with direct assignments to their corresponding
state variables. Correct the call to handleQuestionChange by passing the current
question index and the question object instead of the entire question array and
index. This will align the reactive state management with Svelte 5 conventions
and fix the parameter usage.
1 participant
Summary by CodeRabbit
No changes to visible quiz functionality or UI.