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

Reload routes #3770

Closed
sassanh opened this issue Aug 30, 2016 · 11 comments
Closed

Reload routes #3770

sassanh opened this issue Aug 30, 2016 · 11 comments

Comments

@sassanh
Copy link

sassanh commented Aug 30, 2016

Suppose that I'm using getComponent on some routes, when I'm using live reload, after reload is done it I need getComponent to run again (it'll call callback with the new component) so that it renders the new component. So I need a reload method on router which asks it to re-render the page (calling getComponent is part of it). It's discussed here: #1982 but that's a closed issue and it seems no one read my comment there. Let me know if I should provide additional information.

@sassanh
Copy link
Author

sassanh commented Aug 30, 2016

Currently I do this:

const location = this.context.store.getState().get('routing').get('locationBeforeTransitions').toJS();
this.props.dispatch(push('/'));
this.props.dispatch(push(location));

and I hate these 3 lines in my code. Using these lines introduce new problems for me on hot reloading which is beyond the scope of this thread.

@timdorr
Copy link
Member

timdorr commented Aug 30, 2016

That is currently the best way to do it. React is built around doing a noop if the state and props provided are the same between renders.

@timdorr timdorr closed this as completed Aug 30, 2016
@sassanh
Copy link
Author

sassanh commented Aug 30, 2016

Sorry @timdorr , I didn't understand why this feature request got rejected.

React is built around doing a noop if the state and props provided are the same between renders.

Even changing props of that component doesn't make it render the new component and the fact that changing location to '/' and then changing it back renders new component makes me think implementing this feature is feasible with current version of react and doesn't require any change in react library.

@taion
Copy link
Contributor

taion commented Aug 30, 2016

It's not rejected. This is a dupe of #2182.

@sassanh
Copy link
Author

sassanh commented Aug 30, 2016

This is a dupe of #2182.

No it's not. I don't want to change routes, I just want to re-render the page with my updated component. I hope some day core developers of react-router conclude that they should add it to the core library as I think react-router without reload is just like Chrome without reload.


For others who don't care if their whole project follows functional paradigm like me and have too much problems with the absence of forceReload (like me again) and found this thread: (it works with redux, if you're not using redux read the last paragraph.)

The method I described in my first comment is by no means best solution, not currently nor never. We want a method to call which re-renders the page with the new version of components (or you may have your own reasons to re-render page, I don't ask why, I don't try to teach you that it's wrong or right because I know nor me nor no one is in a position to do so, I don't try to convince you to solve everything by providing new props and states, I just respect your decision and I'm sure you have your own valid reasons and you have better considerations about your own projects than any others.) I were able to simulate the beloved imperative method this way:

If you're using redux and react-router-redux, supposing the component that needs to re-render is connected, just dispatch an action which increments a number (renderRound for example) in store. I tried it and it solved the issue for me, it re-renders smoothly without any side-effects.

If you're using bare react without redux. I guess adding a prop (again renderRound for example) to the Router component and incrementing it should solve the issue:

class Component extends React.Component {
    constructor() {
        this.state = {renderRound: false};
    }

    reRender() {
        this.setState({renderRound: !this.state.renderRound});
    }

    render() {
        return <blahblah>
            ...
            <div onClick={::this.reRender}>Rerender</div>
            ...
            <Router routes={...} history={...} renderRound={this.state.renderRound}/>
            ...
        </blahblah>
    }
}

If it didn't work just open ReactDevTools and try to find the outer most component in the hierarchy which doesn't re-render. Then try to convince it to re-render by changing a prop in it.
If you need this only in your dev environment you can consider editing react-router library in your node-modules folder to accomplish this (make its internal components pass the renderRound to the children so that they re-render.).

@timdorr
Copy link
Member

timdorr commented Aug 30, 2016

Why not just this.forceUpdate()? That would be a bit easier (no state to track).

@sassanh
Copy link
Author

sassanh commented Aug 30, 2016

Thanks for reply.

Sorry for long comment, I try to be descriptive.

Why not just this.forceUpdate()? That would be a bit easier (no state to track).

forceUpdate on the parent of <Router> doesn't work, I mentioned it here #1982 (comment) too. Let me describe the situation in more details:

Suppose this structure: (I use arrow functions just to simplify, the real project uses classes if that matters.)

const history = syncHistoryWithStore(browserHistory, store, {
    selectLocationState (state) {
        return state.get('routing').toJS();
    },
});

const modules = {
    component1: SomeComponent,
    etc,
};

routes = {
    childRoutes: {
        {
            component: X,
            getChildRoutes: (partialNextState, callback) => {
                callback(null, _.map(
                    loadedFromServerRoutes,
                    route => {
                        path: '/y',
                        getComponent: (partialNextState, callback) => {
                            if (!modules[route.compnentName].initialized) {} // initialize it, dispatch some actions, etc
                            callback(null, connect(CONNECTIONS, modules[route.componentName]));
                        }
                    },
                ));
            },
        },
    },
};

const ParentComponent = props => {
    <Router routes={routes} history={history}/>
};

React.render(<Provider><ParentComponent/></Provider>, document.getElementById('root-element'));

Now suppose that I change module.component from SomeComponent to another version of SomeComponent (the display name is the same, though I'm not sure if it's the problem.) Now calling forceUpdate, nor any solution discussed in that thread (#1982) doesn't make the NEW SomeComponent render. To me it seems like getComponent doesn't get called again (I don't remember if I checked it or not, I can check it, just let me know.)

Our behavior of not re-creating an element if the URL params change is related here. That is founded in basic React behavior: If the render function of a component provides the same set of elements back but with different props, this will not create a new instances, but will call componentWillRecieveProps on the existing instances instead. This is all idiomatic React behavior.

I thought it would be the case too.

After all consider that my data didn't change, the thing that is changed is the component itself. What should be done is 1) the reference of the component instance should get updated with the new instance of the new component. Which I think could be achieved if react-router calls getComponent again and update its reference with the new instance it getComponent provides by calling callback. 2) We should convince react to render it. (which shouldn't be hard, changing a dummy prop if there's not a clean way.)

Making router call getComponent in my opinion can be done by dispatching an action provided by react-router-redux or equivalently by calling a function (forceUpdate for example) react-router provides.

@sassanh
Copy link
Author

sassanh commented Aug 30, 2016

The code I provided above is just schematic and may have problems but I guess it gives you an idea about what's going on.

@sassanh
Copy link
Author

sassanh commented Aug 30, 2016

Meanwhile I appreciate if you reopen this issue till I'm convinced it's solved or is not solvable or is addressed in another open issue or at least you yourself (or any other repo owner) think you have provided enough information to convince me but for any reason I didn't get convinced.

@taion
Copy link
Contributor

taion commented Aug 30, 2016

You've already identified the antecedent issue here. This remains a duplicate issue.

@sassanh
Copy link
Author

sassanh commented Aug 30, 2016

You've already identified the antecedent issue here. This remains a duplicate issue.

That issue is closed, if that's the case then please open that issue and I'll migrate the conversation there.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 21, 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

3 participants