From 8755cd59fda693cb792e7be092f2e60e0e244a2b Mon Sep 17 00:00:00 2001 From: Daniel Dietzler <36593685+danieldietzler@users.noreply.github.com> Date: Tue, 25 Nov 2025 00:57:46 +0100 Subject: [PATCH] chore: refactor svelte reactivity (#24072) --- e2e/src/mock-network/timeline-network.ts | 44 ++++++++++++------- .../asset-viewer/activity-viewer.svelte | 8 +--- .../asset-viewer/album-list-item.svelte | 6 +-- .../asset-viewer/asset-viewer.svelte | 10 ++--- .../asset-viewer/photo-viewer.svelte | 1 - .../asset-viewer/video-native-viewer.svelte | 12 ++--- .../asset-viewer/video-remote-viewer.svelte | 1 - .../components/places-page/places-list.svelte | 37 +++++----------- .../shared-components/change-location.svelte | 5 +-- .../context-menu/context-menu.svelte | 39 ++++++++-------- .../search-bar/search-camera-section.svelte | 7 +-- .../search-bar/search-location-section.svelte | 18 +++----- web/src/lib/stores/ocr.svelte.ts | 8 ++-- web/src/routes/(user)/+layout.svelte | 12 +++-- .../[[assetId=id]]/+page.svelte | 7 ++- web/src/routes/+layout.svelte | 4 +- web/src/routes/auth/onboarding/+page.svelte | 14 +++--- 17 files changed, 105 insertions(+), 128 deletions(-) diff --git a/e2e/src/mock-network/timeline-network.ts b/e2e/src/mock-network/timeline-network.ts index 012defe4ab..59bce71dd8 100644 --- a/e2e/src/mock-network/timeline-network.ts +++ b/e2e/src/mock-network/timeline-network.ts @@ -62,50 +62,60 @@ export const setupTimelineMockApiRoutes = async ( return route.continue(); }); - await context.route('**/api/assets/**', async (route, request) => { + await context.route('**/api/assets/*', async (route, request) => { + const url = new URL(request.url()); + const pathname = url.pathname; + const assetId = basename(pathname); + const asset = getAsset(timelineRestData, assetId); + return route.fulfill({ + status: 200, + contentType: 'application/json', + json: asset, + }); + }); + + await context.route('**/api/assets/*/ocr', async (route) => { + return route.fulfill({ status: 200, contentType: 'application/json', json: [] }); + }); + + await context.route('**/api/assets/*/thumbnail?size=*', async (route, request) => { const pattern = /\/api\/assets\/(?[^/]+)\/thumbnail\?size=(?preview|thumbnail)/; const match = request.url().match(pattern); - if (!match) { - const url = new URL(request.url()); - const pathname = url.pathname; - const assetId = basename(pathname); - const asset = getAsset(timelineRestData, assetId); - return route.fulfill({ - status: 200, - contentType: 'application/json', - json: asset, - }); + if (!match?.groups) { + throw new Error(`Invalid URL for thumbnail endpoint: ${request.url()}`); } - if (match.groups?.size === 'preview') { + + if (match.groups.size === 'preview') { if (!route.request().serviceWorker()) { return route.continue(); } - const asset = getAsset(timelineRestData, match.groups?.assetId); + const asset = getAsset(timelineRestData, match.groups.assetId); return route.fulfill({ status: 200, headers: { 'content-type': 'image/jpeg', ETag: 'abc123', 'Cache-Control': 'public, max-age=3600' }, body: await randomPreview( - match.groups?.assetId, + match.groups.assetId, (asset?.exifInfo?.exifImageWidth ?? 0) / (asset?.exifInfo?.exifImageHeight ?? 1), ), }); } - if (match.groups?.size === 'thumbnail') { + if (match.groups.size === 'thumbnail') { if (!route.request().serviceWorker()) { return route.continue(); } - const asset = getAsset(timelineRestData, match.groups?.assetId); + const asset = getAsset(timelineRestData, match.groups.assetId); return route.fulfill({ status: 200, headers: { 'content-type': 'image/jpeg' }, body: await randomThumbnail( - match.groups?.assetId, + match.groups.assetId, (asset?.exifInfo?.exifImageWidth ?? 0) / (asset?.exifInfo?.exifImageHeight ?? 1), ), }); } return route.continue(); }); + await context.route('**/api/albums/**', async (route, request) => { const pattern = /\/api\/albums\/(?[^/?]+)/; const match = request.url().match(pattern); diff --git a/web/src/lib/components/asset-viewer/activity-viewer.svelte b/web/src/lib/components/asset-viewer/activity-viewer.svelte index 73b311769a..d688b2e9dd 100644 --- a/web/src/lib/components/asset-viewer/activity-viewer.svelte +++ b/web/src/lib/components/asset-viewer/activity-viewer.svelte @@ -52,7 +52,7 @@ let innerHeight: number = $state(0); let activityHeight: number = $state(0); let chatHeight: number = $state(0); - let divHeight: number = $state(0); + let divHeight = $derived(innerHeight - activityHeight); let previousAssetId: string | undefined = $state(assetId); let message = $state(''); let isSendingMessage = $state(false); @@ -96,11 +96,7 @@ } isSendingMessage = false; }; - $effect(() => { - if (innerHeight && activityHeight) { - divHeight = innerHeight - activityHeight; - } - }); + $effect(() => { if (assetId && previousAssetId != assetId) { previousAssetId = assetId; diff --git a/web/src/lib/components/asset-viewer/album-list-item.svelte b/web/src/lib/components/asset-viewer/album-list-item.svelte index 21b9b385d8..da0df21839 100644 --- a/web/src/lib/components/asset-viewer/album-list-item.svelte +++ b/web/src/lib/components/asset-viewer/album-list-item.svelte @@ -35,15 +35,13 @@ }); }; - let albumNameArray: string[] = $state(['', '', '']); - // This part of the code is responsible for splitting album name into 3 parts where part 2 is the search query // It is used to highlight the search query in the album name - $effect(() => { + const albumNameArray: string[] = $derived.by(() => { let { albumName } = album; let findIndex = normalizeSearchString(albumName).indexOf(normalizeSearchString(searchQuery)); let findLength = searchQuery.length; - albumNameArray = [ + return [ albumName.slice(0, findIndex), albumName.slice(findIndex, findIndex + findLength), albumName.slice(findIndex + findLength), diff --git a/web/src/lib/components/asset-viewer/asset-viewer.svelte b/web/src/lib/components/asset-viewer/asset-viewer.svelte index 0af27e8373..4e23206659 100644 --- a/web/src/lib/components/asset-viewer/asset-viewer.svelte +++ b/web/src/lib/components/asset-viewer/asset-viewer.svelte @@ -395,13 +395,11 @@ } }); - let currentAssetId = $derived(asset.id); + // primarily, this is reactive on `asset` $effect(() => { - if (currentAssetId) { - untrack(() => handlePromiseError(handleGetAllAlbums())); - ocrManager.clear(); - handlePromiseError(ocrManager.getAssetOcr(currentAssetId)); - } + handlePromiseError(handleGetAllAlbums()); + ocrManager.clear(); + handlePromiseError(ocrManager.getAssetOcr(asset.id)); }); diff --git a/web/src/lib/components/asset-viewer/photo-viewer.svelte b/web/src/lib/components/asset-viewer/photo-viewer.svelte index 261f194d34..2607f6de79 100644 --- a/web/src/lib/components/asset-viewer/photo-viewer.svelte +++ b/web/src/lib/components/asset-viewer/photo-viewer.svelte @@ -171,7 +171,6 @@ $effect(() => { if (assetFileUrl) { - // this can't be in an async context with $effect void cast(assetFileUrl); } }); diff --git a/web/src/lib/components/asset-viewer/video-native-viewer.svelte b/web/src/lib/components/asset-viewer/video-native-viewer.svelte index 92c467bc1e..a25789a76c 100644 --- a/web/src/lib/components/asset-viewer/video-native-viewer.svelte +++ b/web/src/lib/components/asset-viewer/video-native-viewer.svelte @@ -43,7 +43,9 @@ let videoPlayer: HTMLVideoElement | undefined = $state(); let isLoading = $state(true); - let assetFileUrl = $state(''); + let assetFileUrl = $derived( + playOriginalVideo ? getAssetOriginalUrl({ id: assetId, cacheKey }) : getAssetPlaybackUrl({ id: assetId, cacheKey }), + ); let isScrubbing = $state(false); let showVideo = $state(false); @@ -53,11 +55,9 @@ }); $effect(() => { - assetFileUrl = playOriginalVideo - ? getAssetOriginalUrl({ id: assetId, cacheKey }) - : getAssetPlaybackUrl({ id: assetId, cacheKey }); - if (videoPlayer) { - videoPlayer.load(); + // reactive on `assetFileUrl` changes + if (assetFileUrl) { + videoPlayer?.load(); } }); diff --git a/web/src/lib/components/asset-viewer/video-remote-viewer.svelte b/web/src/lib/components/asset-viewer/video-remote-viewer.svelte index 392028c49f..94a7e748c5 100644 --- a/web/src/lib/components/asset-viewer/video-remote-viewer.svelte +++ b/web/src/lib/components/asset-viewer/video-remote-viewer.svelte @@ -35,7 +35,6 @@ $effect(() => { if (assetFileUrl) { - // this can't be in an async context with $effect void cast(assetFileUrl); } }); diff --git a/web/src/lib/components/places-page/places-list.svelte b/web/src/lib/components/places-page/places-list.svelte index 3da4772e0c..bcb90cb18e 100644 --- a/web/src/lib/components/places-page/places-list.svelte +++ b/web/src/lib/components/places-page/places-list.svelte @@ -9,7 +9,6 @@ import { type PlacesGroup, getSelectedPlacesGroupOption } from '$lib/utils/places-utils'; import { Icon } from '@immich/ui'; import { t } from 'svelte-i18n'; - import { run } from 'svelte/legacy'; interface Props { places?: AssetResponseDto[]; @@ -70,39 +69,27 @@ }, }; - let filteredPlaces: AssetResponseDto[] = $state([]); - let groupedPlaces: PlacesGroup[] = $state([]); - - let placesGroupOption: string = $state(PlacesGroupBy.None); - - let hasPlaces = $derived(places.length > 0); - - // Step 1: Filter using the given search query. - run(() => { - if (searchQuery) { - const searchQueryNormalized = normalizeSearchString(searchQuery); + const filteredPlaces = $derived.by(() => { + const searchQueryNormalized = normalizeSearchString(searchQuery); + return searchQueryNormalized + ? places.filter((place) => normalizeSearchString(place.exifInfo?.city ?? '').includes(searchQueryNormalized)) + : places; + }); - filteredPlaces = places.filter((place) => { - return normalizeSearchString(place.exifInfo?.city ?? '').includes(searchQueryNormalized); - }); - } else { - filteredPlaces = places; - } + const placesGroupOption: string = $derived(getSelectedPlacesGroupOption(userSettings)); + const groupingFunction = $derived(groupOptions[placesGroupOption] ?? groupOptions[PlacesGroupBy.None]); + const groupedPlaces: PlacesGroup[] = $derived(groupingFunction(filteredPlaces)); + $effect(() => { searchResultCount = filteredPlaces.length; }); - // Step 2: Group places. - run(() => { - placesGroupOption = getSelectedPlacesGroupOption(userSettings); - const groupFunc = groupOptions[placesGroupOption] ?? groupOptions[PlacesGroupBy.None]; - groupedPlaces = groupFunc(filteredPlaces); - + $effect(() => { placesGroupIds = groupedPlaces.map(({ id }) => id); }); -{#if hasPlaces} +{#if places.length > 0} {#if placesGroupOption === PlacesGroupBy.None} diff --git a/web/src/lib/components/shared-components/change-location.svelte b/web/src/lib/components/shared-components/change-location.svelte index 23fd00190e..0102e34977 100644 --- a/web/src/lib/components/shared-components/change-location.svelte +++ b/web/src/lib/components/shared-components/change-location.svelte @@ -27,7 +27,7 @@ let { asset = undefined, point: initialPoint, onClose }: Props = $props(); let places: PlacesResponseDto[] = $state([]); - let suggestedPlaces: PlacesResponseDto[] = $state([]); + let suggestedPlaces: PlacesResponseDto[] = $derived(places.slice(0, 5)); let searchWord: string = $state(''); let latestSearchTimeout: number; let showLoadingSpinner = $state(false); @@ -52,9 +52,6 @@ }); $effect(() => { - if (places) { - suggestedPlaces = places.slice(0, 5); - } if (searchWord === '') { suggestedPlaces = []; } diff --git a/web/src/lib/components/shared-components/context-menu/context-menu.svelte b/web/src/lib/components/shared-components/context-menu/context-menu.svelte index f63cbd0621..7bf9ba58b3 100644 --- a/web/src/lib/components/shared-components/context-menu/context-menu.svelte +++ b/web/src/lib/components/shared-components/context-menu/context-menu.svelte @@ -33,37 +33,36 @@ children, }: Props = $props(); - let left: number = $state(0); - let top: number = $state(0); + const swap = (direction: string) => (direction === 'left' ? 'right' : 'left'); + + const layoutDirection = $derived(languageManager.rtl ? swap(direction) : direction); + const position = $derived.by(() => { + if (!menuElement) { + return { left: 0, top: 0 }; + } + + const rect = menuElement.getBoundingClientRect(); + const directionWidth = layoutDirection === 'left' ? rect.width : 0; + const menuHeight = Math.min(menuElement.clientHeight, height) || 0; + + const left = Math.max(8, Math.min(window.innerWidth - rect.width, x - directionWidth)); + const top = Math.max(8, Math.min(window.innerHeight - menuHeight, y)); + + return { left, top }; + }); // We need to bind clientHeight since the bounding box may return a height // of zero when starting the 'slide' animation. let height: number = $state(0); let isTransitioned = $state(false); - - $effect(() => { - if (menuElement) { - let layoutDirection = direction; - if (languageManager.rtl) { - layoutDirection = direction === 'left' ? 'right' : 'left'; - } - - const rect = menuElement.getBoundingClientRect(); - const directionWidth = layoutDirection === 'left' ? rect.width : 0; - const menuHeight = Math.min(menuElement.clientHeight, height) || 0; - - left = Math.max(8, Math.min(window.innerWidth - rect.width, x - directionWidth)); - top = Math.max(8, Math.min(window.innerHeight - menuHeight, y)); - } - });
{ diff --git a/web/src/lib/components/shared-components/search-bar/search-camera-section.svelte b/web/src/lib/components/shared-components/search-bar/search-camera-section.svelte index 1d451eacc4..ac158aa8a3 100644 --- a/web/src/lib/components/shared-components/search-bar/search-camera-section.svelte +++ b/web/src/lib/components/shared-components/search-bar/search-camera-section.svelte @@ -64,10 +64,11 @@ } } - let makeFilter = $derived(filters.make); - let modelFilter = $derived(filters.model); - let lensModelFilter = $derived(filters.lensModel); + const makeFilter = $derived(filters.make); + const modelFilter = $derived(filters.model); + const lensModelFilter = $derived(filters.lensModel); + // TODO replace by async $derived, at the latest when it's in stable https://svelte.dev/docs/svelte/await-expressions $effect(() => { handlePromiseError(updateMakes()); }); diff --git a/web/src/lib/components/shared-components/search-bar/search-location-section.svelte b/web/src/lib/components/shared-components/search-bar/search-location-section.svelte index cba5e105af..37a4a3ca9b 100644 --- a/web/src/lib/components/shared-components/search-bar/search-location-section.svelte +++ b/web/src/lib/components/shared-components/search-bar/search-location-section.svelte @@ -7,11 +7,10 @@
diff --git a/web/src/lib/stores/ocr.svelte.ts b/web/src/lib/stores/ocr.svelte.ts index 4922f630ec..f9862b1edc 100644 --- a/web/src/lib/stores/ocr.svelte.ts +++ b/web/src/lib/stores/ocr.svelte.ts @@ -19,21 +19,23 @@ export type OcrBoundingBox = { class OcrManager { #data = $state([]); showOverlay = $state(false); - hasOcrData = $state(false); + #hasOcrData = $derived(this.#data.length > 0); get data() { return this.#data; } + get hasOcrData() { + return this.#hasOcrData; + } + async getAssetOcr(id: string) { this.#data = await getAssetOcr({ id }); - this.hasOcrData = this.#data.length > 0; } clear() { this.#data = []; this.showOverlay = false; - this.hasOcrData = false; } toggleOcrBoundingBox() { diff --git a/web/src/routes/(user)/+layout.svelte b/web/src/routes/(user)/+layout.svelte index ea10c45444..e6e349fe91 100644 --- a/web/src/routes/(user)/+layout.svelte +++ b/web/src/routes/(user)/+layout.svelte @@ -1,8 +1,6 @@ diff --git a/web/src/routes/(user)/search/[[photos=photos]]/[[assetId=id]]/+page.svelte b/web/src/routes/(user)/search/[[photos=photos]]/[[assetId=id]]/+page.svelte index 97964344ef..b58210187b 100644 --- a/web/src/routes/(user)/search/[[photos=photos]]/[[assetId=id]]/+page.svelte +++ b/web/src/routes/(user)/search/[[photos=photos]]/[[assetId=id]]/+page.svelte @@ -44,7 +44,7 @@ } from '@immich/sdk'; import { Icon, IconButton, LoadingSpinner } from '@immich/ui'; import { mdiArrowLeft, mdiDotsVertical, mdiImageOffOutline, mdiPlus, mdiSelectAll } from '@mdi/js'; - import { tick } from 'svelte'; + import { tick, untrack } from 'svelte'; import { t } from 'svelte-i18n'; let { isViewing: showAssetViewer } = assetViewingStore; @@ -71,11 +71,10 @@ let terms = $derived(searchQuery ? JSON.parse(searchQuery) : {}); $effect(() => { + // we want this to *only* be reactive on `terms` // eslint-disable-next-line @typescript-eslint/no-unused-expressions terms; - setTimeout(() => { - handlePromiseError(onSearchQueryUpdate()); - }); + untrack(() => handlePromiseError(onSearchQueryUpdate())); }); const onEscape = () => { diff --git a/web/src/routes/+layout.svelte b/web/src/routes/+layout.svelte index 80d4da1252..379b6b00d6 100644 --- a/web/src/routes/+layout.svelte +++ b/web/src/routes/+layout.svelte @@ -22,7 +22,6 @@ import { modalManager, setTranslations } from '@immich/ui'; import { onMount, type Snippet } from 'svelte'; import { t } from 'svelte-i18n'; - import { run } from 'svelte/legacy'; import '../app.css'; interface Props { @@ -69,7 +68,8 @@ afterNavigate(() => { showNavigationLoadingBar = false; }); - run(() => { + + $effect.pre(() => { if ($user || page.url.pathname.startsWith(AppRoute.MAINTENANCE)) { openWebsocketConnection(); } else { diff --git a/web/src/routes/auth/onboarding/+page.svelte b/web/src/routes/auth/onboarding/+page.svelte index 44cd97637a..ed4235104c 100644 --- a/web/src/routes/auth/onboarding/+page.svelte +++ b/web/src/routes/auth/onboarding/+page.svelte @@ -1,6 +1,6 @@