remix-run/react-router

Keep getting an annoying warning message since version 1.0.1

Closed this issue Β· 49 comments

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.

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

taion commented

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.

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 commented

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

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>

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

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

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.

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.

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?

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

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

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.

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

@bmamouri

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

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>
    )
  }
})

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

@noahmiller Great idea. Thanks a lot.

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?

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

thanks @SimeonC! It also works fine to me.

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?

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.

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!

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 commented

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

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

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.

@mjackson and I were just talking about writing a custom renderer for route config, instead of simply stripping props. The idea was to allow route config to be composed the same way any components are composed, I wonder if it would also help us in this scenario ...

taion commented

@ryanflorence It will work to the extent that it can resolve #2182, but I think those two are somewhat orthogonal. I do think having a separate transition manager makes this easier, though.

To hot-swap routes it seems like we'd need to diff them to know if we need to update anything in the transition manager. If we actually "render" routes (with a custom renderer), we might get the diffing for free.

taion commented

We can always re-run the match if the routes change. I think the issue is actually the "leave" transition hooks.

FYI - Another workaround for this error message is to provide a unique key for Router so that it gets reconciled to a new instance. e.g.

let counter = 0;

class App extends React.Component {
    render() {
        return (
            <Router key={ counter } history={browserHistory}>
                <Route path="/" component={Main}>
                    <Route path="child" component={Child} />
                </Route>
            </Router>
        );
    }
}

ReactDOM.render(<App />, document.body.children[0]);

if (module.hot) {
    counter++;
    module.hot.accept();
}

For everyone still struggling with this message (unrelated to hot-reloading) - I found a solution that works pretty well without including any framework such as Redux or multiple event listenersinside each view component:
Don't pass any of your model to the routes. Trigger model updates in the component that you pass to your "/"-Route, i.e. your main view. Use React.cloneElement() to pass the model to all child route components.
Example:
routes.js

ReactDOM.render(
  <Router history={hashHistory}>
    <Route path="/" component={Main}>
      <IndexRoute component={Some} />
      <Route path="other" name="other" component={Other}>
      <Route path="route" name="view" component={Other}>
    </Route>
  </Router>,
app);

Main.js:

export default class Main extends React.Component {
  constructor(props) {
    super(props);
    this.state = { model: initialModel };
  }

  /*call this function from the outside, from event handlers, actions,
    electron main thread etc. Whatever way you want to set up your app. */
  updateModel(newModel) {
    this.setState({ model: newModel  });
  }

  renderChildren() {
    return React.Children.map(this.props.children, child =>
      React.cloneElement(child, this.state.model)
    );
  }

  render() {
    return (
      <div>
        { this.renderChildren() }
      </div>
    );
  }
}

Yes, the only reason you get this warning is because you re-render Router. You can either stop doing it and move setState (or connect in case of Redux) to the top level route handler.

Alternatively you can define your routes outside the root component’s render function:

var routes = <Route>...</Route>

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

Then even re-rendering that component won’t cause different routes to be passed.

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

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

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 commented

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…

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

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

after hot reloading my states stayed the same

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

great to hear, must be my app having state problems then πŸ€”

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.

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

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} />
  }
}

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>
    );
  };
};

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);
    });
}