5

its first time for me using react-router-dom v6, I am familiar with v4.

I have a list of movies and every movie has an id, I need to be navigated to not-found page if the users entered a wrong movie id in the url.

The problem that I am using a class component so I am stuck to use componentDidMount which causes the following error

You should call navigate() in a React.useEffect(), not when your component is first rendered.

here is my code:

 componentDidMount() {
    let { id } = this.props.params;
    const genres = getGenres();
    this.setState({ genres });
    const movieId = id;
    if (movieId === 'new') return;
    const movie = getMovie(movieId);
    if (!movie) return this.props.navigate('/not-found', { replace: true });
    this.setState({ data: this.mapToViewModel(movie) });
  }

I am using a class component as I mentioned, so I exported the class as a function to make the Useparams() work with me

function WithNavigate(props) {
  let navigate = useNavigate();
  return <MovieForm {...props} params={useParams()} navigate={navigate} />;
}

export default WithNavigate;

My question now how to make the navigate works in the componentDidMount!

Thank you in advance

yasser Eltibili
  • 134
  • 2
  • 8

3 Answers3

6

componentDidMount is rendered after useEffect()

For a simple fix, you can add

let { navigate } = this.props;

to your code like this, with setTimeOut

 componentDidMount() {
    let { id } = this.props.params;
    let { navigate } = this.props;

    const genres = getGenres();
    this.setState({ genres });
    const movieId = id;

    if (movieId === 'new') return;
    const movie = getMovie(movieId);
    if (!movie) {
           return setTimeOut(()=> this.props.navigate('/not-found', { replace: true }));
    }
    this.setState({ data: this.mapToViewModel(movie) });
  }
AnthonyDev220
  • 669
  • 5
  • 17
1

Well the actual solution is how to use useNavigate or you can make wrapperComponent that make it general solution you can use it anywhere in your codebase

import { useNavigate } from 'react-router-dom';

export const withNavigate = (Component) => {
  const Wrapper = (props) => {
    const navigate = useNavigate();
    
    return (
      <Component
        navigate={navigate}
        {...props}
        />
    );
  };
  
  return Wrapper;
};

Now you have only one wrapper function all you have to do is wrap your component wherever you want to use useNavigate and use it like this this.props.navigate('/not-found')

import React from 'react';
import {withNavigate} from 'src/hooks/withNavigate';

class MovieForm extends React.Component{
  // .... rest of the code


 componentDidMount() {
    let { id } = this.props.params;
    const genres = getGenres();
    this.setState({ genres });
    const movieId = id;
    if (movieId === 'new') return;
    const movie = getMovie(movieId);
    if (!movie) return this.props.navigate('/not-found');
    this.setState({ data: this.mapToViewModel(movie) });
  }



  // .... rest of the code
}

export default withNavigate(MovieForm);
Abbas Hussain
  • 1,277
  • 6
  • 14
  • I am taking help from your code and successfully navigate but getting below error on this.props.params: Cannot destructure property 'id' of 'this.props.params' On this line I'm trying to read parameter from URL. I think we need to use useParams but not sure how. Can you please help? – Jamil Aug 24 '22 at 09:09
0

You are defining/using WithNavigate as a React wrapper component instead of as a Higher Order component, which changes how the useNavigate hooks is used and passed to class component.

Instead of

function WithNavigate(props) {
  let navigate = useNavigate();
  return <MovieForm {...props} params={useParams()} navigate={navigate} />;
}

Convert to HOC

function withNavigate = Component => props => {
  const navigate = useNavigate();
  const params = useParams();

  return <Component {...props} params={params} navigate={navigate} />;
}

export default withNavigate;

Then decorate the MovieForm component as the default export.

export default withNavigate(MovieForm);
Drew Reese
  • 165,259
  • 14
  • 153
  • 181
  • 1
    Could you explore further why it works with a HOC and not with a direct Wrapper ? – Cesare Polonara Apr 03 '22 at 19:09
  • @CesarePolonara That's a fair question, and something I thought about after answering. I don't think there *should* much a difference, but it's a good idea to test both versions, perhaps there's something else going on in OP's code. – Drew Reese Apr 03 '22 at 19:12
  • Ok, that looked a weird behaviour to me, so I made some tests: [code](https://stackblitz.com/edit/react-vnpj34?file=src%2FApp.js) and it seems that if you use a direct Wrapper, when navigate() is called inside componentDidMount() , it works only if it's put inside a setTimeout, as @Anthony220 pointed out, but it works as expected in an onClick handler, so it's something related to when componentDidMount places its content in the Event Loop. With an HOC it works properly, and I don't understand where the difference lies! – Cesare Polonara Apr 03 '22 at 19:19
  • @CesarePolonara Hmm, oddly enough, I can't reproduce the error/warning using the wrapper component: https://codesandbox.io/s/new-water-5l3j8q?file=/src/App.js. IMO using a HOC is the more correct method, but you bring up a good point. – Drew Reese Apr 03 '22 at 19:29
  • @DrewReese I agree that HOC a more correct method, especially for large application. Placing a single logic inside one component and share with other components with subscrption is much more convenient. – AnthonyDev220 Apr 03 '22 at 19:47
  • @DrewReese I can't reproduce the warning either, but it simply does not fire the navigate() redirect if called inside componentDidMount in the Wrapper case ! – Cesare Polonara Apr 03 '22 at 20:29
  • 1
    @CesarePolonara Dang, I *can* reproduce the issue when starting from scratch on `"/"` and reloading the app, it displays the warning. My guess here is that `componentDidMount` and `useEffect` with empty dependency aren't completely equivalent. After taking a look at the [source code](https://github.com/remix-run/react-router/blob/7dca9dc38c837ed94796325b1e0582aa72a9313f/packages/react-router/index.tsx#L527-L538) there's actually an "initial render" invariant check that occurs, so the navigation action *can't* occur on the initial render. I haven't an idea why there is this constraint though. – Drew Reese Apr 03 '22 at 20:41
  • 2
    @Drew Reese On Stackblitz it doesn't show any warn, but it simply doesn't fire this.props.navigate(), and now i retried, it doesn't work no matter if using an HOC or a Wrapper ( This makes more sense ). Probably as you pointed out, the activeRef.current from source code is not valorized yet when componentDidMount() is fired, setTimeout with no delay fixes the behaviour probably because it places the execution of navigate at the end of the queue of the Event Loop, and when that time comes activeRef.current is no more null. – Cesare Polonara Apr 03 '22 at 20:53
  • @CesarePolonara Correct, making that part of the code asynchronous bumps its execution to a later render cycle. – Drew Reese Apr 03 '22 at 20:54
  • Yeah this does not solve the problem. Problem is of course with the fact that componentDidMount come before the useEffect hook. So the timeout answer though unfortunate is the right one. – Hasan Wajahat Sep 15 '22 at 10:42
  • @HasanWajahat That's a subjective opinion. *Technically* speaking, a "right" solution is one that solves your problem. OFC, there are solutions that are more, or less, *correct* than others, but again, this is subjective and prone to opinion. It is *my* opinion that using a `setTimeout` to move some logic to the back of the event queue is a bit of a hack, but it works, and is a good-ish solution in certain cases. My answer here totally solves the problem. It isn't the only solution. Class components for all intents and purposes are deprecated, so it is best to use React functions and hooks. – Drew Reese Sep 15 '22 at 19:50