Fix: Build warnings: svelte-kit sync prepare step, reactive shortcuts, and stale QR/link URL#666
Conversation
… shortcut list and QR/link URL)
📝 WalkthroughWalkthroughThis PR adds a ChangesReactivity and Build Tooling Updates
Estimated code review effort: 1 (Trivial) | ~5 minutes Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Pull request overview
This PR removes Svelte/SvelteKit build-time warnings in immichFrame.Web and fixes a real runtime issue where the QR/link in the info overlay could remain stuck on the first-rendered asset as the slideshow advances.
Changes:
- Add an npm
preparescript to runsvelte-kit syncon install so.svelte-kit/tsconfig.jsonexists beforevite build. - Make
overlay-controls.svelteshortcut list reactive via$derived(...)to satisfy Svelte 5 reactivity expectations. - Fix stale “Open in immich” link / QR data in
overlay-qr.svelteby derivingimageUrlfrom the reactiveidprop.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| immichFrame.Web/src/lib/components/elements/overlay-controls.svelte | Makes the keyboard shortcut list reactive via $derived to eliminate reactivity warnings. |
| immichFrame.Web/src/lib/components/elements/imageoverlay/overlay-qr.svelte | Derives imageUrl from id to keep QR/link updated as the displayed asset changes. |
| immichFrame.Web/package.json | Adds prepare: svelte-kit sync so .svelte-kit artifacts exist on fresh installs before builds. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
immichFrame.Web/src/lib/components/elements/overlay-controls.svelte (1)
32-68: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAdd an
updatehandler toshortcuts
use:shortcuts={shortcutList}won’t react to prop changes unless the action exposesupdate(newList). As written, thekeydownlistener stays bound to the mount-time callbacks, so the inline handlers passed fromhome-page.sveltecan go stale on later renders.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@immichFrame.Web/src/lib/components/elements/overlay-controls.svelte` around lines 32 - 68, The `shortcuts` action in `overlay-controls.svelte` only handles mount and destroy, so it won’t react when `shortcutList` changes and the `keydown` listener can keep stale callbacks. Add an `update(newList)` method to `shortcuts` that refreshes the stored shortcut list used by `handleKeyDown`, and make sure `use:shortcuts={shortcutList}` in the same component can rebind to the latest `next`, `back`, `pause`, and `showInfo` actions after rerenders.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@immichFrame.Web/src/lib/components/elements/overlay-controls.svelte`:
- Around line 32-68: The `shortcuts` action in `overlay-controls.svelte` only
handles mount and destroy, so it won’t react when `shortcutList` changes and the
`keydown` listener can keep stale callbacks. Add an `update(newList)` method to
`shortcuts` that refreshes the stored shortcut list used by `handleKeyDown`, and
make sure `use:shortcuts={shortcutList}` in the same component can rebind to the
latest `next`, `back`, `pause`, and `showInfo` actions after rerenders.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: df2ff92f-850f-4fda-a28e-24878685d53b
📒 Files selected for processing (3)
immichFrame.Web/package.jsonimmichFrame.Web/src/lib/components/elements/imageoverlay/overlay-qr.svelteimmichFrame.Web/src/lib/components/elements/overlay-controls.svelte
JW-CH
changed the title
Summary
Fixes three build-time warnings seen in
immichFrame.Web:svelte-kit syncprepare step:vite buildwarnsCannot find base config file "./.svelte-kit/tsconfig.json"on a fresh checkout, since.svelte-kit/doesn't exist yet when Vite first readstsconfig.json. Added the standard"prepare": "svelte-kit sync"script (the usualcreate-sveltescaffold entry) sonpm igenerates it automatically beforenpm run buildever runs.overlay-controls.sveltereactivity warning:shortcutListwas built once as a plainconstfrom thenext/back/pause/showInfoprops instead of reactively. Wrapped it in$derived(...), matching the Svelte 5 compiler's own suggested fix.overlay-qr.sveltestale QR/link (actual bug):imageUrl(used for both the QR code and the "Open in immich" link) was computed once from theidprop at component init. Sinceimage-overlay.svelterenders<OverlayQr id={asset.id} />without forcing a remount, the QR code and link would silently keep pointing at whatever asset was current the first time the component was created, even as the slideshow advanced underneath. WrappedimageUrlin$derived(...)so it now tracks the current asset.No
Dockerfile,docker-compose.yml, or config changes — this is a 3-line, 3-file fix.Test plan
npm run check(svelte-check) — 0 errors, 0 warningsnpm run build— confirmed all four warning strings (Cannot find base config file,overlay-controls.svelte,overlay-qr.svelte,state_referenced_locally) are gone from the outputdocker buildof this branch — same warnings confirmed absent from the full build logoverlay-controls.svelte) still work correctly (tested theishortcut for opening the info overlay)Summary by CodeRabbit