A bug was discovered in d17cc9fa which would raise a failure to push
into the browser's history if a base href was defined. Fix the failure
by removing react-router. Anyway, the usage of react-router was
incorrect because the app must hit the server infrastructure when it
enters a room because the server will choose the very app version then.
This commit is contained in:
Lyubomir Marinov 2017-01-15 18:28:02 -06:00
parent 856732abab
commit ba3d65c01f
6 changed files with 113 additions and 203 deletions

View File

@ -39,8 +39,6 @@
"react-native-vector-icons": "^3.0.0",
"react-native-webrtc": "jitsi/react-native-webrtc",
"react-redux": "^5.0.2",
"react-router": "^3.0.0",
"react-router-redux": "^4.0.7",
"redux": "^3.5.2",
"redux-thunk": "^2.1.0",
"retry": "0.6.1",

View File

@ -1,4 +1,5 @@
import React, { Component } from 'react';
import { Provider } from 'react-redux';
import {
localParticipantJoined,
@ -32,6 +33,25 @@ export class AbstractApp extends Component {
url: React.PropTypes.string
}
/**
* Initializes a new App instance.
*
* @param {Object} props - The read-only React Component props with which
* the new instance is to be initialized.
*/
constructor(props) {
super(props);
this.state = {
/**
* The Route rendered by this App.
*
* @type {Route}
*/
route: undefined
};
}
/**
* Init lib-jitsi-meet and create local participant when component is going
* to be mounted.
@ -62,6 +82,28 @@ export class AbstractApp extends Component {
dispatch(appWillUnmount(this));
}
/**
* Implements React's {@link Component#render()}.
*
* @inheritdoc
* @returns {ReactElement}
*/
render() {
const route = this.state.route;
if (route) {
return (
<Provider store = { this.props.store }>
{
this._createElement(route.component)
}
</Provider>
);
}
return null;
}
/**
* Create a ReactElement from the specified component, the specified props
* and the props of this AbstractApp which are suitable for propagation to
@ -161,6 +203,58 @@ export class AbstractApp extends Component {
return undefined;
}
/**
* Navigates to a specific Route.
*
* @param {Route} route - The Route to which to navigate.
* @returns {void}
*/
_navigate(route) {
let nextState = {
...this.state,
route
};
// The Web App was using react-router so it utilized react-router's
// onEnter. During the removal of react-router, modifications were
// minimized by preserving the onEnter interface:
// (1) Router would provide its nextState to the Route's onEnter. As the
// role of Router is now this AbstractApp, provide its nextState.
// (2) A replace function would be provided to the Route in case it
// chose to redirect to another path.
this._onRouteEnter(route, nextState, pathname => {
// FIXME In order to minimize the modifications related to the
// removal of react-router, the Web implementation is provided
// bellow because the replace function is used on Web only at the
// time of this writing. Provide a platform-agnostic implementation.
// It should likely find the best Route matching the specified
// pathname and navigate to it.
window.location.pathname = pathname;
// Do not proceed with the route because it chose to redirect to
// another path.
nextState = undefined;
});
nextState && this.setState(nextState);
}
/**
* Notifies this App that a specific Route is about to be rendered.
*
* @param {Route} route - The Route that is about to be rendered.
* @private
* @returns {void}
*/
_onRouteEnter(route, ...args) {
// Notify the route that it is about to be entered.
const onEnter = route.onEnter;
if (typeof onEnter === 'function') {
onEnter(...args);
}
}
/**
* Navigates this AbstractApp to (i.e. opens) a specific URL.
*

View File

@ -1,12 +1,9 @@
/* global __DEV__ */
import React from 'react';
import { Linking, Navigator } from 'react-native';
import { Provider } from 'react-redux';
import { Linking } from 'react-native';
import { Platform } from '../../base/react';
import { _getRouteToRender } from '../functions';
import { AbstractApp } from './AbstractApp';
/**
@ -32,7 +29,6 @@ export class App extends AbstractApp {
super(props);
// Bind event handlers so they are only bound once for every instance.
this._navigatorRenderScene = this._navigatorRenderScene.bind(this);
this._onLinkingURL = this._onLinkingURL.bind(this);
// In the Release configuration, React Native will (intentionally) throw
@ -70,47 +66,6 @@ export class App extends AbstractApp {
super.componentWillUnmount();
}
/**
* Implements React's {@link Component#render()}.
*
* @inheritdoc
* @returns {ReactElement}
*/
render() {
const store = this.props.store;
/* eslint-disable brace-style, react/jsx-no-bind */
return (
<Provider store = { store }>
<Navigator
initialRoute = { _getRouteToRender(store.getState) }
ref = { navigator => { this.navigator = navigator; } }
renderScene = { this._navigatorRenderScene } />
</Provider>
);
/* eslint-enable brace-style, react/jsx-no-bind */
}
/**
* Navigates to a specific Route (via platform-specific means).
*
* @param {Route} route - The Route to which to navigate.
* @returns {void}
*/
_navigate(route) {
const navigator = this.navigator;
// TODO Currently, the replace method doesn't support animation. Work
// towards adding it is done in
// https://github.com/facebook/react-native/issues/1981
// XXX React Native's Navigator adds properties to the route it's
// provided with. Clone the specified route in order to prevent its
// modification.
navigator && navigator.replace({ ...route });
}
/**
* Attempts to disable the use of React Native
* {@link ExceptionsManager#handleException} on platforms and in
@ -149,24 +104,6 @@ export class App extends AbstractApp {
}
}
/**
* Renders the scene identified by a specific route in the Navigator of this
* instance.
*
* @param {Object} route - The route which identifies the scene to be
* rendered in the associated Navigator. In the fashion of NavigatorIOS, the
* specified route is expected to define a value for its component property
* which is the type of React component to be rendered.
* @private
* @returns {ReactElement}
*/
_navigatorRenderScene(route) {
// We started with NavigatorIOS and then switched to Navigator in order
// to support Android as well. In order to reduce the number of
// modifications, accept the same format of route definition.
return this._createElement(route.component, {});
}
/**
* Notified by React's Linking API that a specific URL registered to be
* handled by this App was activated.

View File

@ -1,10 +1,3 @@
import React from 'react';
import { Provider } from 'react-redux';
import { browserHistory, Route, Router } from 'react-router';
import { push, replace, syncHistoryWithStore } from 'react-router-redux';
import { RouteRegistry } from '../../base/navigator';
import { appInit } from '../actions';
import { AbstractApp } from './AbstractApp';
@ -21,26 +14,6 @@ export class App extends AbstractApp {
*/
static propTypes = AbstractApp.propTypes
/**
* Initializes a new App instance.
*
* @param {Object} props - The read-only React Component props with which
* the new instance is to be initialized.
*/
constructor(props) {
super(props);
/**
* Create an enhanced history that syncs navigation events with the
* store.
* @link https://github.com/reactjs/react-router-redux#how-it-works
*/
this.history = syncHistoryWithStore(browserHistory, props.store);
// Bind event handlers so they are only bound once for every instance.
this._routerCreateElement = this._routerCreateElement.bind(this);
}
/**
* Inits the app before component will mount.
*
@ -52,26 +25,6 @@ export class App extends AbstractApp {
this.props.store.dispatch(appInit());
}
/**
* Implements React's {@link Component#render()}.
*
* @inheritdoc
* @returns {ReactElement}
*/
render() {
return (
<Provider store = { this.props.store }>
<Router
createElement = { this._routerCreateElement }
history = { this.history }>
{
this._renderRoutes()
}
</Router>
</Provider>
);
}
/**
* Gets a Location object from the window with information about the current
* location of the document.
@ -86,6 +39,7 @@ export class App extends AbstractApp {
* Navigates to a specific Route (via platform-specific means).
*
* @param {Route} route - The Route to which to navigate.
* @protected
* @returns {void}
*/
_navigate(route) {
@ -100,81 +54,19 @@ export class App extends AbstractApp {
/:room/g,
store.getState()['features/base/conference'].room);
return (
store.dispatch(
(window.location.pathname === path ? replace : push)(
path)));
}
// Navigate to the specified Route.
const windowLocation = this._getWindowLocation();
/**
* Invoked by react-router to notify this App that a Route is about to be
* rendered.
*
* @param {Route} route - The Route that is about to be rendered.
* @private
* @returns {void}
*/
_onRouteEnter(route, ...args) {
// Notify the route that it is about to be entered.
const onEnter = route.onEnter;
if (typeof onEnter === 'function') {
onEnter(...args);
if (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);
} 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;
}
// XXX The following is mandatory. Otherwise, moving back & forward
// through the browser's history could leave this App on the Conference
// page without a room name.
// Our Router configuration (at the time of this writing) is such that
// each Route corresponds to a single URL. Hence, entering into a Route
// is like opening a URL.
this._openURL(window.location.toString());
}
/**
* Renders a specific Route (for the purposes of the Router of this App).
*
* @param {Object} route - The Route to render.
* @returns {ReactElement}
* @private
*/
_renderRoute(route) {
const onEnter = (...args) => {
this._onRouteEnter(route, ...args);
};
return (
<Route
component = { route.component }
key = { route.component }
onEnter = { onEnter }
path = { route.path } />
);
}
/**
* Renders the Routes of the Router of this App.
*
* @returns {Array.<ReactElement>}
* @private
*/
_renderRoutes() {
return RouteRegistry.getRoutes().map(this._renderRoute, this);
}
/**
* Create a ReactElement from the specified component and props on behalf of
* the associated Router.
*
* @param {Component} component - The component from which the ReactElement
* is to be created.
* @param {Object} props - The read-only React Component props with which
* the ReactElement is to be initialized.
* @private
* @returns {ReactElement}
*/
_routerCreateElement(component, props) {
return this._createElement(component, props);
}
}

