Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

React.lazy makes Route's proptypes fail #6420

Closed
jgoux opened this issue Oct 24, 2018 · 28 comments

Comments

@jgoux
Copy link

commented Oct 24, 2018

Hello,

The newly introduced React.lazy for easy code-splitting is making the Route component proptypes to fail with the warning :

Warning: Failed prop type: Invalid prop `component` of type `object` supplied to `Route`, expected `function`.

Version

react-router-dom: 4.4.0-beta.4
react: 16.6.0

Test Case

I took the official example from here : https://reactjs.org/docs/code-splitting.html#route-based-code-splitting

https://codesandbox.io/s/xp2nwl2qxw

@nfvs

This comment has been minimized.

Copy link

commented Oct 24, 2018

Same thing with React.memo().

@timdorr

This comment has been minimized.

Copy link
Collaborator

commented Oct 24, 2018

Likely fixed by #6327 and/or #6417.

@shawyu

This comment has been minimized.

Copy link

commented Oct 24, 2018

Did you also upgrade react-dom to 16.6.0? I had the same issues but this tweet from @gaearon to a similar issue solved it for me.

edit: Never mind, I see the package.json now. The above fixed the issue for my case, but perhaps there is also something else going on.
edit2: I see, originally it was not rendering at all but now I still see the console errors. Ignore me 😓

@rnnyrk

This comment has been minimized.

Copy link

commented Oct 26, 2018

So how exactly can I fix this warning? I'd read the #6417 and #6327 issues and saw the PR was merged into the master branch. Currently I'm on version 4.3.1 of react-router but still seeing this warning in the console. Thanks in advance!

@timdorr

This comment has been minimized.

Copy link
Collaborator

commented Oct 26, 2018

Wait for a release :)

@tylerwray

This comment has been minimized.

Copy link

commented Oct 26, 2018

@rnnyrk I just did the following for now 🤷‍♂️

const Home = React.lazy(() => import('./Home')

<Route exact path="/" component={props => <Home {...props} />} />
@jadbox

This comment has been minimized.

Copy link

commented Oct 27, 2018

@tylerwray The trick doesn't seem to work with Typescript since Lazy doesn't have proper typing and jsx elements must be explicitly defined as a jsx component.

@timdorr

This comment has been minimized.

Copy link
Collaborator

commented Nov 1, 2018

This was fixed in beta.5.

@selbekk

This comment has been minimized.

Copy link
Contributor

commented Nov 8, 2018

It's still an issue if react-is is resolved to 16.5.x - took me a good hour to track that down.

If you want, I can create a PR to bump the dependency requirement, perhaps?

Here's a reproduction, forked from the original codesandbox in this issue: https://codesandbox.io/s/q3y66owkj6

@frehner

This comment has been minimized.

Copy link
Contributor

commented Nov 8, 2018

#6447 :)

@selbekk

This comment has been minimized.

Copy link
Contributor

commented Nov 8, 2018

Ah, sorry!

@pierreferry

This comment has been minimized.

Copy link

commented Nov 26, 2018

@rnnyrk I just did the following for now 🤷‍♂️

const Home = React.lazy(() => import('./Home')

<Route exact path="/" component={props => <Home {...props} />} />

This trick does fix the error, but can have some side effect since it makes Home component re-render at every parent render. ( eg: if Home have componentDidMount )

@pshrmn

This comment has been minimized.

Copy link
Collaborator

commented Nov 26, 2018

@pierreferry if you go that path, the render prop would be better.

@wulucxy

This comment has been minimized.

Copy link

commented Dec 20, 2018

@pshrmn hey,when will you publish 4.4.0 version,this is a common problem。

@vtereshyn

This comment has been minimized.

Copy link

commented Dec 29, 2018

Any updates? It would be a great gift for the new year for me :)

@semoal

This comment has been minimized.

Copy link

commented Jan 2, 2019

Same here, any updates?

@gunn4r

This comment has been minimized.

Copy link

commented Jan 8, 2019

Same. The render prop work around resolves the warning. Not sure why this issue is closed though. It needs a fix. ¯\_(ツ)_/¯

@willnew

This comment has been minimized.

Copy link

commented Jan 9, 2019

I resolve this warning by using render <Route exact path="/" render={() => <Home />} />

@vtereshyn

This comment has been minimized.

Copy link

commented Jan 9, 2019

@willnew , as @pierreferry mentioned:
This trick does fix the error, but can have some side effect since it makes Home component re-render at every parent render. ( eg: if Home have componentDidMount )

@pierreferry

This comment has been minimized.

Copy link

commented Jan 9, 2019

@vtereshyn I believe this is different :

I haven't tested render method yet :)
Another thing i haven't tested: using not-inline functions

const renderHome = () => <Home />;

<Route exact path="/" render={renderHome} />
@willnew

This comment has been minimized.

Copy link

commented Jan 9, 2019

@vtereshyn I made a small Demo, you can test with it with Paint flashing option enabled on Chrome developer tool, the Home component didn't get re-rendered every time its parent's state change. so I think its a workaround, though it is a trick

@vtereshyn

This comment has been minimized.

Copy link

commented Jan 9, 2019

@willnew , oh, sorry, misunderstood.
Okay, but its really a trick, so, I will wait for new release from react-router.

@willnew

This comment has been minimized.

Copy link

commented Jan 10, 2019

@vtereshyn it's OK, I made a mistake that I didn't explain in detail in my first comment.
For others who worry about the case which @pierreferry mentioned, you can edit the demo code by entering
<Route exact path="/" render={props => <Home {...props} />} />
and
<Route exact path="/" component={props => <Home {...props} />} />
to see the paint flashing zone

@sunstorymvp

This comment has been minimized.

Copy link

commented Feb 11, 2019

another option is to extend proptype definition for Route component prop (if not removed on build):

// react-router-dom-fix.js
import PropTypes from 'prop-types';
import { Route } from 'react-router-dom';

// suppress prop-types warning on Route component when using with React.lazy
// until react-router-dom@4.4.0 or higher version released
/* eslint-disable react/forbid-foreign-prop-types */
Route.propTypes.component = PropTypes.oneOfType([
  Route.propTypes.component,
  PropTypes.object,
]);
/* eslint-enable react/forbid-foreign-prop-types */

should not have any side effects...

@HaiDang666

This comment has been minimized.

Copy link

commented Feb 13, 2019

I have the same in js code, but it not happen in ts.

@prakashtsi

This comment has been minimized.

Copy link

commented Mar 6, 2019

another option is to extend proptype definition for Route component prop (if not removed on build):

// react-router-dom-fix.js
import PropTypes from 'prop-types';
import { Route } from 'react-router-dom';

// suppress prop-types warning on Route component when using with React.lazy
// until react-router-dom@4.4.0 or higher version released
/* eslint-disable react/forbid-foreign-prop-types */
Route.propTypes.component = PropTypes.oneOfType([
  Route.propTypes.component,
  PropTypes.object,
]);
/* eslint-enable react/forbid-foreign-prop-types */

should not have any side effects...

This works for me. is it trustable @sunstorymvp ?

@sunstorymvp

This comment has been minimized.

Copy link

commented Mar 6, 2019

@prakashtsi works as expected. waiting for new release which is compatible with React.lazy to get rid of this code

@aramvr

This comment has been minimized.

Copy link

commented Apr 16, 2019

I just fixed it with updating react-dom to 16.6.3.
So before this the react version for my project is 16.6.3 and the react-dom version is 16.4.1. So I updated react-dom too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.