Feature/Migrate annotation files to Svelte 5 by joslane · Pull Request #889 · sillsdev/appbuilder-pwa

joslane · 2025-07-30T20:20:05Z

Migrated bookmarks, highlights, history, and notes to Svelte 5
Replaced on:click with onclick
Replaced reactive declarations with state
Avoided use of deprecated let reactivity

Summary by CodeRabbit

Refactor
- Converted several page variables to reactive local state to improve UI reactivity and consistency (including sort order and local data/text handling).
- Switched some event bindings from framework-specific directives to standard HTML onclick handlers.
- Adjusted an accessibility lint directive format.
Style
- Minor code-structure tidy-ups for maintainability.

- Migrated bookmarks, highlights, history, and notes to Svelte 5 - Replaced on:click with onclick - Replaced reactive declarations with `state` - Avoided use of deprecated `let` reactivity

coderabbitai · 2025-07-30T20:20:12Z

Walkthrough

Refactors several Svelte page components to adopt Svelte 5 reactive primitives: replaces direct variable initializations with $state(...), replaces exported props with destructured $props(), converts some Svelte event directives (on:click) to standard DOM attributes (onclick), and adjusts an a11y lint ignore token.

Changes

Cohort / File(s)	Change Summary
sortOrder → $state `src/routes/bookmarks/+page.svelte`, `src/routes/highlights/+page.svelte`, `src/routes/notes/+page.svelte`	Replaced `let sortOrder = SORT_DATE;` with `let sortOrder = $state(SORT_DATE);` to make `sortOrder` a reactive/state value. In `highlights`, updated an a11y ignore comment token format.
Props → $props() and local state `src/routes/history/+page.svelte`	Removed `export let data;` and now reads props via `let { data } = $props();`. Introduced local `history = $state([...data.history].reverse());` and updated template/logic to use the local `history` (including clearing behavior). Also switched some `on:click` usage to `onclick`.
Notes edit: props/text/textarea → $state/$props() `src/routes/notes/edit/[noteid]/+page.svelte`	Replaced `export let data;` with `let { data } = $props();`. Initialized `text` and `textarea` using `$state(...)`. Switched button event bindings from Svelte `on:click` to DOM `onclick`.

Sequence Diagram(s)

sequenceDiagram
    participant Page as Page Component
    participant State as Svelte $state

    Page->>State: call $state(defaultValue)
    State-->>Page: return reactive value
    Note over Page,State: component uses reactive value (e.g., sortOrder)

sequenceDiagram
    participant Page as Page Component
    participant Props as $props provider

    Page->>Props: call $props()
    Props-->>Page: return props object (e.g., { data })
    Page->>Page: initialize local $state derived from props (e.g., history)
    Note over Page: subsequent UI reads local state, not direct prop mutations

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Migrate Plans to Svelte 5 #893 — Svelte 5 migration: converts local vars to $state, swaps on:click → onclick, and updates a11y lint comments.
Feature/migrate contents & about #887 — Prop migration: replaces export let ... with $props() destructuring and related reactive adjustments.
Migrate search to Svelte 5 #826 — Broad Svelte 5 refactor: moves to $state/$props() patterns across route components and updates event binding syntax.

Poem

I hop through code with whiskers bright,
$state in paw, props tucked tight.
Old exports tucked beneath a log,
onclicks chatter, lint notes nod.
A tiny rabbit cheers the sight—new signals in the night 🐇✨

✨ Finishing Touches

🧪 Generate unit tests

Create PR with unit tests
Post copyable unit tests in a comment

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.

❤️ Share

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>, please review it.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

Add @coderabbitai ignore or @coderabbit ignore anywhere in the PR description to prevent this PR from being reviewed.
Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
Please see the configuration documentation for more information.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

Visit our Status Page to check the current availability of CodeRabbit.
Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

joslane · 2025-07-30T20:20:18Z

#819

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (1)

src/routes/notes/edit/[noteid]/+page.svelte (1)
10-10: Consider using a plain variable for DOM binding.

Since textarea is used with bind:this={textarea} on line 98, consider using a plain variable instead of $state() to ensure direct DOM property access works correctly.
-    let textarea = $state();
+    let textarea;

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3fc64e3 and 43969d9.

📒 Files selected for processing (5)

src/routes/bookmarks/+page.svelte (1 hunks)
src/routes/highlights/+page.svelte (2 hunks)
src/routes/history/+page.svelte (2 hunks)
src/routes/notes/+page.svelte (1 hunks)
src/routes/notes/edit/[noteid]/+page.svelte (2 hunks)

🧰 Additional context used

🧠 Learnings (6)

📓 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#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: 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: 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: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#839
File: src/lib/components/BottomNavigationBar.svelte:12-13
Timestamp: 2025-06-18T17:13:27.707Z
Learning: The conversion from `export let propName = defaultValue;` (Svelte 4) to `let { propName = defaultValue } = $props();` (Svelte 5) is the standard migration pattern and maintains proper reactivity without requiring additional reactive constructs.

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: chrisvire
PR: sillsdev/appbuilder-pwa#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: 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: 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.

src/routes/bookmarks/+page.svelte (6)

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: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: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: #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: #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().

src/routes/highlights/+page.svelte (8)

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: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: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: #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: #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: 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: 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: 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.

src/routes/history/+page.svelte (9)

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: #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: chrisvire
PR: #839
File: src/lib/components/BottomNavigationBar.svelte:12-13
Timestamp: 2025-06-18T17:13:27.707Z
Learning: The conversion from export let propName = defaultValue; (Svelte 4) to let { propName = defaultValue } = $props(); (Svelte 5) is the standard migration pattern and maintains proper reactivity without requiring additional reactive constructs.

Learnt from: judah-sotomayor
PR: #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: #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: judah-sotomayor
PR: #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: #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.

src/routes/notes/+page.svelte (7)

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: #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: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: #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: #839
File: src/lib/components/BottomNavigationBar.svelte:12-13
Timestamp: 2025-06-18T17:13:27.707Z
Learning: The conversion from export let propName = defaultValue; (Svelte 4) to let { propName = defaultValue } = $props(); (Svelte 5) is the standard migration pattern and maintains 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: 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.

src/routes/notes/edit/[noteid]/+page.svelte (11)