From f46af67b2ccd15c717b5da26548e4e7b3aaf6999 Mon Sep 17 00:00:00 2001 From: haitaoo Date: Sat, 29 Apr 2023 12:37:30 +0800 Subject: [PATCH] fix(setupApp): Update setupApp logic to prevent invalid Promise.allSettled from being triggered --- ui/src/router/RouteGuard.tsx | 12 +++------ ui/src/router/routes.ts | 2 +- ui/src/utils/floppyNavigation.ts | 3 +++ ui/src/utils/guard.ts | 46 +++++++++++++++++--------------- ui/src/utils/request.ts | 27 ++++++++++++------- 5 files changed, 50 insertions(+), 40 deletions(-) diff --git a/ui/src/router/RouteGuard.tsx b/ui/src/router/RouteGuard.tsx index 6ea39c61..4765499c 100644 --- a/ui/src/router/RouteGuard.tsx +++ b/ui/src/router/RouteGuard.tsx @@ -1,5 +1,5 @@ import { FC, ReactNode, useEffect, useState } from 'react'; -import { useLocation, useNavigate, useLoaderData } from 'react-router-dom'; +import { useNavigate, useLoaderData } from 'react-router-dom'; import { floppyNavigation } from '@/utils'; import { TGuardFunc, TGuardResult } from '@/utils/guard'; @@ -13,7 +13,6 @@ const RouteGuard: FC<{ page?: string; }> = ({ children, onEnter, path, page }) => { const navigate = useNavigate(); - const location = useLocation(); const loaderData = useLoaderData(); const [gk, setKeeper] = useState({ ok: true, @@ -23,6 +22,7 @@ const RouteGuard: FC<{ if (typeof onEnter !== 'function') { return; } + const gr = onEnter({ loaderData, path, @@ -48,12 +48,10 @@ const RouteGuard: FC<{ }; useEffect(() => { /** - * NOTICE: - * Must be put in `useEffect`, - * otherwise `guard` may not get `loggedUserInfo` correctly + * By detecting changes to location.href, many unnecessary tests can be avoided */ applyGuard(); - }, [location]); + }, [window.location.href]); let asOK = gk.ok; if (gk.ok === false && gk.redirect) { @@ -62,10 +60,8 @@ const RouteGuard: FC<{ * but the current page is already the target page for the route guard jump * This should render `children`! */ - asOK = floppyNavigation.equalToCurrentHref(gk.redirect); } - return ( <> {asOK ? children : null} diff --git a/ui/src/router/routes.ts b/ui/src/router/routes.ts index e948fcde..00ea0aab 100644 --- a/ui/src/router/routes.ts +++ b/ui/src/router/routes.ts @@ -269,7 +269,7 @@ const routes: RouteNode[] = [ path: 'admin', page: 'pages/Admin', loader: async () => { - await guard.pullLoggedUser(true); + await guard.pullLoggedUser(); return null; }, guard: () => { diff --git a/ui/src/utils/floppyNavigation.ts b/ui/src/utils/floppyNavigation.ts index 9ba75426..86575eb4 100644 --- a/ui/src/utils/floppyNavigation.ts +++ b/ui/src/utils/floppyNavigation.ts @@ -71,6 +71,9 @@ const navigate = (to: string | number, config: NavigateConfig = {}) => { if (!isRoutableLink(to) && handler !== 'href' && handler !== 'replace') { handler = 'href'; } + if (handler === 'href' && config.options?.replace) { + handler = 'replace'; + } if (handler === 'href') { window.location.href = to; } else if (handler === 'replace') { diff --git a/ui/src/utils/guard.ts b/ui/src/utils/guard.ts index cff941d8..c9c34a2d 100644 --- a/ui/src/utils/guard.ts +++ b/ui/src/utils/guard.ts @@ -20,7 +20,7 @@ import Storage from '@/utils/storage'; import { setupAppLanguage, setupAppTimeZone } from './localize'; import { floppyNavigation, NavigateConfig } from './floppyNavigation'; -import { pullUcAgent, getLoginUrl, getSignUpUrl } from './userCenter'; +import { pullUcAgent, getSignUpUrl } from './userCenter'; type TLoginState = { isLogged: boolean; @@ -105,15 +105,18 @@ export const isIgnoredPath = (ignoredPath: string | string[]) => { return !!matchingPath; }; -let pluLock = false; let pluTimestamp = 0; -export const pullLoggedUser = async (forceRePull = false) => { - // only pull once if not force re-pull - if (pluLock && !forceRePull) { - return; - } - // dedupe pull requests in this time span in 10 seconds - if (Date.now() - pluTimestamp < 1000 * 10) { +export const pullLoggedUser = async (isInitPull = false) => { + /** + * WARN: + * - dedupe pull requests in this time span in 10 seconds + * - isInitPull: + * Requests sent by the initialisation method cannot be throttled + * and may cause Promise.allSettled to complete early in React development mode, + * resulting in inaccurate application data. + */ + // + if (!isInitPull && Date.now() - pluTimestamp < 1000 * 10) { return; } pluTimestamp = Date.now(); @@ -125,7 +128,6 @@ export const pullLoggedUser = async (forceRePull = false) => { console.error(ex); }); if (loggedUserInfo) { - pluLock = true; loggedUserInfoStore.getState().update(loggedUserInfo); } }; @@ -241,16 +243,6 @@ export const allowNewRegistration = () => { return gr; }; -export const loginAgent = () => { - const gr: TGuardResult = { ok: true }; - const loginUrl = getLoginUrl(); - if (loginUrl !== RouteAlias.login) { - gr.ok = false; - gr.redirect = loginUrl; - } - return gr; -}; - export const singUpAgent = () => { const gr: TGuardResult = { ok: true }; const signUpUrl = getSignUpUrl(); @@ -367,7 +359,6 @@ export const handleLoginWithToken = ( /** * Initialize app configuration */ -let appInitialized = false; export const initAppSettingsStore = async () => { const appSettings = await getAppSettings(); if (appSettings) { @@ -389,7 +380,13 @@ export const initAppSettingsStore = async () => { } }; +let appInitialized = false; export const setupApp = async () => { + /** + * This cannot be removed: + * clicking on the current navigation link will trigger a call to the routing loader, + * even though the page is not refreshed. + */ if (appInitialized) { return; } @@ -398,9 +395,14 @@ export const setupApp = async () => { * 1. must pre init logged user info for router guard * 2. must pre init app settings for app render */ - await Promise.allSettled([pullLoggedUser(), initAppSettingsStore()]); + await Promise.allSettled([initAppSettingsStore(), pullLoggedUser(true)]); await Promise.allSettled([pullUcAgent()]); setupAppLanguage(); setupAppTimeZone(); + /** + * WARN: + * Initialization must be completed after all initialization actions, + * otherwise the problem of rendering twice in React development mode can lead to inaccurate data or flickering pages + */ appInitialized = true; }; diff --git a/ui/src/utils/request.ts b/ui/src/utils/request.ts index 21fb7254..6a6ae194 100644 --- a/ui/src/utils/request.ts +++ b/ui/src/utils/request.ts @@ -52,7 +52,6 @@ class Request { // no content return true; } - return data; }, (error) => { @@ -62,9 +61,22 @@ class Request { config: errConfig, } = error.response || {}; const { data = {}, msg = '' } = errModel || {}; + const errorObject: { + code: any; + msg: string; + data: any; + // Currently only used for form errors + isError?: boolean; + // Currently only used for form errors + list?: any[]; + } = { + code: status, + msg, + data, + }; if (status === 400) { if (data?.err_type && errConfig?.passingError) { - return errModel; + return Promise.reject(errorObject); } if (data?.err_type) { if (data.err_type === 'toast') { @@ -94,12 +106,9 @@ class Request { if (data instanceof Array && data.length > 0) { // handle form error - return Promise.reject({ - code: status, - msg, - isError: true, - list: data, - }); + errorObject.isError = true; + errorObject.list = data; + return Promise.reject(errorObject); } if (!data || Object.keys(data).length <= 0) { @@ -177,7 +186,7 @@ class Request { `Request failed with status code ${status}, ${msg || ''}`, ); } - return Promise.reject(false); + return Promise.reject(errorObject); }, ); }