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 · Fixed by gladly-team/tab#457
Closed

React.lazy makes Route's proptypes fail #6420

jgoux opened this issue Oct 24, 2018 · 28 comments · Fixed by gladly-team/tab#457

Comments

@jgoux
Copy link

jgoux 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
Copy link

nfvs commented Oct 24, 2018

Same thing with React.memo().

@timdorr
Copy link
Member

timdorr commented Oct 24, 2018

Likely fixed by #6327 and/or #6417.

@shawyu
Copy link

shawyu 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
Copy link

rnnyrk 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
Copy link
Member

timdorr commented Oct 26, 2018

Wait for a release :)

@tylerwray
Copy link

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

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

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

@jadbox
Copy link

jadbox 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
Copy link
Member

timdorr commented Nov 1, 2018

This was fixed in beta.5.

@selbekk
Copy link
Contributor

selbekk 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
Copy link
Contributor

frehner commented Nov 8, 2018

#6447 :)

@selbekk
Copy link
Contributor

selbekk commented Nov 8, 2018

Ah, sorry!

@pierreferry
Copy link

@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
Copy link
Contributor

pshrmn commented Nov 26, 2018

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

@wulucxy
Copy link

wulucxy commented Dec 20, 2018

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

@vtereshyn
Copy link

vtereshyn commented Dec 29, 2018

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

@semoal
Copy link

semoal commented Jan 2, 2019

Same here, any updates?

@gunn4r
Copy link

gunn4r 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
Copy link

willnew commented Jan 9, 2019

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

@vtereshyn
Copy link

@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
Copy link

pierreferry 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
Copy link

willnew 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
Copy link

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

@willnew
Copy link

willnew 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
Copy link

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
Copy link

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

@prakashtsi
Copy link

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
Copy link

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

@aramvr
Copy link

aramvr 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.

@lock lock bot locked as resolved and limited conversation to collaborators Jun 15, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.