Refactoring Logout Component into Function Component-CodePudding

I am in the process of turning some older class-based react components into function components. I came across a logout class component (that I didn't code) that looks like this:

import React from 'react';
import {
  Text,
} from 'react-native';

import { connect } from 'react-redux';
import { deauthenticateUser } from '../state/actions';

class LogoutScreen extends React.Component {
  constructor() {
    super();
  }

  async shouldComponentUpdate(nextProps, nextState) {
    if(nextState.login == false) {
      this.props.deauthenticateUser();
      this.props.navigation.navigate('Auth');
    }
    return false;
  }

  componentDidMount() {
    this.setState({login:false})
  }

  render() {
    return (
      <Text>Logging out...</Text>
    )
  }
}

// Redux Mapping
const mapStateToProps = state => {
  return { ...state.User }
};
const mapDispatchToProps = dispatch => ({
  deauthenticateUser: user => dispatch(deauthenticateUser(user)),
});
export default connect(mapStateToProps, mapDispatchToProps)(LogoutScreen);

I have refactored it into a function component like so:

import React, { useEffect, useState } from 'react';
import {
  Text,
} from 'react-native';
    
import { connect } from 'react-redux';
import { deauthenticateUser } from '../state/actions';
    
export const LogoutScreen = (props) => { 
  const [login, setLogin] = useState(false);
    
  useEffect(() => {
    if (login === false) {
      props.deauthenticateUser();
      props.navigation.navigate('Auth');
    }
  }, [login]);
    
  return (
    <Text>Logging out...</Text>
  )
}
    
// Redux Mapping
const mapStateToProps = state => {
  return { ...state.User }
};
const mapDispatchToProps = dispatch => ({
  deauthenticateUser: user => dispatch(deauthenticateUser(user)),
});
export default connect(mapStateToProps, mapDispatchToProps)(LogoutScreen);

Question: is [login] useful/necessary as a dependency in the useEffect() hook in this case? What would behave differently if I didn't include it here?

CodePudding user response：

It is necessary to add login as a dependency in react hook useEffect in your code.

Check this plugin out : https://www.npmjs.com/package/eslint-plugin-react-hooks

CodePudding user response：

This is a very weird component - it is performing some critical business logic (logout) simply because it was rendered. That's very strange. Even the original component didn't really do anything - it gets rendered, it immediately sets the login state to false, and then it logs out the user and naigates somewhere.

I hate to say, but you shouldn't be using a component to do this. Instead you should use redux thunks/sagas/whatever to take care detecting when the app is trying to log out. Usually the user clicks a logout button or a session timer fires and updates the app state to something like { loggingOut: true }. Then your thunk/saga/whatever picks up on this change and actually does the work of logging the user out and updating the current routes to redirect the user.

But I kind of expect that last paragraph to fall on deaf ears. Alas, you never needed the "login" state in either the original component or the new component. It's not doing anything and it only holds the value "false" - which is not state, it's just a variable with a constant value (eg. it never becomes "true"). Just get rid of it and log the user out immediately on render:

export const LogoutScreen = (props) => {
  // the effect will only run once (see the empty array as the second arg)
  useEffect(() => {
    props.deauthenticateUser();
    props.navigation.navigate('Auth');
  }, []);

  return (
    <Text>Logging out...</Text>
  )
}

And for completeness, here's what your original component should have looked like:

class LogoutScreen extends React.Component {
  componentDidMount() {
    this.props.deauthenticateUser();
    this.props.navigation.navigate('Auth');
  }

  render() {
    return (
      <Text>Logging out...</Text>
    )
  }
}

CodePudding user response：

I think Ryan's answer is good but insufficiently answers your questions. It seems you might have a larger lack of understanding the React component lifecycle so I'll answer your questions more generally.

useEffect(() => {
  if (XYZ) {
    ... side-effect ...
  }
}, [XYZ]);

Question 1

Is [XYZ] useful/necessary as a dependency in the useEffect() hook in this case?

In this case, code-wise, yes, absolutely it is. It's an external dependency that potentially changes and thus should be listed in the dependency array in order to retrigger the side-effect, i.e. the useEffect's callback function.

Not taking into account the actual business logic/use case here, just looking at code and syntax, what this means is any time dependencies update, run a side-effect.

A good general rule-of-thumb, and something the eslint-plugin-react-hooks plugin will help catch for you, is basically anything that is externally (to the hook) declared within the React function body is considered an external dependency. This includes the props object, any declared component state, any locally declared variables, etc.

From the React docs: Conditionally firing an effect

If you use this[dependency array] optimization, make sure the array includes all values from the component scope (such as props and state) that change over time and that are used by the effect. Otherwise, your code will reference stale values from previous renders. ...

Which leads naturally to your other question.

Question 2

What would behave differently if I didn't include it here?

A useEffect hook, or any hook really, with missing dependencies simply won't be retriggered when the missing dependency value changes.

The rest of the note from above:

... If you want to run an effect and clean it up only once (on mount and unmount), you can pass an empty array ([]) as a second argument. This tells React that your effect doesn’t depend on any values from props or state, so it never needs to re-run. This isn’t handled as a special case — it follows directly from how the dependencies array always works.

If you pass an empty array ([]), the props and state inside the effect will always have their initial values. While passing [] as the second argument is closer to the familiar componentDidMount and componentWillUnmount mental model, there are usually better solutions to avoid re-running effects too often. Also, don’t forget that React defers running useEffect until after the browser has painted, so doing extra work is less of a problem.

We recommend using the exhaustive-deps rule as part of our eslint-plugin-react-hooks package. It warns when dependencies are specified incorrectly and suggests a fix.

The logout effect is pretty poor example since the optimal solution is to handle that logic completely outside a React component and any component lifecycle. Next to that though, the business logic/use case there is to run the logout logic once when the component mounts and then navigate away. The component and useEffect hook exists only to trigger the logout flow and navigate to "Auth", something that is easily achieved in an asynchronous Redux action. In the case you do need to use the component and the useEffect hook then logically it makes sense to omit the dependencies.

useEffect(() => {
  props.deauthenticateUser();
  props.navigation.navigate('Auth');
}, []);

Note however that if you are using the eslint-plugin-react-hooks plugin that it will complain about missing dependencies, props in this case and want to add them. Again, for the reasons listed above, this is completely expected and correct/desired code. BUT if you still wanted to use an empty dependency array so the effect only runs once when the component mounts then you can, at your discretion, disable the linting rule for that specific line. If you do this then the recommendation is to also document why you are deviating from recommended settings.

Example:

useEffect(() => {
  props.deauthenticateUser();
  props.navigation.navigate('Auth');
  // NOTE: Effect triggered on component mount
  // Check dependencies if side-effect logic is updated.
  // eslint-disable-next-line react-hooks/exhaustive-deps
}, []);

A more concrete example is refetching data based on a parameter.

Example:

<Route path="/product/:id" element={<ProductDetail />} />

...

const ProductDetail = () => {
  const { id } = useParams();
  const [details, setDetails] = React.useState();

  useEffect(() => {
    fetch(`/product/${id}`)
      .then(setDetails);
  }, [id, setDetails]);

  ...
};

In this scenario the id parameter and the setDetails function are externally declared and referenced within the useEffect callback and added to the dependency array.