chore(eslint): explicitly disable exhaustive deps on historical code (#…

…6576) I haven't made any attept here to resolve anything here, just to explicitly acknowledge that these are issues that eslint has highlighted and we should ignore, for the sake of easily picking up new issues. If these are truely problematic, then I am assuming they should have been picked up as bugs with the system. This doesn't stop someone from coming in and reviewing the code in this pr retrospectively.
PostHog · Oct 21, 2021 · e4939b7 · e4939b7
1 parent 6160d0c
commit e4939b7
Show file tree

Hide file tree

Showing 48 changed files with 720 additions and 444 deletions.
diff --git a/frontend/src/lib/components/Annotations/AnnotationMarker.tsx b/frontend/src/lib/components/Annotations/AnnotationMarker.tsx
@@ -83,14 +83,18 @@ export function AnnotationMarker({
 
     const visible = focused || (!dynamic && hovered)
 
-    useEffect(() => {
-        if (visible) {
-            reportAnnotationViewed(annotations)
-        } else {
-            reportAnnotationViewed(null)
-            /* We report a null value to cancel (if applicable) the report because the annotation was closed */
-        }
-    }, [visible])
+    useEffect(
+        () => {
+            if (visible) {
+                reportAnnotationViewed(annotations)
+            } else {
+                reportAnnotationViewed(null)
+                /* We report a null value to cancel (if applicable) the report because the annotation was closed */
+            }
+        },
+        // eslint-disable-next-line react-hooks/exhaustive-deps
+        [visible]
+    )
 
     const { user } = useValues(userLogic)
     const { currentTeam } = useValues(teamLogic)
@@ -119,12 +123,16 @@ export function AnnotationMarker({
         closePopup()
     }
 
-    useEffect(() => {
-        document.addEventListener('mousedown', deselect)
-        return () => {
-            document.removeEventListener('mousedown', deselect)
-        }
-    }, [])
+    useEffect(
+        () => {
+            document.addEventListener('mousedown', deselect)
+            return () => {
+                document.removeEventListener('mousedown', deselect)
+            }
+        },
+        // eslint-disable-next-line react-hooks/exhaustive-deps
+        []
+    )
 
     if (
         dynamic &&

diff --git a/frontend/src/lib/components/DateFilter/DateFilterRange.tsx b/frontend/src/lib/components/DateFilter/DateFilterRange.tsx
@@ -46,12 +46,16 @@ export function DateFilterRange(props: {
         }
     }
 
-    useEffect(() => {
-        window.addEventListener('mousedown', onClickOutside)
-        return () => {
-            window.removeEventListener('mousedown', onClickOutside)
-        }
-    }, [calendarOpen])
+    useEffect(
+        () => {
+            window.addEventListener('mousedown', onClickOutside)
+            return () => {
+                window.removeEventListener('mousedown', onClickOutside)
+            }
+        },
+        // eslint-disable-next-line react-hooks/exhaustive-deps
+        [calendarOpen]
+    )
 
     return (
         <div ref={dropdownRef}>

diff --git a/frontend/src/lib/components/FullScreen.tsx b/frontend/src/lib/components/FullScreen.tsx
@@ -2,45 +2,24 @@ import { useEffect } from 'react'
 
 export function FullScreen({ onExit }: { onExit?: () => any }): null {
     const selector = 'aside.ant-layout-sider, .layout-top-content'
-    useEffect(() => {
-        const myClasses = window.document.querySelectorAll(selector) as NodeListOf<HTMLElement>
+    useEffect(
+        () => {
+            const myClasses = window.document.querySelectorAll(selector) as NodeListOf<HTMLElement>
 
-        for (let i = 0; i < myClasses.length; i++) {
-            myClasses[i].style.display = 'none'
-        }
-
-        const handler = (): void => {
-            if (window.document.fullscreenElement === null) {
-                onExit?.()
+            for (let i = 0; i < myClasses.length; i++) {
+                myClasses[i].style.display = 'none'
             }
-        }
-
-        try {
-            window.document.body.requestFullscreen().then(() => {
-                window.addEventListener('fullscreenchange', handler, false)
-            })
-        } catch {
-            // will break on IE11
-        }
-
-        try {
-            window.dispatchEvent(new window.Event('scroll'))
-            window.dispatchEvent(new window.Event('resize'))
-        } catch {
-            // will break on IE11
-        }
-
-        return () => {
-            const elements = window.document.querySelectorAll(selector) as NodeListOf<HTMLElement>
-
-            for (let i = 0; i < elements.length; i++) {
-                elements[i].style.display = 'block'
+
+            const handler = (): void => {
+                if (window.document.fullscreenElement === null) {
+                    onExit?.()
+                }
             }
+
             try {
-                window.removeEventListener('fullscreenchange', handler, false)
-                if (window.document.fullscreenElement !== null) {
-                    window.document.exitFullscreen()
-                }
+                window.document.body.requestFullscreen().then(() => {
+                    window.addEventListener('fullscreenchange', handler, false)
+                })
             } catch {
                 // will break on IE11
             }
@@ -51,8 +30,33 @@ export function FullScreen({ onExit }: { onExit?: () => any }): null {
             } catch {
                 // will break on IE11
             }
-        }
-    }, [])
+
+            return () => {
+                const elements = window.document.querySelectorAll(selector) as NodeListOf<HTMLElement>
+
+                for (let i = 0; i < elements.length; i++) {
+                    elements[i].style.display = 'block'
+                }
+                try {
+                    window.removeEventListener('fullscreenchange', handler, false)
+                    if (window.document.fullscreenElement !== null) {
+                        window.document.exitFullscreen()
+                    }
+                } catch {
+                    // will break on IE11
+                }
+
+                try {
+                    window.dispatchEvent(new window.Event('scroll'))
+                    window.dispatchEvent(new window.Event('resize'))
+                } catch {
+                    // will break on IE11
+                }
+            }
+        },
+        // eslint-disable-next-line react-hooks/exhaustive-deps
+        []
+    )
 
     return null
 }
diff --git a/frontend/src/lib/components/PayCard/PayCard.tsx b/frontend/src/lib/components/PayCard/PayCard.tsx
@@ -37,12 +37,16 @@ export function PayCard({ title, caption, docsLink, identifier }: PayCardProps):
         reportPayGateDismissed(identifier)
     }
 
-    useEffect(() => {
-        if (!window.localStorage.getItem(storageKey)) {
-            setShown(true)
-            reportPayGateShown(identifier)
-        }
-    }, [])
+    useEffect(
+        () => {
+            if (!window.localStorage.getItem(storageKey)) {
+                setShown(true)
+                reportPayGateShown(identifier)
+            }
+        },
+        // eslint-disable-next-line react-hooks/exhaustive-deps
+        []
+    )
 
     if (!shown) {
         return null

diff --git a/frontend/src/lib/components/PropertyFilters/PathItemFilters.tsx b/frontend/src/lib/components/PropertyFilters/PathItemFilters.tsx
@@ -34,11 +34,15 @@ export function PathItemFilters({
     const { filters } = useValues(propertyFilterLogic(logicProps))
     const { setFilter, remove, setFilters } = useActions(propertyFilterLogic(logicProps))
 
-    useEffect(() => {
-        if (propertyFilters && !objectsEqual(propertyFilters, filters)) {
-            setFilters([...propertyFilters, {}])
-        }
-    }, [propertyFilters])
+    useEffect(
+        () => {
+            if (propertyFilters && !objectsEqual(propertyFilters, filters)) {
+                setFilters([...propertyFilters, {}])
+            }
+        },
+        // eslint-disable-next-line react-hooks/exhaustive-deps
+        [propertyFilters]
+    )
 
     return (
         <div className="mb" style={style}>

diff --git a/frontend/src/lib/components/PropertyFilters/components/PropertyValue.tsx b/frontend/src/lib/components/PropertyFilters/components/PropertyValue.tsx
@@ -72,16 +72,20 @@ export function PropertyValue({
     const autoCompleteRef = useRef<HTMLElement>(null)
 
     // update the input field if passed a new `value` prop
-    useEffect(() => {
-        if (!value) {
-            setInput('')
-        } else if (value !== input) {
-            const valueObject = options[propertyKey]?.values?.find((v) => v.id === value)
-            if (valueObject) {
-                setInput(toString(valueObject.name))
+    useEffect(
+        () => {
+            if (!value) {
+                setInput('')
+            } else if (value !== input) {
+                const valueObject = options[propertyKey]?.values?.find((v) => v.id === value)
+                if (valueObject) {
+                    setInput(toString(valueObject.name))
+                }
             }
-        }
-    }, [value])
+        },
+        // eslint-disable-next-line react-hooks/exhaustive-deps
+        [value]
+    )
 
     const loadPropertyValues = useThrottledCallback((newInput) => {
         if (type === 'cohort') {
@@ -119,9 +123,13 @@ export function PropertyValue({
         }
     }
 
-    useEffect(() => {
-        loadPropertyValues('')
-    }, [propertyKey])
+    useEffect(
+        () => {
+            loadPropertyValues('')
+        },
+        // eslint-disable-next-line react-hooks/exhaustive-deps
+        [propertyKey]
+    )
 
     useEffect(() => {
         if (input === '' && shouldBlur) {

diff --git a/frontend/src/lib/components/ResizableTable/index.tsx b/frontend/src/lib/components/ResizableTable/index.tsx
@@ -186,30 +186,34 @@ export function ResizableTable<RecordType extends Record<any, any> = any>({
         })
     }, [initialColumns])
 
-    useLayoutEffect(() => {
-        // Calculate relative column widths (px) once the wrapper is mounted.
-        if (scrollWrapperRef.current) {
-            resizeObserver.observe(scrollWrapperRef.current)
-            const wrapperWidth = scrollWrapperRef.current.clientWidth
-            const gridBasis = columns.reduce((total, { span }) => total + span, 0)
-            const columnSpanWidth = getFullwidthColumnSize(wrapperWidth, gridBasis)
-            setColumns((cols) => {
-                const lastIndex = cols.length
-                const nextColumns = cols.map((column, index) =>
-                    index === lastIndex
-                        ? column
-                        : {
-                              ...column,
-                              width: Math.max(column.defaultWidth || columnSpanWidth * column.span, minColumnWidth),
-                          }
-                )
-                setHeaderColumns(nextColumns)
-                return nextColumns
-            })
-            updateScrollGradient()
-            setHeaderShouldRender(true)
-        }
-    }, [])
+    useLayoutEffect(
+        () => {
+            // Calculate relative column widths (px) once the wrapper is mounted.
+            if (scrollWrapperRef.current) {
+                resizeObserver.observe(scrollWrapperRef.current)
+                const wrapperWidth = scrollWrapperRef.current.clientWidth
+                const gridBasis = columns.reduce((total, { span }) => total + span, 0)
+                const columnSpanWidth = getFullwidthColumnSize(wrapperWidth, gridBasis)
+                setColumns((cols) => {
+                    const lastIndex = cols.length
+                    const nextColumns = cols.map((column, index) =>
+                        index === lastIndex
+                            ? column
+                            : {
+                                  ...column,
+                                  width: Math.max(column.defaultWidth || columnSpanWidth * column.span, minColumnWidth),
+                              }
+                    )
+                    setHeaderColumns(nextColumns)
+                    return nextColumns
+                })
+                updateScrollGradient()
+                setHeaderShouldRender(true)
+            }
+        },
+        // eslint-disable-next-line react-hooks/exhaustive-deps
+        []
+    )
 
     return (
         <div ref={scrollWrapperRef} className="resizable-table-scroll-container" onScroll={updateScrollGradient}>

diff --git a/frontend/src/lib/components/RestrictedArea.tsx b/frontend/src/lib/components/RestrictedArea.tsx
@@ -32,32 +32,36 @@ export function RestrictedArea({
     const { currentOrganization } = useValues(organizationLogic)
     const { currentTeam } = useValues(teamLogic)
 
-    const restrictionReason: null | string = useMemo(() => {
-        let scopeAccessLevel: EitherMembershipLevel | null
-        if (scope === RestrictionScope.Project) {
-            if (!currentTeam) {
-                return 'Loading current project…'
+    const restrictionReason: null | string = useMemo(
+        () => {
+            let scopeAccessLevel: EitherMembershipLevel | null
+            if (scope === RestrictionScope.Project) {
+                if (!currentTeam) {
+                    return 'Loading current project…'
+                }
+                scopeAccessLevel = currentTeam.effective_membership_level
+            } else {
+                if (!currentOrganization) {
+                    return 'Loading current organization…'
+                }
+                scopeAccessLevel = currentOrganization.membership_level
             }
-            scopeAccessLevel = currentTeam.effective_membership_level
-        } else {
-            if (!currentOrganization) {
-                return 'Loading current organization…'
+            if (scopeAccessLevel === null) {
+                return `You don't have access to the current ${scope}.`
             }
-            scopeAccessLevel = currentOrganization.membership_level
-        }
-        if (scopeAccessLevel === null) {
-            return `You don't have access to the current ${scope}.`
-        }
-        if (scopeAccessLevel < minimumAccessLevel) {
-            if (minimumAccessLevel === OrganizationMembershipLevel.Owner) {
-                return `This area is restricted to the ${scope} owner.`
+            if (scopeAccessLevel < minimumAccessLevel) {
+                if (minimumAccessLevel === OrganizationMembershipLevel.Owner) {
+                    return `This area is restricted to the ${scope} owner.`
+                }
+                return `This area is restricted to ${scope} ${membershipLevelToName.get(
+                    minimumAccessLevel
+                )}s and up. Your level is ${membershipLevelToName.get(scopeAccessLevel)}.`
             }
-            return `This area is restricted to ${scope} ${membershipLevelToName.get(
-                minimumAccessLevel
-            )}s and up. Your level is ${membershipLevelToName.get(scopeAccessLevel)}.`
-        }
-        return null
-    }, [currentOrganization])
+            return null
+        },
+        // eslint-disable-next-line react-hooks/exhaustive-deps
+        [currentOrganization]
+    )
 
     return restrictionReason ? (
         <Tooltip title={restrictionReason} placement="topLeft" delayMs={0}>