Unnecessary deep cloning of props causes comparisons with === to fail

[HectorRicardo]

Issue Description

Apparently, when using Navigation.updateProps(), the props passed as parameter are deep cloned. This causes shallow comparisons with '===' to fail inside the children components. I don't expect this behavior, because React does not deep clone properties automatically for you (see example).

Steps to Reproduce / Code Snippets / Screenshots

Repository: https://github.com/HectorRicardo/deepCloneBug

index.js:

import {Navigation} from 'react-native-navigation';

import App from './App';

Navigation.registerComponent('com.myApp.WelcomeScreen', () => App);

Navigation.events().registerAppLaunchedListener(() => {
  Navigation.setRoot({
    root: {
      stack: {
        children: [
          {
            component: {
              id: 'com.myApp.WelcomeScreen',
              name: 'com.myApp.WelcomeScreen',
              passProps: {
                onSelector2ItemSelect: function onSelector2ItemSelect(item) {
                  Navigation.updateProps('com.myApp.WelcomeScreen', {
                    selectedItemSelector2: item,
                    onSelector2ItemSelect,
                  });
                },
              },
            },
          },
        ],
      },
    },
  });
});

App.js

import React, {useState} from 'react';
import {Button, Text, View, StyleSheet} from 'react-native';

const itemsList = [{name: 'item1'}, {name: 'item2'}];

const App = ({selectedItemSelector2, onSelector2ItemSelect}) => {
  const [selectedItemSelector1, setSelectedItemOfSelector1] = useState();
  return (
    <View style={styles.root}>
      <Selector
        description="this selector works"
        items={itemsList}
        selectedItem={selectedItemSelector1}
        onSelectItem={setSelectedItemOfSelector1}
      />
      <Selector
        description="this selector doesn't work"
        items={itemsList}
        selectedItem={selectedItemSelector2}
        onSelectItem={onSelector2ItemSelect}
      />
    </View>
  );
};

const Selector = ({description, items, selectedItem, onSelectItem}) => (
  <View>
    <Text style={styles.description}>{description}</Text>
    <Text style={styles.description}>
      Selected item's name: {selectedItem?.name}
    </Text>
    {items.map(item => (
      <View
        key={item.name}
        style={[
          styles.itemContainer,
          selectedItem === item && styles.selectedItem, // <------ comparison with === fails
        ]}>
        <Text>{item.name}</Text>
        <Button title="Select me" onPress={() => onSelectItem(item)} />
      </View>
    ))}
  </View>
);

export default App;

const styles = StyleSheet.create({
  root: {flex: 1, justifyContent: 'space-evenly'},
  description: {textAlign: 'center'},
  itemContainer: {
    flexDirection: 'row',
    marginTop: 20,
    justifyContent: 'space-evenly',
  },
  selectedItem: {backgroundColor: 'yellow'},
});

---

Environment

• React Native Navigation version: 6.7.1
• React Native version: 0.62.2
• Platform(s) (iOS, Android, or both?): Android
• Device info (Simulator/Device? OS version? Debug/Release?): Huawei P30 Lite Debug. Android version 10.

[HectorRicardo]

After digging through the code, I believe this may fix the issue.

File Commands.ts
Remove line 68 const input = cloneDeep(props);
Replace input with props on line 69.

But I don't understand the rationale of cloning the props. I see that the cloneDeep function is called almost in every method of the class, and I am not sure of the reason behind it.

Also, on file ComponentWrapper.tsx, line 31, instead of using lodash's merge function, why not use assign ?

[N3TC4T]

Maybe there is a reason behind this (@guyca)? , if not I think it's better to we remove it ?

I had a problem with this as well and I had to spend some time to find out using cloneDeep in RNN is the cause of my problem. as lodash cloneDeep will look at object keys and it won't work if you have custom object with custom property accessor. so my object had null values after passing as prop.

[HectorRicardo]

In my humble opinion, there's no need to do deep cloning, since React doesn't do that for you....the more similar this library is to React, the better.

[jfrolich]

I don't think it's good to rely on === here, you probably want to check an identifiable property.

[jinshin1013]

Guys, I'm not sure if it is worth changing the current behaviour of the library as there are multiple ways for the consumer to be checking the equality of the objects. @HectorRicardo you could simply do JSON.stringify(obj1) === JSON.stringify(obj2) to resolve your issue.

Also as @jfrolich suggested, it would be a better practice to be comparing a unique property than an entire object. Closing the issue for now as I don't see an immediate benefit but feel free to provide feedback if this must be implemented.