Reactjs.org: componentWillMount usage for "redirecting" based on condition

[andresmatasuarez]

Hi! Regarding the policy regarding UNSAFE lifecycle methods to be made effective sometime within React 16 lifespan, I was trying to reduce the usage of those methods in my project.

I came across this snippet in my code base, which is a simple HOC to ensure that user is authenticated, and if not, redirect him (using history replace) to login:

  class EnsureAuthenticatedConnector extends Component {
    componentWillMount() {
      if (!this.props.isAuthenticated) {
        this.props.history.replace('/web/login');
      }
    }

    componentWillReceiveProps(nextProps) {
      if (!nextProps.isAuthenticated) {
        nextProps.history.replace('/web/login');
      }
    }

    render() {
      return (
        <ComposedComponent {...this.props.originalProps} />
      );
    }
  }

Now, where should the componentWillMount/ReceiveProps logic go on the upcoming scheme of things? Does it make sense to move it inside constructor?

Also, I have other components that dispatches redux actions on componentWillMount, most of them are just for clearing form data before rendering the relevant component. Is constructor the right place to put this logic into, or should it go inside componentDidUpdate?

Thanks!

[gaearon]

I came across this snippet in my code base, which is a simple HOC to ensure that user is authenticated, and if not, redirect him (using history replace) to login

You could do this in componentDidMount but that would require actually rendering the component once which seems non-ideal.

I think the most idiomatic solution is to use error boundaries for this kind of stuff. For example:

  var accessDenied = {};

  function EnsureAuthenticatedConnector(props) {
    if (!props.isAuthenticated) {
      throw accessDenied;
    }
    return (
      <ComposedComponent {...props.originalProps} />
    );
  }

  class AuthenticatedBoundary extends Component {
    componentDidCatch(error) {
      if (error === accessDenied) {
        this.props.history.replace('/web/login');
      } else {
        throw error;
      }
    }
    render() {
      return this.props.children;
    }
  }

Then if you wrap your app in AuthenticatedBoundary, and it automatically redirects. I know it may seem a bit odd at first, but this is pretty much how authentication works in server-side frameworks too.

If for some reason you can't do that, I guess you could put something like this in a constructor but this is still bad (like any side effect in constructor) and won't work very well with upcoming features like Suspense.

Also, I have other components that dispatches redux actions on componentWillMount, most of them are just for clearing form data before rendering the relevant component. Is constructor the right place to put this logic into, or should it go inside componentDidUpdate?

Dispatching any actions from constructor (or componentWillMount, for that matter) is just as bad as doing that from render. You can use it as a temporary solution, but I suggest rethinking why you need to do that in the first place. Why does a component need to "clean up" data before it mounts? Does it mean another component "leaves a mess"? Should they be properly separated?

[andresmatasuarez]

Thanks for the reply @gaearon

Moving clearance logic from some components' componentWillMount to other components' componentWillUnmount is indeed a good idea., I would definitely give this a look.

But apart from this kind of HOCs, I have a few like this:

  class SetDefaultCompanyIdConnector extends Component {
    componentWillMount() {
      this.props.setCompanyId(this.props.userDefaultCompany);
      // Reducer just performs: state.set('companyId', action.payload.companyId);
    }

    render() {
      return (
        <ComposedComponent {...this.props.originalProps} />
      );
    }
  }

Used basically to keep current company in Redux store synchronized with the selected default company for the signed-in user. So when he/she logs in, company is set to his/her default one.

This logic cannot be in the constructor, at least is not advisable as you say, and if I move it to componentDidUpdate, I get the first render without it being set, so user is redirected to a company-less path, instead of his/her company's dashboard. Is there a recommended approach for this kind of situation? I was relying on componentWillMount to trigger it as soon as I can in the component's life cycle which I do know it doesn't ensure that it will be ready before the first render, but it was something and it had been working until now.

[gaearon]

Logic like this is already pretty bad for performance because you're in the middle of rendering, but then you tell Redux "you know what, we need to dispatch again". This is called a "cascading update". It's usually unnecessary but it requires some rethinking of your data flow.

Used basically to keep current company in Redux store synchronized with the selected default company for the signed-in user. So when he/she logs in, company is set to his/her default one.

Where is userDefaultCompany coming from? Something that knows it (and passes it into React components) can probably take care of updating the store too before anything even starts rendering.

[andresmatasuarez]

userDefaultCompany is coming from the user. User object is fetched fro the API upon sign-in, and default company is included in the response.

So, very roughly, component's instantiation is something like this:

const FinalClientDashboard = compose(
    EnsureAuthenticated,
    WithTitle('Dashboard'),
    SetDefaultCompanyId
  )(ClientDashboard);

Then, route instantiation using ReactRouter ends up like this:

  const DashboardRoutes = ({ match }) => (
    <FinalClientDashboard>
      <Switch>
        <Route exact path={match.path} component={RedirectToCompanyHome} />
        <Route path={`${match.path}/marketplace`} component={MarketplaceRoutes} />
        <Route path={`${match.path}/:companyId`} component={CompanyWiseRoutes} />
      </Switch>
    </FinalClientDashboard>
  );

And the component making the redirection is RedirectToCompanyHome

const RedirectToCompanyHome = WithCompanyId()(({ match, companyId }) => (
    companyId ?
      ( <Redirect from={match.path} to={`${match.path}/${companyId}`} /> ) :
      ( <Redirect from={match.path} to={`${match.path}/marketplace`} /> )
  ));

companyId is not reaching RedirectToCompanyHome "in time", so user gets redirected to marketplace route

[gaearon]

User object is fetched fro the API upon sign-in, and default company is included in the response.

So, immediately when you receive it, you can dispatch an action.