From 30513addfdca6f9221493a5bbdcab4837436f72b Mon Sep 17 00:00:00 2001 From: Sebastian Szewczyk Date: Sat, 30 Sep 2023 22:25:27 +0200 Subject: [PATCH 1/2] Optimized context usages and set up eslint rule to prevent unnecessary rerender in the future --- .eslintrc.js | 2 + .../AnimatedStep/AnimatedStepProvider.js | 5 +- .../AttachmentCarousel/Pager/index.js | 39 ++++++++++----- src/components/DragAndDrop/Provider/index.js | 5 +- src/components/PopoverProvider/index.js | 16 +++---- .../PopoverProvider/index.native.js | 16 +++---- src/components/ScrollViewWithContext.js | 12 +++-- src/components/withKeyboardState.js | 7 ++- src/components/withWindowDimensions/index.js | 48 +++++++++---------- .../withWindowDimensions/index.native.js | 45 ++++++++--------- src/pages/home/report/ReportActionItem.js | 23 ++++----- .../TwoFactorAuth/TwoFactorAuthSteps.js | 7 ++- 12 files changed, 118 insertions(+), 107 deletions(-) diff --git a/.eslintrc.js b/.eslintrc.js index b5b4add538f6..afcea91acc34 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -37,9 +37,11 @@ module.exports = { overrides: [ { files: ['*.js', '*.jsx', '*.ts', '*.tsx'], + plugins: ['react'], rules: { 'rulesdir/onyx-props-must-have-default': 'off', 'react-native-a11y/has-accessibility-hint': ['off'], + 'react/jsx-no-constructed-context-values': 'error', 'react-native-a11y/has-valid-accessibility-descriptors': [ 'error', { diff --git a/src/components/AnimatedStep/AnimatedStepProvider.js b/src/components/AnimatedStep/AnimatedStepProvider.js index 280fbd1a2776..86d40b5bddeb 100644 --- a/src/components/AnimatedStep/AnimatedStepProvider.js +++ b/src/components/AnimatedStep/AnimatedStepProvider.js @@ -1,4 +1,4 @@ -import React, {useState} from 'react'; +import React, {useMemo, useState} from 'react'; import PropTypes from 'prop-types'; import AnimatedStepContext from './AnimatedStepContext'; import CONST from '../../CONST'; @@ -9,8 +9,9 @@ const propTypes = { function AnimatedStepProvider({children}) { const [animationDirection, setAnimationDirection] = useState(CONST.ANIMATION_DIRECTION.IN); + const contextValue = useMemo(() => ({animationDirection, setAnimationDirection}), [animationDirection, setAnimationDirection]); - return {children}; + return {children}; } AnimatedStepProvider.propTypes = propTypes; diff --git a/src/components/Attachments/AttachmentCarousel/Pager/index.js b/src/components/Attachments/AttachmentCarousel/Pager/index.js index 9779963dfc4a..4ca4e77e5136 100644 --- a/src/components/Attachments/AttachmentCarousel/Pager/index.js +++ b/src/components/Attachments/AttachmentCarousel/Pager/index.js @@ -1,5 +1,5 @@ /* eslint-disable es/no-optional-chaining */ -import React, {useRef, useState, useImperativeHandle} from 'react'; +import React, {useRef, useState, useImperativeHandle, useMemo} from 'react'; import {View} from 'react-native'; import PropTypes from 'prop-types'; import {GestureHandlerRootView, createNativeWrapper} from 'react-native-gesture-handler'; @@ -126,21 +126,34 @@ function AttachmentCarouselPager({ scrollEnabled: shouldPagerScroll.value, })); + const contextValue = useMemo(() => ({ + canvasWidth: containerWidth, + canvasHeight: containerHeight, + isScrolling, + pagerRef, + shouldPagerScroll, + onPinchGestureChange, + onTap, + onSwipe, + onSwipeSuccess, + onSwipeDown, + }), [ + containerWidth, + containerHeight, + isScrolling, + pagerRef, + shouldPagerScroll, + onPinchGestureChange, + onTap, + onSwipe, + onSwipeSuccess, + onSwipeDown, + ]); + return ( ({isDraggingOver, setOnDropHandler, dropZoneID: dropZoneID.current}), [isDraggingOver, setOnDropHandler]); return ( - + (dropZone.current = e)} style={[styles.flex1, styles.w100, styles.h100]} diff --git a/src/components/PopoverProvider/index.js b/src/components/PopoverProvider/index.js index efa230d920d5..c0c62cb70cb2 100644 --- a/src/components/PopoverProvider/index.js +++ b/src/components/PopoverProvider/index.js @@ -111,15 +111,15 @@ function PopoverContextProvider(props) { [closePopover], ); + const contextValue = React.useMemo(() => ({ + onOpen, + close: closePopover, + popover: activePopoverRef.current, + isOpen, + }), [onOpen, closePopover, isOpen]); + return ( - + {props.children} ); diff --git a/src/components/PopoverProvider/index.native.js b/src/components/PopoverProvider/index.native.js index f34abcb1fa62..aa22bc7cd605 100644 --- a/src/components/PopoverProvider/index.native.js +++ b/src/components/PopoverProvider/index.native.js @@ -15,15 +15,15 @@ const PopoverContext = React.createContext({ }); function PopoverContextProvider(props) { + const contextValue = React.useMemo(() => ({ + onOpen: () => {}, + close: () => {}, + popover: {}, + isOpen: false, + }), []); + return ( - {}, - close: () => {}, - popover: {}, - isOpen: false, - }} - > + {props.children} ); diff --git a/src/components/ScrollViewWithContext.js b/src/components/ScrollViewWithContext.js index bf0e7c6d62e8..b858f741f1fe 100644 --- a/src/components/ScrollViewWithContext.js +++ b/src/components/ScrollViewWithContext.js @@ -1,4 +1,4 @@ -import React, {useState, useRef} from 'react'; +import React, {useState, useRef, useMemo} from 'react'; import {ScrollView} from 'react-native'; const MIN_SMOOTH_SCROLL_EVENT_THROTTLE = 16; @@ -27,6 +27,11 @@ function ScrollViewWithContext({onScroll, scrollEventThrottle, children, innerRe setContentOffsetY(event.nativeEvent.contentOffset.y); }; + const contextValue = useMemo(() => ({ + scrollViewRef, + contentOffsetY, + }), [scrollViewRef, contentOffsetY]); + return ( {children} diff --git a/src/components/withKeyboardState.js b/src/components/withKeyboardState.js index 8ddf667b4e30..dbd90af19b03 100755 --- a/src/components/withKeyboardState.js +++ b/src/components/withKeyboardState.js @@ -1,4 +1,4 @@ -import React, {forwardRef, createContext, useEffect, useState} from 'react'; +import React, {forwardRef, createContext, useEffect, useState, useMemo} from 'react'; import {Keyboard} from 'react-native'; import PropTypes from 'prop-types'; import getComponentDisplayName from '../libs/getComponentDisplayName'; @@ -31,7 +31,10 @@ function KeyboardStateProvider(props) { }; }, []); - return {children}; + const contextValue = useMemo(() => ({ + isKeyboardShown, + }), [isKeyboardShown]); + return {children}; } KeyboardStateProvider.propTypes = keyboardStateProviderPropTypes; diff --git a/src/components/withWindowDimensions/index.js b/src/components/withWindowDimensions/index.js index 37d5c94688a2..bd08fb673c2a 100644 --- a/src/components/withWindowDimensions/index.js +++ b/src/components/withWindowDimensions/index.js @@ -1,8 +1,8 @@ -import React, {forwardRef, createContext, useState, useEffect} from 'react'; +import React, {forwardRef, createContext, useState, useEffect, useMemo} from 'react'; import PropTypes from 'prop-types'; import lodashDebounce from 'lodash/debounce'; import {Dimensions} from 'react-native'; -import {SafeAreaInsetsContext} from 'react-native-safe-area-context'; +import {useSafeAreaInsets} from 'react-native-safe-area-context'; import getComponentDisplayName from '../../libs/getComponentDisplayName'; import variables from '../../styles/variables'; import getWindowHeightAdjustment from '../../libs/getWindowHeightAdjustment'; @@ -62,30 +62,28 @@ function WindowDimensionsProvider(props) { dimensionsEventListener.remove(); }; }, []); - + const insets = useSafeAreaInsets(); + const adjustment = getWindowHeightAdjustment(insets); + const contextValue = useMemo(() => { + const isExtraSmallScreenWidth = windowDimension.windowWidth <= variables.extraSmallMobileResponsiveWidthBreakpoint; + const isSmallScreenWidth = windowDimension.windowWidth <= variables.mobileResponsiveWidthBreakpoint; + const isMediumScreenWidth = !isSmallScreenWidth && windowDimension.windowWidth <= variables.tabletResponsiveWidthBreakpoint; + const isLargeScreenWidth = !isSmallScreenWidth && !isMediumScreenWidth; + return { + windowHeight: windowDimension.windowHeight + adjustment, + windowWidth: windowDimension.windowWidth, + isExtraSmallScreenWidth, + isSmallScreenWidth, + isMediumScreenWidth, + isLargeScreenWidth, + }; + }, [windowDimension.windowHeight, windowDimension.windowWidth, adjustment]); return ( - - {(insets) => { - const isExtraSmallScreenWidth = windowDimension.windowWidth <= variables.extraSmallMobileResponsiveWidthBreakpoint; - const isSmallScreenWidth = windowDimension.windowWidth <= variables.mobileResponsiveWidthBreakpoint; - const isMediumScreenWidth = !isSmallScreenWidth && windowDimension.windowWidth <= variables.tabletResponsiveWidthBreakpoint; - const isLargeScreenWidth = !isSmallScreenWidth && !isMediumScreenWidth; - return ( - - {props.children} - - ); - }} - + + {props.children} + ); } diff --git a/src/components/withWindowDimensions/index.native.js b/src/components/withWindowDimensions/index.native.js index e147a20c9f4e..bd0b6574ddf4 100644 --- a/src/components/withWindowDimensions/index.native.js +++ b/src/components/withWindowDimensions/index.native.js @@ -1,7 +1,7 @@ -import React, {forwardRef, createContext, useState, useEffect} from 'react'; +import React, {forwardRef, createContext, useState, useEffect, useMemo} from 'react'; import PropTypes from 'prop-types'; import {Dimensions} from 'react-native'; -import {SafeAreaInsetsContext} from 'react-native-safe-area-context'; +import {useSafeAreaInsets} from 'react-native-safe-area-context'; import getComponentDisplayName from '../../libs/getComponentDisplayName'; import variables from '../../styles/variables'; import getWindowHeightAdjustment from '../../libs/getWindowHeightAdjustment'; @@ -60,30 +60,25 @@ function WindowDimensionsProvider(props) { dimensionsEventListener.remove(); }; }, []); - + const insets = useSafeAreaInsets(); + const adjustment = getWindowHeightAdjustment(insets); + const contextValue = useMemo(() => { + const isExtraSmallScreenWidth = windowDimension.windowWidth <= variables.extraSmallMobileResponsiveWidthBreakpoint; + return { + windowHeight: windowDimension.windowHeight + adjustment, + windowWidth: windowDimension.windowWidth, + isExtraSmallScreenWidth, + isSmallScreenWidth: true, + isMediumScreenWidth: false, + isLargeScreenWidth: false, + }; + }, [windowDimension.windowHeight, windowDimension.windowWidth, adjustment]); return ( - - {(insets) => { - const isExtraSmallScreenWidth = windowDimension.windowWidth <= variables.extraSmallMobileResponsiveWidthBreakpoint; - const isSmallScreenWidth = true; - const isMediumScreenWidth = false; - const isLargeScreenWidth = false; - return ( - - {props.children} - - ); - }} - + + {props.children} + ); } diff --git a/src/pages/home/report/ReportActionItem.js b/src/pages/home/report/ReportActionItem.js index d109c972ec69..ecaa8c0a84d4 100644 --- a/src/pages/home/report/ReportActionItem.js +++ b/src/pages/home/report/ReportActionItem.js @@ -1,6 +1,6 @@ import _ from 'underscore'; import lodashGet from 'lodash/get'; -import React, {useState, useRef, useEffect, memo, useCallback, useContext} from 'react'; +import React, {useState, useRef, useEffect, memo, useCallback, useContext, useMemo} from 'react'; import {InteractionManager, View} from 'react-native'; import PropTypes from 'prop-types'; import {withOnyx} from 'react-native-onyx'; @@ -263,6 +263,13 @@ function ReportActionItem(props) { [props.report, props.action, props.emojiReactions], ); + const contextValue = useMemo(() => ({ + anchor: popoverAnchorRef, + report: props.report, + action: props.action, + checkIfContextMenuActive: toggleContextMenuFromActiveReportAction, + }), [props.report, props.action, toggleContextMenuFromActiveReportAction]); + /** * Get the content of ReportActionItem * @param {Boolean} hovered whether the ReportActionItem is hovered @@ -356,12 +363,7 @@ function ReportActionItem(props) { const hasBeenFlagged = !_.contains([CONST.MODERATION.MODERATOR_DECISION_APPROVED, CONST.MODERATION.MODERATOR_DECISION_PENDING], moderationDecision); children = ( {!props.draftMessage ? ( @@ -504,12 +506,7 @@ function ReportActionItem(props) { if (ReportActionsUtils.isTransactionThread(parentReportAction)) { return ( ({setStep: handleSetStep}), [handleSetStep]); const renderStep = () => { switch (currentStep) { @@ -60,9 +61,7 @@ function TwoFactorAuthSteps({account = defaultAccount}) { return ( {renderStep()} From cc4052cdbc6efa1f0da2c4f14ad7563e85c73817 Mon Sep 17 00:00:00 2001 From: Sebastian Szewczyk Date: Thu, 19 Oct 2023 14:11:26 +0200 Subject: [PATCH 2/2] Prettier autofix --- .../AttachmentCarousel/Pager/index.js | 42 +++++++------------ src/components/PopoverProvider/index.js | 21 +++++----- .../PopoverProvider/index.native.js | 21 +++++----- src/components/ScrollViewWithContext.js | 17 ++++---- src/components/withKeyboardState.js | 9 ++-- src/components/withWindowDimensions/index.js | 8 +--- .../withWindowDimensions/index.native.js | 8 +--- src/pages/home/report/ReportActionItem.js | 23 +++++----- .../TwoFactorAuth/TwoFactorAuthSteps.js | 8 +--- 9 files changed, 64 insertions(+), 93 deletions(-) diff --git a/src/components/Attachments/AttachmentCarousel/Pager/index.js b/src/components/Attachments/AttachmentCarousel/Pager/index.js index 4ca4e77e5136..d10a5abad6b7 100644 --- a/src/components/Attachments/AttachmentCarousel/Pager/index.js +++ b/src/components/Attachments/AttachmentCarousel/Pager/index.js @@ -126,35 +126,25 @@ function AttachmentCarouselPager({ scrollEnabled: shouldPagerScroll.value, })); - const contextValue = useMemo(() => ({ - canvasWidth: containerWidth, - canvasHeight: containerHeight, - isScrolling, - pagerRef, - shouldPagerScroll, - onPinchGestureChange, - onTap, - onSwipe, - onSwipeSuccess, - onSwipeDown, - }), [ - containerWidth, - containerHeight, - isScrolling, - pagerRef, - shouldPagerScroll, - onPinchGestureChange, - onTap, - onSwipe, - onSwipeSuccess, - onSwipeDown, - ]); + const contextValue = useMemo( + () => ({ + canvasWidth: containerWidth, + canvasHeight: containerHeight, + isScrolling, + pagerRef, + shouldPagerScroll, + onPinchGestureChange, + onTap, + onSwipe, + onSwipeSuccess, + onSwipeDown, + }), + [containerWidth, containerHeight, isScrolling, pagerRef, shouldPagerScroll, onPinchGestureChange, onTap, onSwipe, onSwipeSuccess, onSwipeDown], + ); return ( - + ({ - onOpen, - close: closePopover, - popover: activePopoverRef.current, - isOpen, - }), [onOpen, closePopover, isOpen]); - - return ( - - {props.children} - + const contextValue = React.useMemo( + () => ({ + onOpen, + close: closePopover, + popover: activePopoverRef.current, + isOpen, + }), + [onOpen, closePopover, isOpen], ); + + return {props.children}; } PopoverContextProvider.defaultProps = defaultProps; diff --git a/src/components/PopoverProvider/index.native.js b/src/components/PopoverProvider/index.native.js index aa22bc7cd605..e4da13752b6d 100644 --- a/src/components/PopoverProvider/index.native.js +++ b/src/components/PopoverProvider/index.native.js @@ -15,18 +15,17 @@ const PopoverContext = React.createContext({ }); function PopoverContextProvider(props) { - const contextValue = React.useMemo(() => ({ - onOpen: () => {}, - close: () => {}, - popover: {}, - isOpen: false, - }), []); - - return ( - - {props.children} - + const contextValue = React.useMemo( + () => ({ + onOpen: () => {}, + close: () => {}, + popover: {}, + isOpen: false, + }), + [], ); + + return {props.children}; } PopoverContextProvider.defaultProps = defaultProps; diff --git a/src/components/ScrollViewWithContext.js b/src/components/ScrollViewWithContext.js index b858f741f1fe..1d183e250767 100644 --- a/src/components/ScrollViewWithContext.js +++ b/src/components/ScrollViewWithContext.js @@ -27,10 +27,13 @@ function ScrollViewWithContext({onScroll, scrollEventThrottle, children, innerRe setContentOffsetY(event.nativeEvent.contentOffset.y); }; - const contextValue = useMemo(() => ({ - scrollViewRef, - contentOffsetY, - }), [scrollViewRef, contentOffsetY]); + const contextValue = useMemo( + () => ({ + scrollViewRef, + contentOffsetY, + }), + [scrollViewRef, contentOffsetY], + ); return ( - - {children} - + {children} ); } diff --git a/src/components/withKeyboardState.js b/src/components/withKeyboardState.js index dbd90af19b03..3154f7e98d67 100755 --- a/src/components/withKeyboardState.js +++ b/src/components/withKeyboardState.js @@ -31,9 +31,12 @@ function KeyboardStateProvider(props) { }; }, []); - const contextValue = useMemo(() => ({ - isKeyboardShown, - }), [isKeyboardShown]); + const contextValue = useMemo( + () => ({ + isKeyboardShown, + }), + [isKeyboardShown], + ); return {children}; } diff --git a/src/components/withWindowDimensions/index.js b/src/components/withWindowDimensions/index.js index bd08fb673c2a..16e5985e0985 100644 --- a/src/components/withWindowDimensions/index.js +++ b/src/components/withWindowDimensions/index.js @@ -78,13 +78,7 @@ function WindowDimensionsProvider(props) { isLargeScreenWidth, }; }, [windowDimension.windowHeight, windowDimension.windowWidth, adjustment]); - return ( - - {props.children} - - ); + return {props.children}; } WindowDimensionsProvider.propTypes = windowDimensionsProviderPropTypes; diff --git a/src/components/withWindowDimensions/index.native.js b/src/components/withWindowDimensions/index.native.js index bd0b6574ddf4..363196b3fd4d 100644 --- a/src/components/withWindowDimensions/index.native.js +++ b/src/components/withWindowDimensions/index.native.js @@ -73,13 +73,7 @@ function WindowDimensionsProvider(props) { isLargeScreenWidth: false, }; }, [windowDimension.windowHeight, windowDimension.windowWidth, adjustment]); - return ( - - {props.children} - - ); + return {props.children}; } WindowDimensionsProvider.propTypes = windowDimensionsProviderPropTypes; diff --git a/src/pages/home/report/ReportActionItem.js b/src/pages/home/report/ReportActionItem.js index 0d3d2021e3d3..db5ac43d9d34 100644 --- a/src/pages/home/report/ReportActionItem.js +++ b/src/pages/home/report/ReportActionItem.js @@ -274,12 +274,15 @@ function ReportActionItem(props) { [props.report, props.action, props.emojiReactions], ); - const contextValue = useMemo(() => ({ - anchor: popoverAnchorRef, - report: props.report, - action: props.action, - checkIfContextMenuActive: toggleContextMenuFromActiveReportAction, - }), [props.report, props.action, toggleContextMenuFromActiveReportAction]); + const contextValue = useMemo( + () => ({ + anchor: popoverAnchorRef, + report: props.report, + action: props.action, + checkIfContextMenuActive: toggleContextMenuFromActiveReportAction, + }), + [props.report, props.action, toggleContextMenuFromActiveReportAction], + ); /** * Get the content of ReportActionItem @@ -373,9 +376,7 @@ function ReportActionItem(props) { } else { const hasBeenFlagged = !_.contains([CONST.MODERATION.MODERATOR_DECISION_APPROVED, CONST.MODERATION.MODERATOR_DECISION_PENDING], moderationDecision); children = ( - + {!props.draftMessage ? ( + - {renderStep()} - - ); + return {renderStep()}; } TwoFactorAuthSteps.propTypes = TwoFactorAuthPropTypes;