From f3e66c2feea5ea89e0eff92ed73f2ef0bf403e5b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fernando=20Fern=C3=A1ndez?= <ferferga@hotmail.com>
Date: Sat, 18 May 2024 12:54:53 +0200
Subject: [PATCH 1/2] refactor: non working watchers in all branches
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Fernando Fernández <ferferga@hotmail.com>
---
 .../src/components/Layout/AppBar/AppBar.vue   |  4 +-
 frontend/src/plugins/remote/sdk/index.ts      |  4 +-
 frontend/src/plugins/router/index.ts          | 57 +++++++++----------
 frontend/src/store/index.ts                   | 13 ++++-
 4 files changed, 40 insertions(+), 38 deletions(-)

diff --git a/frontend/src/components/Layout/AppBar/AppBar.vue b/frontend/src/components/Layout/AppBar/AppBar.vue
index d5ed673b022..dd77d974dbe 100644
--- a/frontend/src/components/Layout/AppBar/AppBar.vue
+++ b/frontend/src/components/Layout/AppBar/AppBar.vue
@@ -19,8 +19,8 @@
     <SearchField />
     <VSpacer />
     <AppBarButtonLayout
-      v-if="!remote.socket.isConnected.value || !isConnectedToServer"
-      :color="!remote.socket.isConnected.value ? 'yellow' : 'red'">
+      v-hide="remote.socket.isConnected.value && isConnectedToServer"
+      :color="isConnectedToServer ? 'yellow' : 'red'">
       <template #icon>
         <VIcon>
           <IMdiNetworkOffOutline />
diff --git a/frontend/src/plugins/remote/sdk/index.ts b/frontend/src/plugins/remote/sdk/index.ts
index 9a2c02306a9..7a942cda2c9 100644
--- a/frontend/src/plugins/remote/sdk/index.ts
+++ b/frontend/src/plugins/remote/sdk/index.ts
@@ -3,7 +3,7 @@
  * It also sets the header and base URL for our axios instance
  */
 import type { Api } from '@jellyfin/sdk';
-import { watchEffect } from 'vue';
+import { watchSyncEffect } from 'vue';
 import RemotePluginAuthInstance from '../auth';
 import RemotePluginAxiosInstance from '../axios';
 import SDK, { useOneTimeAPI } from './sdk-utils';
