fix: no longer spread key prop on BottomNavigationBar - resolves #4401

[nick42d]

Motivation

Resolves #4401 with suggested fix from @CommanderRedYT

Related issue

#4401

Test plan

Tested following contribution guidelines https://github.com/callstack/react-native-paper/blob/main/CONTRIBUTING.md

[callstack-bot]

Hey @nick42d, thank you for your pull request 🤗. The documentation from this branch can be viewed here.

[hello-ccchen]

any update?

[prageeth]

We are waiting for this too, would be great to have this merged and included in the next release. Thanks

[skoolaidl]

Is this going to get merged?

[shuffledex]

come on guys! launch a new release asap!

[elibroftw]

'key' is specified more than once, so this usage will be overwritten.

I suggest keeping it at one line

renderTouchable = (props: TouchableProps<Route>) => <Touchable {...props} key={props.key} />

[CommanderRedYT]

You sure this works? props will still contain the key, so not sure if react will stop complaining when it is not split

[elibroftw]

Have you never added duplicate prop to a component? The last prop takes precedence. Also the error is a Typescript error visible in an IDE. I'm also already using this code in production since I didn't want the ripples that used to be present in the bottom tabs (I'm not sure if the ripples are removed in more recent versions):

renderTouchable={props => (
            <TouchableWithoutFeedback {...props} key={props.key} >
              <View {...props} key={props.key} />
            </TouchableWithoutFeedback>
          )}

[RothaSoeurn]

Have you never added duplicate prop to a component? The last prop takes precedence. Also the error is a Typescript error visible in an IDE. I'm also already using this code in production since I didn't want the ripples that used to be present in the bottom tabs (I'm not sure if the ripples are removed in more recent versions):

renderTouchable={props => (
            <TouchableWithoutFeedback {...props} key={props.key} >
              <View {...props} key={props.key} />
            </TouchableWithoutFeedback>
          )}

it copy it and pass it work thian k u

[nick42d]

'key' is specified more than once, so this usage will be overwritten.

I suggest keeping it at one line

renderTouchable = (props: TouchableProps<Route>) => <Touchable {...props} key={props.key} />

This seems like a bit of a nitpick to me. Is this required?

[ChromeQ]

Surely it is better to destructure the props and avoid any potential duplicate keys, as the initial suggestion suggests, who cares if it spreads onto 2 lines rather than one, it's just source code and makes no difference to the published and production code.

[seanblonien]

I have tested this with patch-package and it works and gets rid of the error. idk what y'all are going on about but this looks great and we need to merge it so we don't have to patch the library

here is exactly my patch-package for proof:

diff --git a/node_modules/react-native-paper/src/components/BottomNavigation/BottomNavigationBar.tsx b/node_modules/react-native-paper/src/components/BottomNavigation/BottomNavigationBar.tsx
index 0bfe303..0195d70 100644
--- a/node_modules/react-native-paper/src/components/BottomNavigation/BottomNavigationBar.tsx
+++ b/node_modules/react-native-paper/src/components/BottomNavigation/BottomNavigationBar.tsx
@@ -360,7 +360,9 @@ const BottomNavigationBar = <Route extends BaseRoute>({
   navigationState,
   renderIcon,
   renderLabel,
-  renderTouchable = (props: TouchableProps<Route>) => <Touchable {...props} />,
+  renderTouchable = ({key, ...props}: TouchableProps<Route> & { key: string }) => (
+    <TouchableRipple key={key} {...props} />
+  ),
   getLabelText = ({ route }: { route: Route }) => route.title,
   getBadge = ({ route }: { route: Route }) => route.badge,
   getColor = ({ route }: { route: Route }) => route.color,

[hello-ccchen]

any update? 🥹

[lourencorodrigo]

Please guys. Launch a new release with this fix.