View File

@ -107,8 +107,8 @@ function _urlStringToObject(url) {
try {
urlObj = new URL(url);
} catch (ex) {
// The return value will signal the failure & the logged
// exception will provide the details to the developers.
// The return value will signal the failure & the logged exception
// will provide the details to the developers.
console.log(`${url} seems to be not a valid URL, but it's OK`, ex);
}
}

View File

@ -2,8 +2,6 @@
import React from 'react';
import ReactDOM from 'react-dom';
import { browserHistory } from 'react-router';
import { routerMiddleware, routerReducer } from 'react-router-redux';
import { compose, createStore } from 'redux';
import Thunk from 'redux-thunk';
@ -13,23 +11,14 @@ import { MiddlewareRegistry, ReducerRegistry } from './features/base/redux';
const logger = require('jitsi-meet-logger').getLogger(__filename);
// Create combined reducer from all reducers in registry + routerReducer from
// 'react-router-redux' module (stores location updates from history).
// @see https://github.com/reactjs/react-router-redux#routerreducer.
const reducer = ReducerRegistry.combineReducers({
routing: routerReducer
});
// Create combined reducer from all reducers in registry.
const reducer = ReducerRegistry.combineReducers();
// Apply all registered middleware from the MiddlewareRegistry + additional
// 3rd party middleware:
// - Thunk - allows us to dispatch async actions easily. For more info
// @see https://github.com/gaearon/redux-thunk.
// - routerMiddleware - middleware from 'react-router-redux' module to track
// changes in browser history inside Redux state. For more information
// @see https://github.com/reactjs/react-router-redux.
let middleware = MiddlewareRegistry.applyMiddleware(
Thunk,
routerMiddleware(browserHistory));
let middleware = MiddlewareRegistry.applyMiddleware(Thunk);
// Try to enable Redux DevTools Chrome extension in order to make it available
// for the purposes of facilitating development.