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
DrkCoater opened this Issue Dec 11, 2015 · 49 comments

Comments

@DrkCoater

DrkCoater commented Dec 11, 2015

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

This comment has been minimized.

Contributor

ryanflorence commented Dec 11, 2015

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

@taion

This comment has been minimized.

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 Dec 11, 2015

@EvHaus

This comment has been minimized.

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

This comment has been minimized.

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.

@DrkCoater

This comment has been minimized.

DrkCoater commented Dec 14, 2015

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>
@DrkCoater

This comment has been minimized.

DrkCoater commented Dec 15, 2015

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

@timdorr

This comment has been minimized.

Collaborator

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.

@DrkCoater

This comment has been minimized.

DrkCoater commented Dec 15, 2015

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

This comment has been minimized.

Collaborator

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

This comment has been minimized.

taylorstine commented Dec 21, 2015

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

This comment has been minimized.

jonstuebe commented Dec 27, 2015

@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

Fix store/history getting recreated due to HMR.
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:

ReactTraining/react-router#2704
@jpierson

This comment has been minimized.

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

This comment has been minimized.

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

This comment has been minimized.

bmamouri commented Jan 11, 2016

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

@timdorr

This comment has been minimized.

Collaborator

timdorr commented Jan 11, 2016

@bmamouri

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

This comment has been minimized.

CedricLor commented Jan 12, 2016

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

This comment has been minimized.

noahmiller commented Jan 12, 2016

@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

This comment has been minimized.

CedricLor commented Jan 12, 2016

@noahmiller Great idea. Thanks a lot.

@crashbell

This comment has been minimized.

crashbell commented Jan 13, 2016

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

This comment has been minimized.

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

This comment has been minimized.

crashbell commented Jan 21, 2016

thanks @SimeonC! It also works fine to me.

@mattdell

This comment has been minimized.

mattdell commented Jan 22, 2016

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

This comment has been minimized.

jpierson commented Jan 22, 2016

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

This comment has been minimized.

jpierson commented Jan 22, 2016

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

This comment has been minimized.

desduvauchelle commented Jan 24, 2016

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

This comment has been minimized.

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

This comment has been minimized.

Collaborator

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

This comment has been minimized.

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.

madeinfree added a commit to madeinfree/react-basic-starter that referenced this issue Jul 28, 2016

@bsr203

This comment has been minimized.

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

This comment has been minimized.

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

This comment has been minimized.

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

This comment has been minimized.

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

This comment has been minimized.

sterzhakov commented Oct 27, 2016

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

@peter-mouland

This comment has been minimized.

peter-mouland commented Nov 6, 2016

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

This comment has been minimized.

sterzhakov commented Nov 6, 2016

after hot reloading my states stayed the same

@fizal619

This comment has been minimized.

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

This comment has been minimized.

peter-mouland commented Nov 9, 2016

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

@supnate

This comment has been minimized.

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

This comment has been minimized.

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.

@p4bloch

This comment has been minimized.

p4bloch commented Dec 14, 2016

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

This comment has been minimized.

satyadeeproat commented Dec 27, 2016

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

This comment has been minimized.

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

Fix "You cannot change <Router routes>; it will be ignored" error mes…
…sage by implementing solution in Github: ReactTraining/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 added a commit to MyCryptoHQ/MyCrypto that referenced this issue Sep 8, 2017

Hot module reload fixes (#181)
* 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: ReactTraining/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`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment