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

Keep getting an annoying warning message since version 1.0.1 #2704

Closed
HungryJake opened this issue Dec 11, 2015 · 49 comments
Closed

Keep getting an annoying warning message since version 1.0.1 #2704

HungryJake opened this issue Dec 11, 2015 · 49 comments

Comments

@HungryJake
Copy link

Hi,
I'm keep getting an annoying warning message in the browser console after I upgrade to version 1.0.1 or 1.0.2, Every time I navigate to a different route...:
Warning: You cannot change ; it will be ignored
I've checked and made sure that routes prop nor the children prop of the Router has not been modified, (maybe react is doing some internal things)...
Does not happen in version 1.0.0 or lower, force me to keep react-router to be 1.0.0...
Is this a bug?

Thank you.

@ryanflorence
Copy link
Member

Can you copy/paste your code that renders the <Router/>

@taion
Copy link
Contributor

taion commented Dec 11, 2015

The warning was a previously missing one - something you are doing is changing the set of routes passed into your <Router>, and we currently don't support that. This is to let you know that you are doing something potentially dangerous and wrong.

@taion taion closed this as completed Dec 11, 2015
@EvHaus
Copy link

EvHaus commented Dec 14, 2015

I too just started getting this warning after upgrading to 1.0.2. It occurs immediately after webpack's hot loader reloads the modules after an update.

I don't see anything unusual about how my Router is defined:

export default class App extends Component {
    render () {
        return (
            <Router history={history}>
                <Route component={AppView} path="/">
                    <IndexRoute component={Home} />
                    <Route component={Home} path="/home" />
                    <Route component={Changelog} path="/changelog" />
                    <Route component={Loader} path="/:type/:component" />
                    <Route component={FourOhFour} path="*" />
                </Route>
            </Router>
        );
    }
}

render(<App />, document.getElementById('App'));

Everything continues to work as normal, but the warning pops up after any module is hot reloaded.

@taion
Copy link
Contributor

taion commented Dec 14, 2015

Hot reloading routes doesn't work, I'm pretty sure, so that sounds like an appropriate error in context.

@HungryJake
Copy link
Author

