From 3017ebfd4870a63ca1c090efa421787b0a05f59e Mon Sep 17 00:00:00 2001 From: Joe Karow <58997957+JoeKarow@users.noreply.github.com> Date: Tue, 9 Apr 2024 17:22:14 -0400 Subject: [PATCH] fix excess rerendering --- apps/app/next.config.mjs | 2 - .../pages/org/[slug]/[orgLocationId]/edit.tsx | 84 ++++++++++-------- packages/ui/hooks/useGoogleMapMarker.tsx | 57 +++++++++--- packages/ui/hooks/useGoogleMaps.ts | 2 +- packages/ui/providers/GoogleMaps.tsx | 88 ++++++++++++++----- packages/ui/quick-lint-js.config | 7 ++ 6 files changed, 163 insertions(+), 77 deletions(-) create mode 100644 packages/ui/quick-lint-js.config diff --git a/apps/app/next.config.mjs b/apps/app/next.config.mjs index 1065ba6dfd..3d621882c0 100644 --- a/apps/app/next.config.mjs +++ b/apps/app/next.config.mjs @@ -106,8 +106,6 @@ const nextConfig = { ) } - config.devtool = 'eval-source-map' - if (!isLocalDev) { config.plugins.push( new webpack.DefinePlugin({ diff --git a/apps/app/src/pages/org/[slug]/[orgLocationId]/edit.tsx b/apps/app/src/pages/org/[slug]/[orgLocationId]/edit.tsx index 55e0d4e6fc..166ef3c4c6 100644 --- a/apps/app/src/pages/org/[slug]/[orgLocationId]/edit.tsx +++ b/apps/app/src/pages/org/[slug]/[orgLocationId]/edit.tsx @@ -5,7 +5,7 @@ import Head from 'next/head' import { useRouter } from 'next/router' import { useTranslation } from 'next-i18next' import { type GetServerSidePropsContext } from 'nextjs-routes' -import { useEffect, useRef, useState } from 'react' +import { useCallback, useEffect, useRef, useState } from 'react' import { FormProvider, useForm } from 'react-hook-form' import { z } from 'zod' @@ -41,7 +41,7 @@ const formSchema = z.object({ }) type FormSchema = z.infer const OrgLocationPage: NextPage> = ({ - organizationId, + organizationId: _organizationId, }) => { const apiUtils = api.useUtils() const { t } = useTranslation() @@ -58,13 +58,16 @@ const OrgLocationPage: NextPage data.map(({ id, defaultText }) => ({ value: id, label: defaultText })), - refetchOnWindowFocus: false, - } - ) + + // for use with MultiSelectPopover + + // const { data: orgServices } = api.service.getNames.useQuery( + // { organizationId }, + // { + // select: (returnedData) => returnedData.map(({ id, defaultText }) => ({ value: id, label: defaultText })), + // refetchOnWindowFocus: false, + // } + // ) const defaultFormValues = data ? { id: data.id, name: data.name, services: data.services.map(({ serviceId }) => serviceId) } : undefined @@ -101,9 +104,32 @@ const OrgLocationPage: NextPage { - if (data && status === 'success') setLoading(false) + if (data && status === 'success') { + setLoading(false) + } }, [data, status]) - if (loading || !data) return + + const tabHandler = useCallback((tab: string) => { + setActiveTab(tab) + switch (tab) { + case 'services': { + servicesRef.current?.scrollIntoView({ behavior: 'smooth' }) + break + } + case 'photos': { + photosRef.current?.scrollIntoView({ behavior: 'smooth' }) + break + } + case 'reviews': { + reviewsRef.current?.scrollIntoView({ behavior: 'smooth' }) + break + } + } + }, []) + + if (loading || !data) { + return + } const { // emails, @@ -130,7 +156,7 @@ const OrgLocationPage: NextPage { + onClick: () => { router.push({ pathname: '/org/[slug]/edit', query: { slug: data.organization.slug }, @@ -153,39 +179,19 @@ const OrgLocationPage: NextPage - { - setActiveTab(tab) - switch (tab) { - case 'services': { - servicesRef.current?.scrollIntoView({ behavior: 'smooth' }) - break - } - case 'photos': { - photosRef.current?.scrollIntoView({ behavior: 'smooth' }) - break - } - case 'reviews': { - reviewsRef.current?.scrollIntoView({ behavior: 'smooth' }) - break - } - } - }} - > + {t('services')} {t('photo', { count: 2 })} @@ -236,7 +242,9 @@ export const getServerSideProps = async ({ res, }: GetServerSidePropsContext<'/org/[slug]/[orgLocationId]/edit'>) => { const urlParams = z.object({ slug: z.string(), orgLocationId: prefixedId('orgLocation') }).safeParse(params) - if (!urlParams.success) return { notFound: true } + if (!urlParams.success) { + return { notFound: true } + } const { slug, orgLocationId: id } = urlParams.data const session = await checkServerPermissions({ ctx: { req, res }, diff --git a/packages/ui/hooks/useGoogleMapMarker.tsx b/packages/ui/hooks/useGoogleMapMarker.tsx index c9a4e9c456..144bc7137e 100644 --- a/packages/ui/hooks/useGoogleMapMarker.tsx +++ b/packages/ui/hooks/useGoogleMapMarker.tsx @@ -1,6 +1,8 @@ import { MantineProvider, Stack, Text, Title } from '@mantine/core' +import { useWhyDidYouUpdate } from 'ahooks' import { useRouter } from 'next/router' import { type Route } from 'nextjs-routes' +import { useCallback, useMemo } from 'react' import { createRoot } from 'react-dom/client' import { Link } from '~ui/components/core/Link' @@ -22,7 +24,7 @@ const getHref: GetHref = ({ slug, locationId }) => { } type SlugOnly = { slug: string - locationId?: undefined + locationId?: never } type LocationAndSlug = { slug: string @@ -33,15 +35,25 @@ export const useGoogleMapMarker = () => { const router = useRouter() const variant = useCustomVariant() const { mapIsReady, infoWindow, marker, map } = useGoogleMaps() - return { - get(id: string) { - return marker.get(id) - }, - add({ id, lat, lng, name, address, slug, locationId }: AddMarkerParams) { - if (!mapIsReady) throw new Error('map is not ready') + const markerInfoBoxLinkClickHandler = useCallback( + ({ slug, locationId }: SlugOnly | LocationAndSlug) => + () => + router.push(getHref({ slug, locationId })), + [router] + ) + + // eslint-disable-next-line react-hooks/exhaustive-deps + const getMarker = useCallback((id: string) => marker.get(id), []) + const addMarker = useCallback( + ({ id, lat, lng, name, address, slug, locationId }: AddMarkerParams) => { + if (!mapIsReady) { + throw new Error('map is not ready') + } + console.trace('adding new marker', { id, lat, lng, name, address, slug, locationId }) const position = new google.maps.LatLng({ lat, lng }) const newMarker = marker.get(id) ?? new google.maps.marker.AdvancedMarkerElement() + // console.trace('adding new marker', name, position.toString()) newMarker.map = map newMarker.position = position @@ -55,7 +67,7 @@ export const useGoogleMapMarker = () => { {slug ? ( router.push(getHref({ slug, locationId }))} + onClick={markerInfoBoxLinkClickHandler({ slug, locationId })} > {name} @@ -75,17 +87,38 @@ export const useGoogleMapMarker = () => { marker.set(id, newMarker) return newMarker }, - remove(markerId: string) { + // eslint-disable-next-line react-hooks/exhaustive-deps + [infoWindow, map, mapIsReady, markerInfoBoxLinkClickHandler] + ) + + const removeMarker = useCallback( + (markerId: string) => { const markerItem = marker.get(markerId) - if (!markerItem) return false + if (!markerItem) { + return false + } markerItem.map = null google.maps.event.clearInstanceListeners(markerItem) marker.remove(markerId) return true }, - } -} + // eslint-disable-next-line react-hooks/exhaustive-deps + [] + ) + const markerFns = useMemo( + () => ({ get: getMarker, add: addMarker, remove: removeMarker }), + [addMarker, getMarker, removeMarker] + ) + useWhyDidYouUpdate('useGoogleMapMarker', { + router, + infoWindow, + map, + mapIsReady, + clickHandler: markerInfoBoxLinkClickHandler, + }) + return markerFns +} interface AddMarkerParams { id: string lat: number diff --git a/packages/ui/hooks/useGoogleMaps.ts b/packages/ui/hooks/useGoogleMaps.ts index 137e9d40b9..0d665424a9 100644 --- a/packages/ui/hooks/useGoogleMaps.ts +++ b/packages/ui/hooks/useGoogleMaps.ts @@ -7,7 +7,7 @@ export const useGoogleMaps = (): UseGoogleMapsReturn => { if (!context) { throw new Error('useGoogleMaps must be used within a GoogleMapsProvider') } - if (context.map && context.isReady && context.infoWindow) { + if (context.isReady) { return { map: context.map, infoWindow: context.infoWindow, diff --git a/packages/ui/providers/GoogleMaps.tsx b/packages/ui/providers/GoogleMaps.tsx index 85acef0e9d..4b417a2df2 100644 --- a/packages/ui/providers/GoogleMaps.tsx +++ b/packages/ui/providers/GoogleMaps.tsx @@ -2,7 +2,7 @@ import { useEventEmitter, useMap } from 'ahooks' import { type EventEmitter } from 'ahooks/lib/useEventEmitter' import { createContext, type ReactNode, useEffect, useMemo, useRef, useState } from 'react' -export const GoogleMapContext = createContext(null) +export const GoogleMapContext = createContext(null) GoogleMapContext.displayName = 'GoogleMapContext' export const GoogleMapsProvider = ({ children }: { children: ReactNode }) => { @@ -10,11 +10,14 @@ export const GoogleMapsProvider = ({ children }: { children: ReactNode }) => { const [infoWindow, setInfoWindow] = useState() const [isReady, setIsReady] = useState(false) const [markers, marker] = useMap() - - const mapEvents = { - ready: useEventEmitter(), - initialPropsSet: useEventEmitter(), - } + const eventEmitter = useEventEmitter() + const mapEvents = useMemo( + () => ({ + ready: eventEmitter, + initialPropsSet: eventEmitter, + }), + [eventEmitter] + ) const mapEventRef = useRef(mapEvents) const initialCamera = useMemo(() => { return { @@ -32,33 +35,57 @@ export const GoogleMapsProvider = ({ children }: { children: ReactNode }) => { useEffect(() => { if (map && infoWindow) { const infoListener = google.maps.event.addListener(infoWindow, 'closeclick', () => { - if (cameraRef.current.center) map.panTo(cameraRef.current.center) + if (cameraRef.current.center) { + map.panTo(cameraRef.current.center) + } }) const mapListener = google.maps.event.addListener(map, 'click', () => { infoWindow.close() - if (cameraRef.current.center) map.panTo(cameraRef.current.center) + if (cameraRef.current.center) { + map.panTo(cameraRef.current.center) + } }) - markers.forEach((marker) => (marker.map = map)) + markers.forEach((singleMarker) => (singleMarker.map = map)) return () => { google.maps.event.removeListener(infoListener) google.maps.event.removeListener(mapListener) } } + return () => void 0 // eslint-disable-next-line react-hooks/exhaustive-deps }, [map, infoWindow]) - const contextValue = { - map, - infoWindow, - setMap, - setInfoWindow, - isReady, - mapEvents, - camera: cameraRef.current, - marker, - markers, - } + const mapIsReady = typeof map !== 'undefined' && typeof infoWindow !== 'undefined' + + const contextValue: ContextValue = useMemo( + () => + mapIsReady + ? { + map, + infoWindow, + setMap, + setInfoWindow, + mapEvents, + marker, + markers, + isReady: true, + camera: cameraRef.current, + } + : { + setMap, + setInfoWindow, + mapEvents, + marker, + markers, + map: undefined, + infoWindow: undefined, + isReady: false, + camera: cameraRef.current, + }, + // eslint-disable-next-line react-hooks/exhaustive-deps + [isReady] + ) return {children} } @@ -74,14 +101,27 @@ export interface MarkerState { reset: () => void } -interface GoogleMapContextValue { - map: google.maps.Map | undefined - infoWindow: google.maps.InfoWindow | undefined +interface GoogleMapContextBase { setMap: (map: google.maps.Map) => void setInfoWindow: (infoWindow: google.maps.InfoWindow) => void - isReady: boolean mapEvents: MapEvents camera: google.maps.CameraOptions marker: MarkerState markers: Map } + +interface GoogleMapReadyContext extends GoogleMapContextBase { + map: google.maps.Map + infoWindow: google.maps.InfoWindow + isReady: true +} +interface GoogleMapNotReadyContext extends GoogleMapContextBase { + map: undefined + infoWindow: undefined + isReady: false +} + +type GoogleMapContext = GoogleMapReadyContext | GoogleMapNotReadyContext +type ContextValue = TReady extends true + ? GoogleMapReadyContext + : GoogleMapNotReadyContext diff --git a/packages/ui/quick-lint-js.config b/packages/ui/quick-lint-js.config new file mode 100644 index 0000000000..7a77f1245a --- /dev/null +++ b/packages/ui/quick-lint-js.config @@ -0,0 +1,7 @@ +{ + "globals": { + "google": { + "writable": false + } + } +}