Forward unused props in ReanimatedSwipeable and Pressable to their inner components

[latekvo]

Description

Currently large portion of the props accepted by ReanimatedSwipeable and Pressable components remain unused.
This issue mostly refers to all the accessibility props.

This PR adds the use of spread operator to ReanimatedSwipeable and Pressable to forward all the remaining props to inner components, which are not explicitly used otherwise.

Fixes: #3032

Test plan

• turn on radio description
• open the nested Pressable example
• add accessibilityLabel prop to one of the Pressables
• navigate over the modified Pressable with your keyboard (use Tab and Shift-Tab to navigate)
• hear as your label is being read out, which wouldn't occur without this change

[latekvo]

I just found that using the following code works just as well as the ManagedProps method:

const { children, style, ...rest } = props;

The only downside being that it requires to pull out props before being used, as opposed to accessing them directly from the props object.

And so if we've got 20 unique props in one of our components, that'll require having a 20-line-long header of extracted props as opposed to pulling them on-the-go.

@j-piasecki @m-bert Would we prefer to stick with the ManagedProps method, or remove it in favour of this new one?

[m-bert]

ManagedProps looks like using a sledgehammer to crack a nut. In my opinion using spread syntax is enough.

And so if we've got 20 unique props in one of our components, that'll require having a 20-line-long header of extracted props as opposed to pulling them on-the-go.

I think that it is unrealistic edge-case that we shouldn't be concerned about.

Also, I have a couple of comments on the code, but first let's see if @j-piasecki agrees with me 😅

[j-piasecki]

I'd go with spreading the props. It may be verbose, but it's explicit in what it does.

[latekvo]

I think that it is unrealistic edge-case that we shouldn't be concerned about.

i believe it's the exact case this PR is dealing with.
The 2 components utilising ManagedProps are ReanimatedSwipeable and Pressable, both using 16 and 17 unique props respectively.

Although with all that said, I think spread operator approach is indeed simpler and more readable than ManagedProps.

---

Converted to spread operator approach in 0ee242f.

[m-bert]

i believe it's the exact case this PR is dealing with.

Oh okay, what I had in mind was only passing props to components, I forgot about the whole implementation dealing with different props. Nevertheless I believe that spread syntax is better 😅

[m-bert]

Please update title and description of this PR.

[m-bert]

Do we need to use useMemo?

[latekvo]

I think so, we have some useStates which may cause re-renders independently of props changing.
If we were to remove this useMemo, those props would trigger every useCallback to reload each render as well, making useCallbacks useless.

[j-piasecki]

I don't think it's necessary, memoization of props passed to the component should be on the user. For example, we cannot correctly memoize event handlers, without knowing what's in their closure (and we don't have access to it).

Adding to that, I don't think it actually does anything here. React uses Object.is to compare the dependencies. You're passing an entire props object as a dependency, which is created anew for every render and objects are compared based on reference, not value.

[latekvo]

I see, that makes sense.

---

I'll remove all the useMemo on props instances, all useCallback instances which depend on any of the props.

Do you think there are any other sensible ways to go about optimising the now non-memoized functions, or should they be left as-is?

I won't remove the ones which compare primitive value props, as these are compared by value.

[j-piasecki]

Other calls to useCallback and useMemo are fine. I meant specifically this one where you used the entire props object in the dependency array.

[j-piasecki]

If a user uses useCallback to memoize event handlers we want to be able to utilize that and make less updates ourselves.

[latekvo]

Yes I understand, I kept all the other useMemo and useCallback instances as all the values they depend on may be primitive values, sorry for the previous commotion.

Removed prop memoization in cc3ecdc

[m-bert]

Same as above

[latekvo]

I think we do want to keep this one, as the ForwardedRef prop may change without changing SwipeableProps prop.
While spread in itself doesn't use up too much resources, it will trigger dependency changes for multiple useCallbacks further down the file.

[j-piasecki]

Same here