Doesn't expire cached items prop with arrow functions

[kesha-antonov]

Describe the bug
Doesn't expire cached items prop with functions

To Reproduce

Pass function to onPress in items
Even though you can pass updated items - it's cache on the lib side

const holdItemMenuItems = (
  [
    {
      icon: 'copy-outline',
      text: 'Скопировать',
      onPress: () => {
        console.log('holdItemMenuItems text', text)
        Clipboard.setString(text)
      },
    },
  ]
)

Expected behavior
It should call items with new function passed

Actual behavior
Caches function forever

Screenshots

hold-menu-bug.mp4

Package versions

• React: 18.1.0
• React Native: 0.70.5
• React Native Reanimated: 2.13.0
• React Native Hold Menu: latest
• Expo: 47.0.6

[kesha-antonov]

This is my temp fix:

patches/react-native-hold-menu+0.1.5.patch

diff --git a/node_modules/react-native-hold-menu/src/utils/validations.ts b/node_modules/react-native-hold-menu/src/utils/validations.ts
index 856a61c..e95d717 100644
--- a/node_modules/react-native-hold-menu/src/utils/validations.ts
+++ b/node_modules/react-native-hold-menu/src/utils/validations.ts
@@ -12,8 +12,10 @@ function fieldAreSame(obj1: MenuItemProps, obj2: MenuItemProps) {
     const val2 = obj2[key];
 
     if (val1 !== val2) {
-      if (typeof val1 === 'function' && typeof val2 === 'function')
-        return val1.toString() === val2.toString();
+      // FIXME: ARROW FUNCTIONS ALWAYS EQUAL
+      // if (typeof val1 === 'function' && typeof val2 === 'function') {
+      //   return val1.toString() === val2.toString();
+      // }
       return false;
     }

[AlexSirenko]

@kesha-antonov check out this https://enesozturk.github.io/react-native-hold-menu/docs/props#actionparams

[kesha-antonov]

@kesha-antonov check out this https://enesozturk.github.io/react-native-hold-menu/docs/props#actionparams

How it's related to caching problem?

[AlexSirenko]

@kesha-antonov I don't know why caching worked that way, but if you want to pass some changing variable in press function (state, prop, or smth that is changed) you should use actionParams.
So your case should be like that:

const holdItemMenuItems = (
  [
    {
      icon: 'copy-outline',
      text: 'Скопировать',
      onPress: (textParam: string) => {
        console.log('holdItemMenuItems text', textParam)
        Clipboard.setString(textParam)
      },
    },
  ]
)

// there some code

return (
// some jsx
<HoldMenu items={holdItemMenuItems} actionParams={{
// there should go array of params that will be passed to onPress callback.
'Скопировать': [text]
}}>
// some jsx children
</HoldMenu>
)

[kesha-antonov]

@kesha-antonov I don't know why caching worked that way, but if you want to pass some changing variable in press function (state, prop, or smth that is changed) you should use actionParams.

So your case should be like that:

const holdItemMenuItems = (

  [

    {

      icon: 'copy-outline',

      text: 'Скопировать',

      onPress: (textParam: string) => {

        console.log('holdItemMenuItems text', textParam)

        Clipboard.setString(textParam)

      },

    },

  ]

)



// there some code



return (

// some jsx

<HoldMenu items={holdItemMenuItems} actionParams={{

// there should go array of params that will be passed to onPress callback.

'Скопировать': [text]

}}>

// some jsx children

</HoldMenu>

)

It's not straightforward and looks more like a bug than a feature. I passed new props and I expect it to be used in a react component.
I think here caching should be reconsidered for functions.

[NoodleOfDeath]

@kesha-antonov it's a key reactivity issue. they use the index as the key, so react doesnt know that the menu item needs to be recomputed. Rule of thumb, never use an index as your key when mapping components; or expose this to the developer so they can explicitly tell react when components needs to be recomputed

Here is a fix that only recomputes menu items when you want them to (like any mapping of components) rather than every single tick with your temp fix:

#106