CODEWITHSUNDEEP
javascriptreactjsreact-hooks
Jagmohan Mandre
Jagmohan Mandre

Reputation: 81

Can I refactor multiple useEffect to single useEffect?

I have width in state which changes with window resize and showFilters as props which changes from true to false. And I want to remove listener on unmount. So, I have used three useState for each these conditions. So, is there any refactor I can do to use all these in single useEffect.

import React, { useEffect, useRef, useState } from 'react'
import PropTypes from 'prop-types'
import { Icon } from 'antd'

import TrendsChart from './trendsChart'

import styled from '../styled-components'

const Chart = ({ showFilters }) => {
  const [width, setWidth] = useState(null)
  useEffect(() => {
    window.addEventListener('resize', handleWindowResize)
    updateWidth()
  }, [width])
  useEffect(() => {
    setTimeout(updateWidth, 200)
  }, [showFilters])
  useEffect(() => () => {
    window.removeEventListener('resize', handleWindowResize)
  })
  const updateWidth = () => {
    const containerWidth = chartRef.current.clientWidth
    setWidth(Math.floor(containerWidth))
  }
  const handleWindowResize = () => {
    updateWidth()
  }
  const chartRef = useRef()
  function render() {
    return (
      <styled.chart>
        <styled.chartHeader>
          Daily
        </styled.chartHeader>
        <styled.trendsChart id="chartRef" ref={chartRef}>
          <TrendsChart width={width} showFilters={showFilters}/>
        </styled.trendsChart>
        <div>
          <Icon type="dash" />&nbsp;&nbsp;&nbsp;Credit Trend
        </div>
      </styled.chart>
    )
  }
  return (
    render()
  )
}

Chart.propTypes = {
  showFilters: PropTypes.bool.isRequired
}
export default Chart

Upvotes: 2

Views: 1054

Answers (2)

xadm
xadm

Reputation: 8418

You should look at CONDITIONS, when each effect works.

Listener should be installed ONCE, with cleanup:

useEffect(() => {
  window.addEventListener('resize',handleWindowResize)
  return () => window.removeEventListener('resize',handleWindowResize)
},[])

const handleWindowResize = () => {
  const containerWidth = chartRef.current.clientWidth
  setWidth(Math.floor(containerWidth))
}

NOTICE: [] as useEffect parameter, without this effect works on every render

... and it should be enough as:

  • handleWindowResize sets width on window size changes;
  • showFilters causes rerender automatically

Upvotes: 1

Wesley Loh
Wesley Loh

Reputation: 163

from what i understand is two of your useEffect can be merge

useEffect(() => {
window.addEventListener('resize',handleWindowResize)
return () => window.removeEventListener('resize',handleWindowResize)
},[width])

For the set timeout part, from what i understand. That is not needed because react will rerender everytime the width(state) is changed. Hope it is help. I'm new to react too.

Upvotes: 1

Related Questions