From caea02a322355cbd3dbfa33096bcd7acb6e18ea1 Mon Sep 17 00:00:00 2001 From: Lyubo Marinov Date: Mon, 31 Jul 2017 19:34:19 -0500 Subject: [PATCH] [RN] Mitigate 'Not joining a new URL while in a conference' --- react/features/app/actions.js | 13 +++++++------ react/features/app/components/AbstractApp.js | 20 +++++++++++++++----- react/features/app/components/App.web.js | 8 +++++++- react/features/app/middleware.js | 10 ++++------ 4 files changed, 33 insertions(+), 18 deletions(-) diff --git a/react/features/app/actions.js b/react/features/app/actions.js index 5aa793472..43731027d 100644 --- a/react/features/app/actions.js +++ b/react/features/app/actions.js @@ -47,8 +47,8 @@ function _appNavigateToMandatoryLocation( const newHost = newLocation.host; if (oldHost === newHost) { - dispatchSetLocationURL(); - dispatchSetRoom(); + dispatchSetLocationURL() + .then(dispatchSetRoom); } else { // If the host has changed, we need to load the config of the new host // and set it, and only after that we can navigate to a different route. @@ -80,8 +80,9 @@ function _appNavigateToMandatoryLocation( return; } - dispatchSetLocationURL(); - dispatch(setConfig(config)); + return ( + dispatchSetLocationURL() + .then(() => dispatch(setConfig(config)))); } /** @@ -90,7 +91,7 @@ function _appNavigateToMandatoryLocation( * @returns {void} */ function dispatchSetLocationURL() { - dispatch(setLocationURL(new URL(newLocation.toString()))); + return dispatch(setLocationURL(new URL(newLocation.toString()))); } /** @@ -99,7 +100,7 @@ function _appNavigateToMandatoryLocation( * @returns {void} */ function dispatchSetRoom() { - dispatch(setRoom(newLocation.room)); + return dispatch(setRoom(newLocation.room)); } } diff --git a/react/features/app/components/AbstractApp.js b/react/features/app/components/AbstractApp.js index 38dd53542..f77422126 100644 --- a/react/features/app/components/AbstractApp.js +++ b/react/features/app/components/AbstractApp.js @@ -184,7 +184,7 @@ export class AbstractApp extends Component { * @returns {ReactElement} */ render() { - const route = this.state.route; + const { route } = this.state; if (route) { return ( @@ -348,15 +348,14 @@ export class AbstractApp extends Component { * Navigates to a specific Route. * * @param {Route} route - The Route to which to navigate. - * @returns {void} + * @returns {Promise} */ _navigate(route) { if (RouteRegistry.areRoutesEqual(this.state.route, route)) { - return; + return Promise.resolve(); } let nextState = { - ...this.state, route }; @@ -375,7 +374,18 @@ export class AbstractApp extends Component { nextState = undefined; }); - nextState && this.setState(nextState); + // XXX React's setState is asynchronous which means that the value of + // this.state.route above may not even be correct. If the check is + // performed before setState completes, the app may not navigate to the + // expected route. In order to mitigate the problem, _navigate was + // changed to return a Promise. + return new Promise(resolve => { + if (nextState) { + this.setState(nextState, resolve); + } else { + resolve(); + } + }); } /** diff --git a/react/features/app/components/App.web.js b/react/features/app/components/App.web.js index e62a1738e..4a0a8f770 100644 --- a/react/features/app/components/App.web.js +++ b/react/features/app/components/App.web.js @@ -84,18 +84,24 @@ export class App extends AbstractApp { // Navigate to the specified Route. const windowLocation = this.getWindowLocation(); + let promise; if (!route || windowLocation.pathname === path) { // The browser is at the specified path already and what remains is // to make this App instance aware of the route to be rendered at // the current location. - super._navigate(route); + + // XXX Refer to the super's _navigate for an explanation why a + // Promise is returned. + promise = super._navigate(route); } else { // The browser must go to the specified location. Once the specified // location becomes current, the App will be made aware of the route // to be rendered at it. windowLocation.pathname = path; } + + return promise || Promise.resolve(); } /** diff --git a/react/features/app/middleware.js b/react/features/app/middleware.js index bde2c8b6a..589f2410b 100644 --- a/react/features/app/middleware.js +++ b/react/features/app/middleware.js @@ -103,7 +103,7 @@ function _navigate({ dispatch, getState }) { } } - app._navigate(routeToRender); + return app._navigate(routeToRender); } /** @@ -121,11 +121,9 @@ function _navigate({ dispatch, getState }) { * specified {@code action}. */ function _setLocationURL({ getState }, next, action) { - const result = next(action); - - getState()['features/app'].app._navigate(undefined); - - return result; + return ( + getState()['features/app'].app._navigate(undefined) + .then(() => next(action))); } /**