Permalink
Browse files

Warn when users have multiple stateful navigation containers (#3819)

* First pass at warning when users explicitly render nested navigators

* Clean up tests around warnings

* Update comment

* Update comment again
  • Loading branch information...
brentvatne committed Mar 25, 2018
1 parent b7c6d07 commit 68a2a106f370003dc1d46385fd8b5992be189ee2
@@ -1,46 +1,51 @@
import React from 'react';
import 'react-native';
import { StyleSheet, View } from 'react-native';
import renderer from 'react-test-renderer';
import NavigationActions from '../NavigationActions';
import StackNavigator from '../navigators/createStackNavigator';
const FooScreen = () => <div />;
const BarScreen = () => <div />;
const BazScreen = () => <div />;
const CarScreen = () => <div />;
const DogScreen = () => <div />;
const ElkScreen = () => <div />;
const NavigationContainer = StackNavigator(
{
foo: {
screen: FooScreen,
},
bar: {
screen: BarScreen,
},
baz: {
screen: BazScreen,
},
car: {
screen: CarScreen,
},
dog: {
screen: DogScreen,
},
elk: {
screen: ElkScreen,
},
},
{
initialRouteName: 'foo',
}
);
jest.useFakeTimers();
import createStackNavigator from '../navigators/createStackNavigator';
import { _TESTING_ONLY_reset_container_count } from '../createNavigationContainer';
describe('NavigationContainer', () => {
jest.useFakeTimers();
beforeEach(() => {
_TESTING_ONLY_reset_container_count();
});
const FooScreen = () => <div />;
const BarScreen = () => <div />;
const BazScreen = () => <div />;
const CarScreen = () => <div />;
const DogScreen = () => <div />;
const ElkScreen = () => <div />;
const NavigationContainer = createStackNavigator(
{
foo: {
screen: FooScreen,
},
bar: {
screen: BarScreen,
},
baz: {
screen: BazScreen,
},
car: {
screen: CarScreen,
},
dog: {
screen: DogScreen,
},
elk: {
screen: ElkScreen,
},
},
{
initialRouteName: 'foo',
}
);
describe('state.nav', () => {
it("should be preloaded with the router's initial state", () => {
const navigationContainer = renderer
@@ -198,4 +203,57 @@ describe('NavigationContainer', () => {
});
});
});
describe('warnings', () => {
function spyConsole() {
let spy = {};
beforeEach(() => {
spy.console = jest.spyOn(console, 'error').mockImplementation(() => {});
});
afterEach(() => {
spy.console.mockRestore();
});
return spy;
}
describe('detached navigators', () => {
beforeEach(() => {
_TESTING_ONLY_reset_container_count();
});
let spy = spyConsole();
it('warns when you render more than one navigator explicitly', () => {
class BlankScreen extends React.Component {
render() {
return <View />;
}
}
class RootScreen extends React.Component {
render() {
return (
<View>
<ChildNavigator />
</View>
);
}
}
const ChildNavigator = createStackNavigator({
Child: BlankScreen,
});
const RootStack = createStackNavigator({
Root: RootScreen,
});
renderer.create(<RootStack />).toJSON();
expect(spy).toMatchSnapshot();
});
});
});
});
@@ -0,0 +1,13 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP
exports[`NavigationContainer warnings detached navigators warns when you render more than one navigator explicitly 1`] = `
Object {
"console": [MockFunction] {
"calls": Array [
Array [
"You should only render one navigator explicitly in your app, and other navigators should by rendered by including them in that navigator. Full details at: https://v2.reactnavigation.org/docs/common-mistakes.html#explicitly-rendering-more-than-one-navigator",
],
],
},
}
`;
@@ -1,11 +1,12 @@
import React from 'react';
import withLifecyclePolyfill from 'react-lifecycles-compat';
import { Linking, AsyncStorage } from 'react-native';
import withLifecyclePolyfill from 'react-lifecycles-compat';
import { BackHandler } from './PlatformHelpers';
import NavigationActions from './NavigationActions';
import addNavigationHelpers from './addNavigationHelpers';
import invariant from './utils/invariant';
import docsUrl from './utils/docsUrl';
function isStateful(props) {
return !props.navigation;
@@ -32,12 +33,22 @@ function validateProps(props) {
}
}
// Track the number of stateful container instances. Warn if >0 and not using the
// detached prop to explicitly acknowledge the behavior. We should deprecated implicit
// stateful navigation containers in a future release and require a provider style pattern
// instead in order to eliminate confusion entirely.
let _statefulContainerCount = 0;
export function _TESTING_ONLY_reset_container_count() {
_statefulContainerCount = 0;
}
// We keep a global flag to catch errors during the state persistence hydrating scenario.
// The innermost navigator who catches the error will dispatch a new init action.
let _reactNavigationIsHydratingState = false;
// Unfortunate to use global state here, but it seems necessesary for the time being. There seems to
// be some problems with cascading componentDidCatch handlers. Ideally the inner non-stateful navigator
// catches the error and re-throws it, to be caught by the top-level stateful navigator.
// Unfortunate to use global state here, but it seems necessesary for the time
// being. There seems to be some problems with cascading componentDidCatch
// handlers. Ideally the inner non-stateful navigator catches the error and
// re-throws it, to be caught by the top-level stateful navigator.
/**
* Create an HOC that injects the navigation and manages the navigation state
@@ -186,6 +197,16 @@ export default function createNavigationContainer(Component) {
return;
}
if (__DEV__ && !this.props.detached) {
if (_statefulContainerCount > 0) {
console.error(
`You should only render one navigator explicitly in your app, and other navigators should by rendered by including them in that navigator. Full details at: ${docsUrl(
'common-mistakes.html#explicitly-rendering-more-than-one-navigator'
)}`
);
}
}
_statefulContainerCount++;
Linking.addEventListener('url', this._handleOpenURL);
const { persistenceKey } = this.props;
@@ -257,6 +278,10 @@ export default function createNavigationContainer(Component) {
this._isMounted = false;
Linking.removeEventListener('url', this._handleOpenURL);
this.subs && this.subs.remove();
if (this._isStateful()) {
_statefulContainerCount--;
}
}
// Per-tick temporary storage for state.nav
@@ -4,6 +4,7 @@ import renderer from 'react-test-renderer';
import StackNavigator from '../createStackNavigator';
import withNavigation from '../../views/withNavigation';
import { _TESTING_ONLY_reset_container_count } from '../../createNavigationContainer';
const styles = StyleSheet.create({
header: {
@@ -32,6 +33,10 @@ const routeConfig = {
};
describe('StackNavigator', () => {
beforeEach(() => {
_TESTING_ONLY_reset_container_count();
});
it('renders successfully', () => {
const MyStackNavigator = StackNavigator(routeConfig);
const rendered = renderer.create(<MyStackNavigator />).toJSON();
@@ -0,0 +1,3 @@
export default function docsUrl(path) {
return `https://v2.reactnavigation.org/docs/${path}`;
}

0 comments on commit 68a2a10

Please sign in to comment.