Skip to content

Commit

Permalink
fix: Do not depend only on konnector.slug for useEffect dependencies
Browse files Browse the repository at this point in the history
Used React.memo and use-deep-compare-effect for hooks to not have
infinite rerenders. But the real cause for these rerenders comes from
the home application which generates a new konnector object on each
rerender.
  • Loading branch information
doubleface authored and doubleface committed Oct 24, 2022
1 parent 8ce5e78 commit 65869cf
Show file tree
Hide file tree
Showing 5 changed files with 427 additions and 19 deletions.
1 change: 1 addition & 0 deletions packages/cozy-harvest-lib/package.json
Expand Up @@ -38,6 +38,7 @@
"node-polyglot": "^2.4.0",
"react-final-form": "^3.7.0",
"react-markdown": "^4.2.2",
"use-deep-compare-effect": "^1.8.1",
"uuid": "^3.3.2"
},
"devDependencies": {
Expand Down
Expand Up @@ -10,6 +10,7 @@ import {
intentsApiProptype,
innerAccountModalOverridesProptype
} from '../../../helpers/proptypes'
import isEqual from 'lodash/isEqual'

const BIContractActivationWindow = ({
konnector,
Expand Down Expand Up @@ -49,7 +50,7 @@ const BIContractActivationWindow = ({
if (konnectorPolicy.fetchExtraOAuthUrlParams) {
handleLinkFetch()
}
}, [konnector.slug, account, client, konnectorPolicy])
}, [konnector, account, client, konnectorPolicy])

if (!konnectorPolicy.fetchExtraOAuthUrlParams) return null

Expand Down Expand Up @@ -90,4 +91,6 @@ BIContractActivationWindow.propTypes = {
innerAccountModalOverrides: innerAccountModalOverridesProptype
}

export default withLocales(BIContractActivationWindow)
// use isEqual to avoid an infinite rerender since the konnector object is a new one on each render
// when used in the home application
export default React.memo(withLocales(BIContractActivationWindow), isEqual)
7 changes: 5 additions & 2 deletions packages/cozy-harvest-lib/src/components/OAuthForm.jsx
Expand Up @@ -14,6 +14,7 @@ import { ERROR_EVENT, LOGIN_SUCCESS_EVENT } from '../models/flowEvents'
import { KonnectorJobError } from '../helpers/konnectors'
import { findKonnectorPolicy } from '../konnector-policies'
import flag from 'cozy-flags'
import isEqual from 'lodash/isEqual'

/**
* The OAuth Form is responsible for displaying a form for OAuth konnectors. It
Expand Down Expand Up @@ -67,7 +68,7 @@ export const OAuthForm = props => {
if (konnectorPolicy.isBIWebView && flag('harvest.bi.fullwebhooks')) {
flow.expectTriggerLaunch({ konnector })
}
}, [client, flow, konnector.slug])
}, [flow, konnector])

const handleOAuthCancel = err => {
flow.triggerEvent(ERROR_EVENT, translateOauthError(err))
Expand Down Expand Up @@ -150,4 +151,6 @@ OAuthForm.propTypes = {
intentsApi: intentsApiProptype
}

export default compose(withLocales)(OAuthForm)
// use isEqual to avoid an infinite rerender since the konnector object is a new one on each render
// when used in the home application
export default React.memo(compose(withLocales)(OAuthForm), isEqual)
@@ -1,15 +1,19 @@
import { useEffect, useState } from 'react'
import { useState } from 'react'

import { findKonnectorPolicy } from '../../konnector-policies'

import useDeepCompareEffect from 'use-deep-compare-effect'

const useOAuthExtraParams = ({ account, client, konnector, reconnect }) => {
const konnectorPolicy = findKonnectorPolicy(konnector)
const needsExtraParams = konnectorPolicy.fetchExtraOAuthUrlParams

const [fetchStatus, setFetchStatus] = useState('loading')
const [extraParams, setExtraParams] = useState(null)

useEffect(() => {
// use useDeepCompareEffect to avoid an infinite rerender since the konnector object is a new one
// on each render when used in the home application
useDeepCompareEffect(() => {
const load = async () => {
try {
const extraParams = await konnectorPolicy.fetchExtraOAuthUrlParams({
Expand All @@ -27,14 +31,7 @@ const useOAuthExtraParams = ({ account, client, konnector, reconnect }) => {
if (needsExtraParams) {
load()
}
}, [
account,
client,
konnector.slug,
konnectorPolicy,
needsExtraParams,
reconnect
])
}, [account, client, konnector, konnectorPolicy, needsExtraParams, reconnect])

return {
fetchStatus,
Expand Down

0 comments on commit 65869cf

Please sign in to comment.