Skip to content

Commit

Permalink
fix: check duplicate names only for immediate nested screens
Browse files Browse the repository at this point in the history
  • Loading branch information
satya164 committed Mar 5, 2021
1 parent 13d8553 commit 36a9b4f
Show file tree
Hide file tree
Showing 3 changed files with 99 additions and 22 deletions.
6 changes: 3 additions & 3 deletions packages/core/src/BaseNavigationContainer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ const BaseNavigationContainer = React.forwardRef(

if (!parent.isDefault && !independent) {
throw new Error(
"Looks like you have nested a 'NavigationContainer' inside another. Normally you need only one container at the root of the app, so this was probably an error. If this was intentional, pass 'independent={true}' explicitely. Note that this will make the child navigators disconnected from the parent and you won't be able to navigate between them."
"Looks like you have nested a 'NavigationContainer' inside another. Normally you need only one container at the root of the app, so this was probably an error. If this was intentional, pass 'independent={true}' explicitly. Note that this will make the child navigators disconnected from the parent and you won't be able to navigate between them."
);
}

Expand Down Expand Up @@ -356,8 +356,8 @@ const BaseNavigationContainer = React.forwardRef(
);

if (duplicateRouteNamesResult.length) {
const message = `Found screens with the same name in multiple navigators. Check:\n${duplicateRouteNamesResult.map(
([_, locations]) => `\n${locations.join(', ')}`
const message = `Found screens with the same name nested inside one another. Check:\n${duplicateRouteNamesResult.map(
(locations) => `\n${locations.join(', ')}`
)}\n\nThis can cause confusing behavior during navigation. Consider using unique names for each screen instead.`;

if (!duplicateNameWarnings.includes(message)) {
Expand Down
82 changes: 82 additions & 0 deletions packages/core/src/__tests__/BaseNavigationContainer.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -831,3 +831,85 @@ it('works with state change events in independent nested container', () => {
type: 'test',
});
});

it('warns for duplicate route names nested inside each other', () => {
const TestNavigator = (props: any) => {
const { state, descriptors } = useNavigationBuilder(MockRouter, props);

return (
<React.Fragment>
{descriptors[state.routes[state.index].key].render()}
</React.Fragment>
);
};

const TestScreen = () => <></>;

const spy = jest.spyOn(console, 'warn').mockImplementation();

render(
<BaseNavigationContainer>
<TestNavigator>
<Screen name="foo">
{() => (
<TestNavigator>
<Screen name="foo" component={TestScreen} />
<Screen name="baz" component={TestScreen} />
</TestNavigator>
)}
</Screen>
<Screen name="bar" component={TestScreen} />
</TestNavigator>
</BaseNavigationContainer>
);

expect(spy.mock.calls[0][0]).toMatch(
'Found screens with the same name nested inside one another.'
);

render(
<BaseNavigationContainer>
<TestNavigator>
<Screen name="qux">
{() => (
<TestNavigator>
<Screen name="foo">
{() => (
<TestNavigator>
<Screen name="foo" component={TestScreen} />
<Screen name="baz" component={TestScreen} />
</TestNavigator>
)}
</Screen>
<Screen name="bar" component={TestScreen} />
</TestNavigator>
)}
</Screen>
</TestNavigator>
</BaseNavigationContainer>
);

expect(spy.mock.calls[1][0]).toMatch(
'Found screens with the same name nested inside one another.'
);

render(
<BaseNavigationContainer>
<TestNavigator initialRouteName="bar">
<Screen name="foo" component={TestScreen} />
<Screen name="bar">
{() => (
<TestNavigator>
<Screen name="foo" component={TestScreen} />
<Screen name="baz" component={TestScreen} />
</TestNavigator>
)}
</Screen>
</TestNavigator>
</BaseNavigationContainer>
);

expect(spy).toHaveBeenCalledTimes(2);

spy.mockRestore();
});
33 changes: 14 additions & 19 deletions packages/core/src/checkDuplicateRouteNames.tsx
Original file line number Diff line number Diff line change
@@ -1,38 +1,33 @@
import type { NavigationState, PartialState } from '@react-navigation/routers';

export default function checkDuplicateRouteNames(state: NavigationState) {
const allRouteNames = new Map<string, string[]>();
const duplicates: string[][] = [];

const getRouteNames = (
location: string,
state: NavigationState | PartialState<NavigationState>
) => {
state.routeNames?.forEach((name) => {
const current = allRouteNames.get(name);
const currentLocation = location ? `${location} > ${name}` : name;
state.routes.forEach((route: typeof state.routes[0]) => {
const currentLocation = location
? `${location} > ${route.name}`
: route.name;

if (current) {
current.push(currentLocation);
} else {
allRouteNames.set(name, [currentLocation]);
}
});
route.state?.routeNames?.forEach((routeName) => {
if (routeName === route.name) {
duplicates.push([
currentLocation,
`${currentLocation} > ${route.name}`,
]);
}
});

state.routes.forEach((route: typeof state.routes[0]) => {
if (route.state) {
getRouteNames(
location ? `${location} > ${route.name}` : route.name,
route.state
);
getRouteNames(currentLocation, route.state);
}
});
};

getRouteNames('', state);

const duplicates = Array.from(allRouteNames.entries()).filter(
([_, value]) => value.length > 1
);

return duplicates;
}

0 comments on commit 36a9b4f

Please sign in to comment.