@@ -21,7 +21,7 @@ class RemotePluginSDK {
     /**
      * Configure app's axios instance to perform requests to the given Jellyfin server.
      */
-    watchEffect(() => {
+    watchSyncEffect(() => {
       const server = auth.currentServer;
       const accessToken = auth.currentUserToken;
 
diff --git a/frontend/src/plugins/router/index.ts b/frontend/src/plugins/router/index.ts
index 4c966ba62bd..a337d85b863 100644
--- a/frontend/src/plugins/router/index.ts
+++ b/frontend/src/plugins/router/index.ts
@@ -1,4 +1,4 @@
-import { watch } from 'vue';
+import { watchSyncEffect } from 'vue';
 import {
   createRouter,
   createWebHashHistory,
@@ -64,35 +64,30 @@ router.back = (): ReturnType<typeof router.back> => {
 /**
  * Re-run the middleware pipeline when the user logs out or state is cleared
  */
-watch(
-  [
-    (): typeof remote.auth.currentUser => remote.auth.currentUser,
-    (): typeof remote.auth.servers => remote.auth.servers
-  ],
-  async () => {
-    if (!remote.auth.currentUser && remote.auth.servers.length <= 0) {
-      /**
-       * We run the redirect to /server/add as it's the first page in the login flow
-       *
-       * In case the whole localStorage is gone at runtime, if we're at the login
-       * page, redirecting to /server/login wouldn't work, as we're in that same page.
-       * /server/add doesn't depend on the state of localStorage, so it's always safe to
-       * redirect there and leave the middleware take care of the final destination
-       * (when servers are already available, for example)
-       */
-      await router.replace('/server/add');
-    } else if (
-      !remote.auth.currentUser
-      && remote.auth.servers.length
-      && remote.auth.currentServer
-    ) {
-      await (remote.auth.currentServer.StartupWizardCompleted ? router.replace('/server/login') : router.replace('/wizard'));
-    } else if (
-      !remote.auth.currentUser
-      && remote.auth.servers.length
-      && !remote.auth.currentServer
-    ) {
-      await router.replace('/server/select');
-    }
+watchSyncEffect(() => {
+  if (!remote.auth.currentUser && remote.auth.servers.length <= 0) {
+    /**
+     * We run the redirect to /server/add as it's the first page in the login flow
+     *
+     * In case the whole localStorage is gone at runtime, if we're at the login
+     * page, redirecting to /server/login wouldn't work, as we're in that same page.
+     * /server/add doesn't depend on the state of localStorage, so it's always safe to
+     * redirect there and leave the middleware take care of the final destination
+     * (when servers are already available, for example)
+     */
+    void router.replace('/server/add');
+  } else if (
+    !remote.auth.currentUser
+    && remote.auth.servers.length > 0
+    && remote.auth.currentServer
+  ) {
+    void (remote.auth.currentServer.StartupWizardCompleted ? router.replace('/server/login') : router.replace('/wizard'));
+  } else if (
+    !remote.auth.currentUser
+    && remote.auth.servers.length > 0
+    && !remote.auth.currentServer
+  ) {
+    void router.replace('/server/select');
   }
+}
 );
diff --git a/frontend/src/store/index.ts b/frontend/src/store/index.ts
index 4f3cd68f599..04548eb0928 100644
--- a/frontend/src/store/index.ts
+++ b/frontend/src/store/index.ts
@@ -75,9 +75,16 @@ export const isSlow = useMediaQuery('(update:slow)');
  */
 const network = useNetwork();
 export const isConnectedToServer = computedAsync(async () => {
-  if (network.isSupported.value && network.isOnline.value) {
-    return true;
-  } else if (!isNil(remote.auth.currentServer) || !remote.socket.isConnected.value) {
+  /**
+   * These can't be merged inside the if statement as they must be picked up by watchEffect, and the OR operation
+   * stops evaluating in the first await tick as soon as the first truthy value is found.
+   *
+   * See https://vuejs.org/guide/essentials/watchers.html#watcheffect
+   */
+  const socket = remote.socket.isConnected.value;
+  const networkAPI = network.isOnline.value;
+
+  if (!isNil(remote.auth.currentServer) || !socket || !networkAPI) {
     try {
       await remote.sdk.newUserApi(getSystemApi).getPingSystem();
 

From dadce95d5862b2609beaeaddd96c62518f710885 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fernando=20Fern=C3=A1ndez?= <ferferga@hotmail.com>
Date: Tue, 14 May 2024 18:47:08 +0200
Subject: [PATCH 2/2] fix: items changing on navigation, library page
 reactivity
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Fernando Fernández <ferferga@hotmail.com>
---
 frontend/src/App.vue                    |  2 +-
 frontend/src/composables/apis.ts        | 53 +++++++++++++++----------
 frontend/src/pages/item/[itemId].vue    | 16 +++-----
 frontend/src/pages/library/[itemId].vue | 28 +++++--------
 4 files changed, 50 insertions(+), 49 deletions(-)

diff --git a/frontend/src/App.vue b/frontend/src/App.vue
index eddfb247f6c..89b46f26486 100644
--- a/frontend/src/App.vue
+++ b/frontend/src/App.vue
@@ -15,7 +15,7 @@
                 :mode="defaultTransitionMode ?? route.meta.layout.transition.mode">
                 <Suspense suspensible>
                   <JView
-                    :key="route.path"
+                    :key="route.name"
                     :comp="Component" />
                 </Suspense>
               </JTransition>
diff --git a/frontend/src/composables/apis.ts b/frontend/src/composables/apis.ts
index 16b0bc1770c..809663c6c7a 100644
--- a/frontend/src/composables/apis.ts
+++ b/frontend/src/composables/apis.ts
@@ -2,7 +2,7 @@ import type { Api } from '@jellyfin/sdk';
 import type { BaseItemDto, BaseItemDtoQueryResult } from '@jellyfin/sdk/lib/generated-client';
 import type { AxiosResponse } from 'axios';
 import { deepEqual } from 'fast-equals';
-import { computed, getCurrentScope, isRef, shallowRef, toValue, unref, watch, type ComputedRef, type Ref } from 'vue';
+import { computed, effectScope, getCurrentScope, isRef, shallowRef, toValue, unref, watch, type ComputedRef, type Ref } from 'vue';
 import { until } from '@vueuse/core';
 import { useLoading } from '@/composables/use-loading';
 import { useSnackbar } from '@/composables/use-snackbar';
@@ -11,6 +11,7 @@ import { remote } from '@/plugins/remote';
 import { isConnectedToServer } from '@/store';
 import { apiStore } from '@/store/api';
 import { isArray, isNil } from '@/utils/validation';
+import { router } from '@/plugins/router';
 
 /* eslint-disable @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-return */
 type OmittedKeys = 'fields' | 'userId' | 'enableImages' | 'enableTotalRecordCount' | 'enableImageTypes';
@@ -191,6 +192,7 @@ function _sharedInternalLogic<T extends Record<K, (...args: any[]) => any>, K ex
   const stringArgs = computed(() => {
     return JSON.stringify(argsRef.value);
   });
+
   /**
    * TODO: Check why previous returns unknown by default without the type annotation
    */
@@ -275,29 +277,38 @@ function _sharedInternalLogic<T extends Record<K, (...args: any[]) => any>, K ex
     argsRef.value = normalizeArgs();
 
     if (getCurrentScope() !== undefined) {
-      if (args.length) {
-        watch(args, async (_newVal, oldVal) => {
-          const normalizedArgs = normalizeArgs();
-
-          /**
-           * Does a deep comparison to avoid useless double requests
-           */
-          if (!normalizedArgs.every((a, index) => deepEqual(a, toValue(oldVal[index])))) {
-            argsRef.value = normalizedArgs;
-            await runNormally();
-          }
-        });
-      }
+      const handleArgsChange = async (_: typeof args, old: typeof args | undefined): Promise<void> => {
+        const normalizedArgs = normalizeArgs();
+
+        /**
+         * Does a deep comparison to avoid useless double requests
+         */
+        if (old && !normalizedArgs.every((a, index) => deepEqual(a, toValue(old[index])))) {
+          argsRef.value = normalizedArgs;
+          await runNormally();
+        }
+      };
+      const scope = effectScope();
 
-      watch(isConnectedToServer, runWithRetry);
+      scope.run(() => {
+        if (args.length) {
+          watch(args, handleArgsChange);
+        }
 
-      if (isRef(api)) {
-        watch(api, runNormally);
-      }
+        watch(isConnectedToServer, runWithRetry);
 
-      if (isRef(methodName)) {
-        watch(methodName, runNormally);
-      }
+        if (isRef(api)) {
+          watch(api, runNormally);
+        }
+
+        if (isRef(methodName)) {
+          watch(methodName, runNormally);
+        }
+      });
+
+      watch(() => router.currentRoute.value.name, () => scope.stop(),
+        { once: true, flush: 'sync' }
+      );
     }
 
     /**
diff --git a/frontend/src/pages/item/[itemId].vue b/frontend/src/pages/item/[itemId].vue
index 5e48c987e26..80527ae637c 100644
--- a/frontend/src/pages/item/[itemId].vue
+++ b/frontend/src/pages/item/[itemId].vue
@@ -321,16 +321,12 @@ const { data: relatedItems } = await useBaseItem(getLibraryApi, 'getSimilarItems
   itemId: route.params.itemId,
   limit: 12
 }));
-const { data: currentSeries } = await useBaseItem(getUserLibraryApi, 'getItem')(
-  () => ({
-    itemId: item.value.SeriesId ?? ''
-  })
-);
-const { data: childItems } = await useBaseItem(getItemsApi, 'getItems')(
-  () => ({
-    parentId: item.value.Id
-  })
-);
+const { data: currentSeries } = await useBaseItem(getUserLibraryApi, 'getItem')(() => ({
+  itemId: item.value.SeriesId ?? ''
+}));
+const { data: childItems } = await useBaseItem(getItemsApi, 'getItems')(() => ({
+  parentId: item.value.Id
+}));
 
 const selectedSource = ref<MediaSourceInfo>();
 const currentVideoTrack = ref<number>();
diff --git a/frontend/src/pages/library/[itemId].vue b/frontend/src/pages/library/[itemId].vue
index e9d356643bc..e0ba58dc611 100644
--- a/frontend/src/pages/library/[itemId].vue
+++ b/frontend/src/pages/library/[itemId].vue
@@ -9,11 +9,16 @@
       <VChip
         size="small"
         class="ma-2 hidden-sm-and-down">
-        <template v-if="!fullQueryIsCached">
+        <template v-if="loading && items.length === lazyLoadLimit && initialId === route.params.itemId">
           {{ t('lazyLoading', { value: items.length }) }}
         </template>
+        <VProgressCircular
+          v-else-if="loading"
+          indeterminate
+          width="2"
+          size="16" />
         <template v-else>
-          {{ items?.length ?? 0 }}
+          {{ items.length ?? 0 }}
         </template>
       </VChip>
       <VDivider
@@ -63,7 +68,7 @@
 
 <script setup lang="ts">
 import {
-  BaseItemKind, SortOrder, type BaseItemDto
+  BaseItemKind, SortOrder
 } from '@jellyfin/sdk/lib/generated-client';
 import { getArtistsApi } from '@jellyfin/sdk/lib/utils/api/artists-api';
 import { getGenresApi } from '@jellyfin/sdk/lib/utils/api/genres-api';
@@ -74,7 +79,6 @@ import { getStudiosApi } from '@jellyfin/sdk/lib/utils/api/studios-api';
 import { computed, onBeforeMount, ref, shallowRef } from 'vue';
 import { useI18n } from 'vue-i18n';
 import { useRoute } from 'vue-router';
-import { apiStore } from '@/store/api';
 import { methodsAsObject, useBaseItem } from '@/composables/apis';
 import type { Filters } from '@/components/Buttons/FilterButton.vue';
 import { useItemPageTitle } from '@/composables/page-title';
@@ -83,6 +87,7 @@ const { t } = useI18n();
 const route = useRoute('/library/[itemId]');
 
 const lazyLoadLimit = 50;
+const initialId = route.params.itemId;
 const COLLECTION_TYPES_MAPPINGS: Record<string, BaseItemKind> = {
   tvshows: BaseItemKind.Series,
   movies: BaseItemKind.Movie,
@@ -95,7 +100,6 @@ const innerItemKind = shallowRef<BaseItemKind>();
 const sortBy = shallowRef<string>();
 const sortAscending = shallowRef(true);
 const queryLimit = shallowRef<number | undefined>(lazyLoadLimit);
-const lazyLoadIds = shallowRef<BaseItemDto['Id'][]>([]);
 const filters = ref<Filters>({
   status: [],
   features: [],
@@ -158,7 +162,6 @@ const isSortable = computed(
       'Person',
       'Genre',
       'MusicGenre',
-      'MusicGenre',
       'Studio'
     ].includes(viewType.value)
 );
@@ -201,7 +204,7 @@ const method = computed(() => methods.value.methodName);
 /**
  * TODO: Improve the type situation of this statement
  */
-const { loading, data: queryItems } = await useBaseItem(api, method)(() => ({
+const { loading, data: items } = await useBaseItem(api, method)(() => ({
   parentId: parentId.value,
   personTypes: viewType.value === 'Person' ? ['Actor'] : undefined,
   includeItemTypes: viewType.value ? [viewType.value] : undefined,
@@ -220,24 +223,15 @@ const { loading, data: queryItems } = await useBaseItem(api, method)(() => ({
   isHd: filters.value.types.includes('isHD') || undefined,
   is4K: filters.value.types.includes('is4K') || undefined,
   is3D: filters.value.types.includes('is3D') || undefined,
-  startIndex: queryLimit.value ? undefined : lazyLoadLimit,
   limit: queryLimit.value
 }));
 
-/**
- * The queryItems for the 2nd request will return the items from (lazyloadLimit, n],
- * so checking if just the first matches is a good solution
- */
-const fullQueryIsCached = computed(() => loading.value ? !queryLimit.value && queryItems.value[0].Id !== lazyLoadIds.value[0] : true);
-const items = computed(() => fullQueryIsCached.value ? [...(apiStore.getItemsById(lazyLoadIds.value) as BaseItemDto[]), ...queryItems.value] : queryItems.value);
-
 useItemPageTitle(library);
 
 /**
- * We fetch the 1st 100 items and, after mount, we fetch the rest.
+ * We fetch the 1st 50 items and, after mount, we fetch the rest.
  */
 onBeforeMount(() => {
-  lazyLoadIds.value = queryItems.value.map(i => i.Id);
   queryLimit.value = undefined;
 });
 </script>