gor181 / react-notification-system-redux

Redux wrapper for react-notification-system
MIT License
324 stars 59 forks source link

Notification not displayed after route change #42

Open j-gurda opened 7 years ago

j-gurda commented 7 years ago

I'm currently investigating issue related to notification not being displayed after route change. Following scenario describes the case: 1) I have separate route which presents form for adding new entity 2) When I click "Save" button AJAX request is dispatched and on success of that request I dispatch new notification and immediately change route using history.push('/some-url')

Following pseudo-code presents that:

function onSuccess(data){
   dispatch(Notifications.success( .... ));
   history.push('/some-url');
}

3) After route is changed notification is not displayed however notification is still inside Redux store. Also no RNS_HIDE_NOTIFICATION is fired. 4) When I invoke an action which displays another notification but does not change the route "old" notifications are displayed too what looks like queuing up of notifications.. All notifications disappear after expected time.

Here's list of dependencies:

    "axios": "^0.16.2",
    "common-tags": "^1.4.0",
    "immutability-helper": "^2.3.0",
    "jwt-decode": "^2.2.0",
    "prop-types": "^15.5.10",
    "query-string": "^5.0.0",
    "react": "^15.5.4",
    "react-autosize-textarea": "^0.4.8",
    "react-bootstrap": "^0.31.0",
    "react-bootstrap-typeahead": "^2.0.0-alpha.1",
    "react-collapsible": "^1.5.0",
    "react-cookies": "0.0.1",
    "react-dom": "^15.5.4",
    "react-html-id": "^0.1.1",
    "react-notification-system": "^0.2.14",
    "react-notification-system-redux": "^1.1.4",
    "react-numeric-input": "^2.0.7",
    "react-redux": "^5.0.5",
    "react-router-bootstrap": "^0.24.2",
    "react-router-dom": "^4.1.1",
    "redux": "^3.6.0",
    "redux-form": "^6.7.0",
    "redux-thunk": "^2.2.0"

I'm relatively new to React and Redux and I'm trying to build minimal app which reproduces that issue but maybe you could figure out faster what's going on.

j-gurda commented 7 years ago

I found cause of an issue. I used Route component higher in component hierarchy than Notifications component. That caused need of mounting Notifications component every time route was changed. Here is sample pseudo code:

<Route exact 
    path="/" 
    component={DashboardPage}>
    <Notifications
           notifications={notifications}/>
</Route>

In source code of Notifications you refresh messages only on componentWillReceiveProps so when Notifications is freshly mounted componentWillReceiveProps is not called.

There are two possible solutions for that: 1) Use Notifications above Route component so changing route does not cause remounting of it:

<Notifications
           notifications={notifications}/>
<Route exact 
    path="/" 
    component={DashboardPage}/>   
<Route exact 
    path="/some_other_url" 
    component={OtherPage}/>  

2) Patch Notifications component to update notifications also on componentDid(Will)Mount.

Thank you for great piece of software!

gor181 commented 7 years ago

Thank you for reporting the detailed use case, I'm really glad that you are finding the lib as useful. I agree that we should add behavior of updating the notifications on will/didmount as you suggested.