From 9c03a9789df5ec8fe04332b369ba051d8af4510a Mon Sep 17 00:00:00 2001 From: Luca Pagliaro Date: Mon, 28 Oct 2024 22:24:00 +0100 Subject: [PATCH 1/4] refactor: BootProvider and log events --- .../extension/src/companion/Companion.tsx | 10 +- packages/extension/src/newtab/App.tsx | 8 +- packages/shared/src/contexts/BootProvider.tsx | 22 ++--- .../shared/src/hooks/log/useLogContextData.ts | 65 +++++++------ .../shared/src/hooks/log/useLogPageView.ts | 33 +++---- packages/shared/src/hooks/log/useLogQueue.ts | 97 ++++++++++--------- 6 files changed, 121 insertions(+), 114 deletions(-) diff --git a/packages/extension/src/companion/Companion.tsx b/packages/extension/src/companion/Companion.tsx index 14898502a2..7ef39a6a0c 100644 --- a/packages/extension/src/companion/Companion.tsx +++ b/packages/extension/src/companion/Companion.tsx @@ -110,13 +110,11 @@ export default function Companion({ }), }); const [assetsLoadedDebounce] = useDebounceFn(() => setAssetsLoaded(true), 10); - const routeChangedCallbackRef = useLogPageView(); + const routeChangedCallback = useLogPageView(); useEffect(() => { - if (routeChangedCallbackRef.current) { - routeChangedCallbackRef.current(); - } - }, [routeChangedCallbackRef]); + routeChangedCallback?.(); + }, [routeChangedCallback]); const [checkAssets, clearCheckAssets] = useDebounceFn(() => { if (containerRef?.current?.offsetLeft === 0) { @@ -133,7 +131,7 @@ export default function Companion({ } checkAssets(); - routeChangedCallbackRef.current(); + routeChangedCallback?.(); // @NOTE see https://dailydotdev.atlassian.net/l/cp/dK9h1zoM // eslint-disable-next-line react-hooks/exhaustive-deps }, [containerRef]); diff --git a/packages/extension/src/newtab/App.tsx b/packages/extension/src/newtab/App.tsx index fd46eb39ed..2d7437c3bc 100644 --- a/packages/extension/src/newtab/App.tsx +++ b/packages/extension/src/newtab/App.tsx @@ -108,7 +108,7 @@ function InternalApp(): ReactElement { const { contentScriptGranted } = useContentScriptStatus(); const { hostGranted, isFetching: isCheckingHostPermissions } = useHostStatus(); - const routeChangedCallbackRef = useLogPageView(); + const routeChangedCallback = useLogPageView(); useConsoleLogo(); const { value: extensionOverlay } = useConditionalFeature({ @@ -122,10 +122,10 @@ function InternalApp(): ReactElement { const shouldRedirectOnboarding = !user && isPageReady && !isTesting; useEffect(() => { - if (routeChangedCallbackRef.current && isPageReady) { - routeChangedCallbackRef.current(); + if (isPageReady && currentPage) { + routeChangedCallback?.(); } - }, [isPageReady, routeChangedCallbackRef, currentPage]); + }, [isPageReady, routeChangedCallback, currentPage]); const { dismissToast } = useToastNotification(); diff --git a/packages/shared/src/contexts/BootProvider.tsx b/packages/shared/src/contexts/BootProvider.tsx index 02e9a702be..8b54203c02 100644 --- a/packages/shared/src/contexts/BootProvider.tsx +++ b/packages/shared/src/contexts/BootProvider.tsx @@ -84,12 +84,8 @@ const updateLocalBootData = ( return result; }; -const getCachedOrNull = () => { - try { - return JSON.parse(storage.getItem(BOOT_LOCAL_KEY)); - } catch (err) { - return null; - } +const getCachedBootOrNull = () => { + return JSON.parse(storage.getItem(BOOT_LOCAL_KEY) ?? 'null'); }; export type PreloadFeeds = ({ @@ -153,7 +149,6 @@ export const BootDataProvider = ({ const logged = initialData?.user as LoggedUser; const shouldRefetch = !!logged?.providers && !!logged?.id; const lastAppliedChangeRef = useRef>(); - const isInitialFetch = useRef(); const { data: bootData, @@ -168,8 +163,6 @@ export const BootDataProvider = ({ const result = await getBootData(app); preloadFeedsRef.current({ feeds: result.feeds, user: result.user }); updateLocalBootData(bootData || {}, result); - isInitialFetch.current = isInitialFetch.current === undefined; - return result; }, refetchOnWindowFocus: shouldRefetch, @@ -178,6 +171,7 @@ export const BootDataProvider = ({ placeholderData: initialData, }); + const isInitialFetch = !isFetched; const isBootReady = isFetched && !isError; const loadedFromCache = !!bootData; const { user, settings, alerts, notifications, squads } = bootData || {}; @@ -186,7 +180,7 @@ export const BootDataProvider = ({ const updatedAtActive = user ? dataUpdatedAt : null; const updateQueryCache = useCallback( (updatedBootData: Partial, update = true) => { - const cachedData = getCachedOrNull() || {}; + const cachedData = getCachedBootOrNull() ?? {}; const lastAppliedChange = lastAppliedChangeRef.current; let updatedData = { ...updatedBootData }; if (update) { @@ -225,12 +219,14 @@ export const BootDataProvider = ({ ); const updateSettings = useCallback( - (updatedSettings) => updateQueryCache({ settings: updatedSettings }), + (updatedSettings: Boot['settings']) => + updateQueryCache({ settings: updatedSettings }), [updateQueryCache], ); const updateAlerts = useCallback( - (updatedAlerts) => updateQueryCache({ alerts: updatedAlerts }), + (updatedAlerts: Boot['alerts']) => + updateQueryCache({ alerts: updatedAlerts }), [updateQueryCache], ); @@ -274,7 +270,7 @@ export const BootDataProvider = ({ isFetched={isBootReady} isLegacyLogout={bootData?.isLegacyLogout} accessToken={bootData?.accessToken} - isPastRegistration={isInitialFetch.current} + isPastRegistration={isInitialFetch} squads={squads} > >, sendBeacon: () => void, ): LogContextData { - return useMemo( - () => ({ - logEvent(event: LogEvent) { - pushToQueue([generateEvent(event, sharedPropsRef, getPage())]); - }, - logEventStart(id, event) { - if (!durationEventsQueue.current.has(id)) { - durationEventsQueue.current.set( - id, - generateEvent(event, sharedPropsRef, getPage()), - ); - } - }, - logEventEnd(id, now = new Date()) { - const event = durationEventsQueue.current.get(id); - if (event) { - durationEventsQueue.current.delete(id); - event.event_duration = - now.getTime() - event.event_timestamp.getTime(); - if (window.scrollY > 0 && event.event_name !== 'page inactive') { - event.page_state = 'active'; - } - pushToQueue([event]); + const logEvent = useCallback( + (event: LogEvent) => { + pushToQueue([generateEvent(event, sharedPropsRef, getPage())]); + }, + [getPage, pushToQueue, sharedPropsRef], + ); + const logEventStart = useCallback( + (id, event) => { + if (!durationEventsQueue.current.has(id)) { + durationEventsQueue.current.set( + id, + generateEvent(event, sharedPropsRef, getPage()), + ); + } + }, + [durationEventsQueue, getPage, sharedPropsRef], + ); + const logEventEnd = useCallback( + (id, now = new Date()) => { + const event = durationEventsQueue.current.get(id); + if (event) { + durationEventsQueue.current.delete(id); + event.event_duration = now.getTime() - event.event_timestamp.getTime(); + if (window.scrollY > 0 && event.event_name !== 'page inactive') { + event.page_state = 'active'; } - }, - sendBeacon, - }), - [sharedPropsRef, getPage, pushToQueue, durationEventsQueue, sendBeacon], + pushToQueue([event]); + } + }, + [durationEventsQueue, pushToQueue], ); + + return { + logEvent, + logEventStart, + logEventEnd, + sendBeacon, + }; } diff --git a/packages/shared/src/hooks/log/useLogPageView.ts b/packages/shared/src/hooks/log/useLogPageView.ts index 6f9868d471..f8e93ac92f 100644 --- a/packages/shared/src/hooks/log/useLogPageView.ts +++ b/packages/shared/src/hooks/log/useLogPageView.ts @@ -1,32 +1,29 @@ -import { MutableRefObject, useContext, useEffect, useRef } from 'react'; +import { useCallback, useContext, useEffect } from 'react'; import { useRouter } from 'next/router'; import LogContext from '../../contexts/LogContext'; -export default function useLogPageView(): MutableRefObject<() => void> { +export default function useLogPageView(): () => void { const router = useRouter(); const { logEventStart, logEventEnd } = useContext(LogContext); - const routeChangedCallbackRef = useRef<() => void>(); - const lifecycleCallbackRef = useRef<(event: CustomEvent) => void>(); + const routeChangedCallback = useCallback(() => { + logEventEnd('page view'); + logEventStart('page view', { event_name: 'page view' }); + }, [logEventEnd, logEventStart]); - useEffect(() => { - routeChangedCallbackRef.current = () => { - logEventEnd('page view'); - logEventStart('page view', { event_name: 'page view' }); - }; - - lifecycleCallbackRef.current = (event) => { + const lifecycleCallback = useCallback( + (event: CustomEvent) => { if (event.detail.newState === 'active') { logEventStart('page view', { event_name: 'page view' }); } - }; - }, [logEventStart, logEventEnd]); + }, + [logEventStart], + ); useEffect(() => { - const handleRouteChange = () => routeChangedCallbackRef.current(); + const handleRouteChange = () => routeChangedCallback(); router.events.on('routeChangeComplete', handleRouteChange); - const handleLifecycle = (event: CustomEvent) => - lifecycleCallbackRef.current(event); + const handleLifecycle = (event: CustomEvent) => lifecycleCallback(event); window.addEventListener('statechange', handleLifecycle); return () => { @@ -35,7 +32,7 @@ export default function useLogPageView(): MutableRefObject<() => void> { }; // @NOTE see https://dailydotdev.atlassian.net/l/cp/dK9h1zoM // eslint-disable-next-line react-hooks/exhaustive-deps - }, []); + }, [lifecycleCallback, routeChangedCallback]); - return routeChangedCallbackRef; + return routeChangedCallback; } diff --git a/packages/shared/src/hooks/log/useLogQueue.ts b/packages/shared/src/hooks/log/useLogQueue.ts index a43dad4058..17d7320e27 100644 --- a/packages/shared/src/hooks/log/useLogQueue.ts +++ b/packages/shared/src/hooks/log/useLogQueue.ts @@ -1,5 +1,5 @@ import { useMutation } from '@tanstack/react-query'; -import { MutableRefObject, useMemo, useRef } from 'react'; +import { MutableRefObject, useCallback, useRef } from 'react'; import { apiUrl } from '../../lib/config'; import useDebounceFn from '../useDebounceFn'; import { ExtensionMessageType } from '../../lib/extension'; @@ -56,49 +56,56 @@ export default function useLogQueue({ } }, 500); - return useMemo( - () => ({ - pushToQueue: (events) => { - queueRef.current.push(...events); - if (enabledRef.current) { - debouncedSendEvents(); - } - }, - setEnabled: (enabled) => { - enabledRef.current = enabled; - if (enabled && queueRef.current.length) { - debouncedSendEvents(); - } - }, - queueRef, - sendBeacon: () => { - if (queueRef.current.length) { - const events = queueRef.current; - queueRef.current = []; - const blob = new Blob([JSON.stringify({ events })], { - type: 'application/json', - }); - if (backgroundMethod) { - backgroundMethod?.({ - url: LOG_ENDPOINT, - type: ExtensionMessageType.FetchRequest, - args: { - body: JSON.stringify({ events }), - credentials: 'include', - method: 'POST', - headers: { - 'content-type': 'application/json', - }, - }, - }); - } else { - navigator.sendBeacon(LOG_ENDPOINT, blob); - } - } - }, - }), - // @NOTE see https://dailydotdev.atlassian.net/l/cp/dK9h1zoM - // eslint-disable-next-line react-hooks/exhaustive-deps - [queueRef, debouncedSendEvents, enabledRef], + const pushToQueue = useCallback( + (events) => { + queueRef.current.push(...events); + if (enabledRef.current) { + debouncedSendEvents(); + } + }, + [debouncedSendEvents], ); + + const sendBeacon = useCallback(() => { + if (queueRef.current.length) { + const events = queueRef.current; + queueRef.current = []; + const blob = new Blob([JSON.stringify({ events })], { + type: 'application/json', + }); + if (backgroundMethod) { + backgroundMethod?.({ + url: LOG_ENDPOINT, + type: ExtensionMessageType.FetchRequest, + args: { + body: JSON.stringify({ events }), + credentials: 'include', + method: 'POST', + headers: { + 'content-type': 'application/json', + }, + }, + }); + } else { + navigator.sendBeacon(LOG_ENDPOINT, blob); + } + } + }, [backgroundMethod]); + + const setEnabled = useCallback( + (enabled: boolean) => { + enabledRef.current = enabled; + if (enabled && queueRef.current.length) { + debouncedSendEvents(); + } + }, + [debouncedSendEvents], + ); + + return { + pushToQueue, + setEnabled, + queueRef, + sendBeacon, + }; } From 1c3f281bca7d3b5c5c3eacb1fcacfac3e83f2db4 Mon Sep 17 00:00:00 2001 From: Luca Pagliaro Date: Tue, 29 Oct 2024 11:45:48 +0100 Subject: [PATCH 2/4] refactor: revert getCachedBootOrNull --- packages/shared/src/contexts/BootProvider.tsx | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/shared/src/contexts/BootProvider.tsx b/packages/shared/src/contexts/BootProvider.tsx index 8b54203c02..4eec7d8995 100644 --- a/packages/shared/src/contexts/BootProvider.tsx +++ b/packages/shared/src/contexts/BootProvider.tsx @@ -85,7 +85,11 @@ const updateLocalBootData = ( }; const getCachedBootOrNull = () => { - return JSON.parse(storage.getItem(BOOT_LOCAL_KEY) ?? 'null'); + try { + return JSON.parse(storage.getItem(BOOT_LOCAL_KEY)); + } catch (err) { + return null; + } }; export type PreloadFeeds = ({ From 540d67b0b7b5b87268f6f20bc28a39259fc45f41 Mon Sep 17 00:00:00 2001 From: Luca Pagliaro Date: Mon, 4 Nov 2024 18:07:43 +0100 Subject: [PATCH 3/4] fix: ssr shift due to firstLoad --- packages/shared/src/contexts/AuthContext.tsx | 10 +++++----- packages/shared/src/contexts/BootProvider.tsx | 5 ++--- .../shared/src/hooks/referral/useJoinReferral.spec.tsx | 2 +- 3 files changed, 8 insertions(+), 9 deletions(-) diff --git a/packages/shared/src/contexts/AuthContext.tsx b/packages/shared/src/contexts/AuthContext.tsx index 44cf1a356f..641badf673 100644 --- a/packages/shared/src/contexts/AuthContext.tsx +++ b/packages/shared/src/contexts/AuthContext.tsx @@ -17,7 +17,7 @@ import { AccessToken, Boot, Visit } from '../lib/boot'; import { isCompanionActivated } from '../lib/element'; import { AuthTriggers, AuthTriggersType } from '../lib/auth'; import { Squad } from '../graphql/sources'; -import { checkIsExtension, isNullOrUndefined } from '../lib/func'; +import { checkIsExtension } from '../lib/func'; export interface LoginState { trigger: AuthTriggersType; @@ -103,7 +103,7 @@ export type AuthContextProviderProps = { isFetched?: boolean; isLegacyLogout?: boolean; children?: ReactNode; - firstLoad?: boolean; + isAuthReady?: boolean; } & Pick< AuthContextData, | 'getRedirectUri' @@ -131,14 +131,14 @@ export const AuthContextProvider = ({ isLegacyLogout, accessToken, squads, - firstLoad, + isAuthReady, }: AuthContextProviderProps): ReactElement => { const [loginState, setLoginState] = useState(null); const endUser = user && 'providers' in user ? user : null; const referral = user?.referralId || user?.referrer; const referralOrigin = user?.referralOrigin; - if (firstLoad === true && endUser && !endUser?.infoConfirmed) { + if (isAuthReady === true && endUser && !endUser?.infoConfirmed) { logout(LogoutReason.IncomleteOnboarding); } @@ -149,7 +149,7 @@ export const AuthContextProvider = ({ return ( { updateUser={jest.fn()} tokenRefreshed={false} isFetched - firstLoad={false} + isAuthReady={false} > {children} From d5a54d5739783970f62e298140ec900c9838be1d Mon Sep 17 00:00:00 2001 From: Luca Pagliaro Date: Tue, 5 Nov 2024 12:29:19 +0100 Subject: [PATCH 4/4] fix: test with wrong context set --- packages/shared/src/hooks/referral/useJoinReferral.spec.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/shared/src/hooks/referral/useJoinReferral.spec.tsx b/packages/shared/src/hooks/referral/useJoinReferral.spec.tsx index 04dbc27c79..9f9bf55878 100644 --- a/packages/shared/src/hooks/referral/useJoinReferral.spec.tsx +++ b/packages/shared/src/hooks/referral/useJoinReferral.spec.tsx @@ -20,7 +20,7 @@ describe('useJoinReferral hook', () => { updateUser={jest.fn()} tokenRefreshed={false} isFetched - isAuthReady={false} + isAuthReady > {children}