Thanks for your reply and sorry about not getting back in time.
I don' think I'm doing anything special there. First I thought it was because I was passing context down from parent, (I've move context from Router to Canvas for testing) but even I've remove context it still happening. Then I tried to create just a simple test component to route, it still can reproduce this warning... I'm also using webpack by the way.
Here's my route setup:

<Canvas context={state.context} id="canvas">
  <Router>
    <Route path="/" component={App}>
      <IndexRoute component={HomePage}/>
      <Route path="/testcomp" component={TestComp}/>
      <Route path="login" component={LoginPage}/>
      <Route path="register" component={RegisterPage}/>
      <Route path="*" component={ContentPage}/>
    </Route>
  </Router>
</Canvas>

@HungryJake
Copy link
Author

Btw I'm not using hot reloading... I'm using react-transform, which seems to work fine for me.

@timdorr
Copy link
Member

timdorr commented Dec 15, 2015

@DrkCoater react-transform is a tool that is built on hot reloading (the module.hot API, specifically). It only supports the hot reloading of pure React components. react-router uses objects and functions that exist outside of React's component infrastructure, so react-transform can't manage their reloading. We are looking to add hot reload support into router in a future version, though.

@HungryJake
Copy link
Author

thanks. I still don't quite understand what taion meant by "Hot reloading routes doesn't work,"... Is that mean Hot reloading not working so it's causing warning message appear? Trying to identify whether it's a configuration issue or coding issue now.

@timdorr
Copy link
Member

timdorr commented Dec 15, 2015

Hot reloading of routes doesn't yet work. We don't have any mechanism or API for replacing routes in place. So changing your route config will not be reloaded into your app during development until you refresh the full page.

@taylorstine
Copy link

I want to update my routing logic along with the state of my app (I'm using redux). So say I have a list of todos in my store, and each todo has it's own page. I want to do a check in the component prop of the /todo/:id route to see if my state contains that todo, and render an error page if it does not. I have this working, but each time the state is updated, this warning shows up.

Is this something react-router is not intended to handle? If not, do you have any recommendations?

@jonstuebe
Copy link

@taion so does this mean that if I change absolutely anything inside of a component that is being hot loaded that it will always "warn" me now due to it being inside of a route?

tec27 added a commit to ShieldBattery/ShieldBattery that referenced this issue Dec 28, 2015
This fixes warnings/errors about the store and history being
recreated when HMR happens by moving their creation up a level.
Almost all changes in the app will be caught at or before app.jsx,
so they'll never reach index.jsx and thus never re-run that code.

Note that there will still be errors if you actually change the
routes (and will require a full reload), but there's not much
we can do about that right now. See:

remix-run/react-router#2704
@jpierson
Copy link

jpierson commented Jan 9, 2016

@jonstuebe That seems to be the behavior that I see too and it isn't very useful if I understand all of the points here.

The warning was a previously missing one - something you are doing is changing the set of routes passed into your Router, and we currently don't support that. This is to let you know that you are doing something potentially dangerous and wrong.

So according to @taion it sounds like the warning was only intended to show when the list of routes have changed and not necessarily the components to which those routes lead. If this is the case then perhaps a new issue needs to be opened to report that the warnings are showing when they are unintended. I think the fact that this issue used the word "annoying" that it may be safe to assume that original issue here was not complaining about a useful warning that would result from actually making a change directly to the routes.

This warning happens to be the last in a list of frequent warnings in an app I'm developing and it causes a bunch of noise so I'm eager to resolve what ever underlying issue is causing them.

@taion
Copy link
Contributor

taion commented Jan 9, 2016

Things will break whenever the set of routes breaks referential identity. Just don't hot reload your routes.js or whatever – it will do no good.

@bmamouri
Copy link

@taion how prevent routes.js from hot reloading? Is this possible with react-hot-loader?

@timdorr
Copy link
Member

timdorr commented Jan 11, 2016

@bmamouri

if (module.hot) {
  module.hot.decline("./routes.js");
}

@CedricLor
Copy link

I am having the same issue here, but with React-Intl v2.0.0-beta-2.

If I update the locale of the new <IntlProvider> provided by React-Intl which is wrapping the <Router>, the router thinks that I am trying to hot reload the Routes (or maybe React-Intl is effectively hot reloading the Routes, but I think it is only updating the context).

By the way, it does not seem to be preventing the app, React-Intl and the router to keep on working as expected. It is just spitting out the warning.

Here is my code:

const Root = React.createClass({
  PropTypes: {
    siteCurrentLocale: PropTypes.string.isRequired
  },

  render () {
    const intlData = {
      locale:   props.siteCurrentLocale,
      messages: i18n[props.siteCurrentLocale]
    }

    return (
      <IntlProvider key="intl" {...intlData}>
        <Router history={ history } >
          <Route path="/(:locale/)" component={ App } >
            <IndexRoute component={ NewsCardsContainer }/>
            <Route path="/(:locale/)articles" component={ NewsCardsContainer }/>
            <Route path="/(:locale/)article/:id" component={ IndividualNewsContainer }/>
          </Route>
        </Router>
      </IntlProvider>
    )
  }
})

@noahmiller
Copy link

@CedricLor: we worked around the same issue by defining the routes in a constant:
https://github.com/rackt/react-router/blob/latest/docs/guides/basics/RouteConfiguration.md#alternate-configuration

For example:

const routeConfig = [
  { path: '/:locale',
    component: App,
    indexRoute: { component: NewsCardsContainer },
    ...
  }
];

return (
  <IntlProvider key="intl" {...intlData}>
    <Router history={history} routes={routeConfig} />
  </IntlProvider>
)

@CedricLor
Copy link

@noahmiller Great idea. Thanks a lot.

@crashbell
Copy link

I'm running into the same problem, please view my sample code below. I use onEnter to handle authentication so I can navigate to correct pages base on the authenticated flag:

_requireAnonymous(nextState, replaceState) {
  if (isAuthenticated) {
    replaceState({nextPathname: nextState.location.pathname}, '/dashboard')
  }
  return true
}

<Router history={history}>
  <Route path='/.' component={Loader}/>
  <Route component={Main}>
    <Route path='/' component={Container}>
      <IndexRoute component={Default} onEnter={::this._requireAnonymous}/>
      <Route path='login' component={Login} onEnter={::this._requireAnonymous}/>
      <Route path='register' component={Register} onEnter={::this._requireAnonymous}/>
      <Route path='dashboard' component={dashboard}/>
    </Route>
    <Route path='/logout' onEnter={this.props.logout} />
  </Route>
</Router>

This LoC has checked that the Router's children were changed, if I remove onEnter handler, the warning will disappear. Seems like I'm doing the authentication checker in a wrong way, do you have any suggestion for me?

@SimeonC
Copy link

SimeonC commented Jan 21, 2016

@crashbell I've run into the same error but it seems the problem is if I have the component wrapped in a react-redux connect export default connect(state => state)(App) where App is defined as in you mentioned. In this case @noahmiller's solution worked though I had to define the routes config in the constructor if your using the class syntax.

@crashbell
Copy link

thanks @SimeonC! It also works fine to me.

@mattdell
Copy link

Strangely this was happening because I had a reducer called application declared in my Redux store.

I changed it from
export {default as application} from './ApplicationReducer';

to
export {default as app} from './ApplicationReducer';

and that fixed the issue for me with routes being re-rendered. Is application some sort of reserved name?

@jpierson
Copy link

I'm having this issue in a react-redux scenario as well and I'm guessing that that is my problem although I'm not sure if I understand white how to follow @noahmiller's workaround example to move from Route components to a configuration. I don't want to screw something up in the process just to work around the warning.

@jpierson
Copy link

Oh, just changing this below seemed to do the same trick for me.

const routes = 
    <Route path="/" component={App}>
        <IndexRedirect to="default"/>
        <Route name="one" path="one" component={One}/>
        <Route name="two" path="two" component={Two}/>
        <Route name="three" path="three" component={Three}/>
        <Route path="*" component={NotFound} />
    </Route>;

    return (
      <Provider store={store}>
        <div>
            <Router history={history}>
                {routes}
            </Router>
            {__DEV__ && <DevTools />}
        </div>  
      </Provider>
    );

Thanks @noahmiller!

@desduvauchelle
Copy link

Same issue but only when I call parent handling functions.

Example. My component holds data like the current logged user, current project, ...
getInitialState: function (){ return { project: {}, user: {} },
As well as handlers...so that my whole UI updates depending on the state...ex:
setProject: function(newProject){ this.setState({project:newProject}); }

<Router>
        <Route path="/" component={Layout} {...this.state} setProject={this.setProject}>
            <IndexRoute name="projects" component={Projects}></IndexRoute>
            <Route path="/product/:productId/:view" name="product" component={Product} />
             <Route path="*" component={NotFound}/>
        </Route>
</Router>

As soon as I call the setProject in the my child component, I get the error...
If you go for defining your ROUTES as a constant, how do you pass states and handlers?

EX

const routesConfig =  (
    <Router history={browserHistory}>
        <Route path="/" component={Layout} {...this.state} setProduct={this._setProduct}>
            <IndexRoute name="products" component={Products}></IndexRoute>
            <Route path="/product/:productId/:view" name="product" component={Product} />
            <Route path="scrum" name="scrum" component={Scrum}></Route>

            <Route path="login" component={Login}></Route>
            <Route path="register" component={Register}></Route>
            <Route path="forgotten" component={Forgotten}></Route>

             <Route path="*" component={NotFound}/>
        </Route>
    </Router>
);

this in the example above is not defined. Any ideas?

@taion
Copy link
Contributor

taion commented Jan 25, 2016

@rackt/redux Do you have any recommendations for what to do here? react-redux <Provider> makes the same check, with a caveat that it only warns once. Should we just do that here?

@timdorr
Copy link
Member

timdorr commented Jan 25, 2016

@desduvauchelle You should be connecting your components to the store set via <Provider>. You don't need to, nor should you, try to pass them as props via <Route>. The connect function provides you with a lot of rendering optimizations specific to Redux and pulling data from state.

@taion It appears to be because most people are defining and rendering their routes within a component. And, as is usually the case, they have react-transform set up, which is re-rendering their routes repeatedly, causing this error to trip. Routes should be defined outside of a component. I put them into a separate file, for example. This route config should not include the <Router>, as you will need to define that separately for client and server contexts. I have this, verbatim, in one of my apps working just fine:

ReactDOM.render(
  (
    <Provider store={store}>
      <Router history={browserHistory} children={routes(store)} />
    </Provider>
  ),
  document.getElementById('root')
);

Everything below that is my app code and gets hot reloaded by react-transform just fine.

@wwayne
Copy link

wwayne commented Jan 29, 2016

Same situation like @globexdesigns. Have you solved this? For me, the error still there even I separate routes into a single file and module.hot.decline that file.

@bsr203
Copy link

bsr203 commented Jul 30, 2016

@gaearon I am setting everything const, still I get error.

const routes = createRoutes(routeConf);
const history = syncHistoryWithStore(browserHistory, store);
const router = (<Router
  history={history}
  render={applyRouterMiddleware(useRelay)}
  environment={Relay.Store}
>
  {routes}
</Router>
);

ReactDOM.render(<AppContainer><Root store={store} router={router} /></AppContainer>, rootEl);

the error is at the first check on history

/* istanbul ignore next: sanity check */
  componentWillReceiveProps: function componentWillReceiveProps(nextProps) {
    process.env.NODE_ENV !== 'production' ? (0, _routerWarning2.default)(nextProps.history === this.props.history, 'You cannot change <Router history>; it will be ignored') : void 0;

    process.env.NODE_ENV !== 'production' ? (0, _routerWarning2.default)((nextProps.routes || nextProps.children) === (this.props.routes || this.props.children), 'You cannot change <Router routes>; it will be ignored') : void 0;
  },
nextProps.history === this.props.history

what can I do to avoid this?? thanks.

@bsr203
Copy link

bsr203 commented Jul 30, 2016

@gaearon after playing around, adding a specific handler like this seems solved the issue

  module.hot.accept(Root, () => {
    console.warn('hot! ./Root');
    ReactDOM.render(<AppContainer><Root store={store} router={router} /></AppContainer>, rootEl);
  });

thanks.

@aTable
Copy link

aTable commented Oct 14, 2016

Remembering the original problem that state is being modified at the router level where the routes are dependent on state, here's another option:

<Router key={new Date()} ... >

@ESWAT
Copy link

ESWAT commented Oct 18, 2016

Cleared up the errors on my end. FWIW this is what my index/main looks like:

const root = document.getElementById('root');

const renderApp = () => {
  const routes = require('./routes').default;

  render(
    <AppContainer>
      <Provider {...stores} children={routes} />
    </AppContainer>,
    root
  );
};

renderApp();

if (module.hot) {
  module.hot.accept('./routes', () => {
    unmountComponentAtNode(root);
    renderApp();
  });

For routes:

const routes = (
  <Router history={browserHistory}>
    <Route path="/" component={App}>
      <IndexRoute getComponent={(location, cb) => {
        require.ensure([], (require) => {
          cb(null, require('~/views/Home'));
        });
      }}
      />
      <Route path="/compose" getComponent={(location, cb) => {
        require.ensure([], (require) => {
          cb(null, require('~/views/Compose'));
        });
      }}
      etc…

@sterzhakov
Copy link

I found a very simple solution:
<Router key={Math.random()} history={browserHistory} >...</Router>

@peter-mouland
Copy link

does providing a key not cause you to lose state?

I have added they key to a project, https://github.com/peter-mouland/react-lego#react-hot-loader-v3, and when hot-reloading is complete state is lost. Without the key i do get the warning, but hot-reload completes keeping state intact

@sterzhakov
Copy link

after hot reloading my states stayed the same

@fizal619
Copy link

fizal619 commented Nov 9, 2016

@frankwallis's Solution worked for me. Adding a unique key to the router.

let counter = 0
const App=(props)=>{
    counter++
        return (
        <Router key={counter} history={browserHistory}>

edit:

changed it to Math.random() @sterjakovigor thanks!

@peter-mouland
Copy link

great to hear, must be my app having state problems then 🤔

@supnate
Copy link

supnate commented Nov 17, 2016

Here is my workaround, just adding one line:

import React from 'react';
import { Provider } from 'react-redux';
import { Router } from 'react-router';
import routeConfig from '../common/routeConfig';

export default class Root extends React.Component {
  render() {
    const { store, history } = this.props; // eslint-disable-line
    if (!this.routeConfig) this.routeConfig = routeConfig;

    return (
      <Provider store={store}>
        <Router history={history} routes={this.routeConfig} />
      </Provider>
    );
  }
}

I looked at the source code: https://github.com/ReactTraining/react-router/blob/6eeb7ad358f987520f5b519e48bdd31f725cbade/modules/Router.js , the warning is just logged when routes changes, so just always pass the first one to it.

I guess this has better performance than adding a key which causes the page fully refreshed on every update.

@ilyabo
Copy link

ilyabo commented Nov 18, 2016

@supnate Thanks for this workaround! It not only has better performance, but it actually works. Because the Math.random() workaround effectively invalidates the whole tree of components and hence prevents the state from being retained upon a hot reload.

@pechitook
Copy link

My two cents. Based on @gaearon's comment, you will actually have to send an array of Routes to avoid JSX warning that you must have a root element.

const routes = [
  <Route path="/" component={Home} />,
  <Route path"/blog" component={Blog} />
]

class Root extends Component {
  render() {
    return <Router routes={routes} />
  }
}

@satyadeeproat
Copy link

I want to pass props to routes. I am doing it this way but getting error.
Failed prop type: Invalid prop routes supplied to Router in Router
Anyone knows what I am doing wrong

const routeConfig = (type) => {
  return (      
  <Route>
  <Route path='/' component={Template}>
    <Route path='/pageform(/:id)' component={PageForm} />
  </Route>
  
  <Route path='/:pageSlug' component={Page} type=type/>
  </Route>
);
}
class App extends React.Component {
  render() {
    return (
      <MuiThemeProvider muiTheme={getMuiTheme()}>
        <Provider store={store}>
          <Router history={browserHistory} routes={routeConfig type='abc'} />
        </Provider>
      </MuiThemeProvider>
    );
  };
};

@JasonKid
Copy link

JasonKid commented Jan 6, 2017

I think this will be much better for various environment:

import React from 'react';
import ReactDOM from 'react-dom';
import {AppContainer} from 'react-hot-loader';
import Router from './Router';

const render = (Component) => {
    ReactDOM.render(
        <AppContainer>
            <Component key={module.hot ? new Date() : undefined}/>
        </AppContainer>,
        document.getElementById('root')
    );
};

render(Router);

if(module.hot) {
    module.hot.accept('./Router', () => {
        render(Router);
    });
}

dternyak added a commit to MyCryptoHQ/MyCrypto that referenced this issue Sep 8, 2017
…sage by implementing solution in Github: remix-run/react-router#2704 (comment)

Router is only instantiated once in a production setting (e.g. not webpack-dev-server), so there is minimal overhead on producing a random key value for `Router`.
dternyak pushed a commit to MyCryptoHQ/MyCrypto that referenced this issue Sep 8, 2017
* accept hot module changes, move routes into root component

* Fix "You cannot change <Router routes>; it will be ignored" error message by implementing solution in Github: remix-run/react-router#2704 (comment)

Router is only instantiated once in a production setting (e.g. not webpack-dev-server), so there is minimal overhead on producing a random key value for `Router`.
@lock lock bot locked as resolved and limited conversation to collaborators Jan 19, 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

No branches or pull requests