fix: race condition between js and worklet

[kirillzyusko]

📜 Description

Fixed an issue where scrollPosition.value is updated with out-of-date value.

💡 Motivation and Context

The problem happens, because without animation onMove/onEnd dispatched almost simultaneously. But js handler can not update position.value and in onEnd handler in scrollPosition.value = position.value; code we set scrollPosition.value = 0.

When text input grows:

layout.value = input.value;
scrollPosition.value += maybeScroll(keyboardHeight.value, true);

We call maybeScroll, but scrollPosition.value === 0 and because of that we scroll by 16px (though we had to scroll for actualScroll (170) + 16 but we do 0 + 16, and because of that position is incorrect.

To fix the problem onEnd should actually update scrollPosition to the expected value, and to achieve that we need to update a value from worklet.

Now it's slightly problematic, because current REA doesn't support both: js + worklet handlers (see https://github.com/software-mansion/react-native-reanimated/issues/6204), so I'm always re-dispatching nativeEvent manually via runOnJS.

The issue was caught originally in https://github.com/kirillzyusko/react-native-keyboard-controller/pull/488

📢 Changelog

JS

• fix race condition between scroll position update and keyboard movement.

🤔 How Has This Been Tested?

• Set focus to TextInput#3
• press enter 2 times

Tested on Android 12 (Pixel 2) with disabled animations (detox).

📸 Screenshots (if appropriate):

Before	After

📝 Checklist

• [x] CI successfully passed
• [x] I added new mocks and corresponding unit-tests if library API was changed

[github-actions[bot]]

📊 Package size report

Current size	Target Size	Difference
144688 bytes	144183 bytes	505 bytes 📈

[mobily]

@kirillzyusko hello! 👋 I was wondering if the new Reanimated hook useComposedEventHandler could be helpful in this case for merging scroll handlers together, just like this:

const internalOnScroll = useAnimatedScrollHandler(
  {
    onScroll: (event) => {
      position.value = event.contentOffset.y;
    },
  },
  [],
);

const onScroll = useComposedEventHandler(
  [internalOnScroll, onScrollProps].filter(Boolean),
);

[kirillzyusko]

@mobily I tried your code, but in this case onScroll from KeyboardAwareScrollView is not fired 🙈

And I think useComposedEventHandler gives an access only to nativeEvent, am I right?

[mobily]

in this case onScroll from KeyboardAwareScrollView is not fired 🙈

If you pass a function, that's the correct behavior. However, passing another scroll handler works fine and the event is fired. The current implementation causes crashes when passing a scroll handler because it's not supported internally by KeyboardAwareScrollView. Considering that KeyboardAwareScrollView is built on top of Animated.ScrollView (https://github.com/kirillzyusko/react-native-keyboard-controller/blob/main/src/components/KeyboardAwareScrollView/index.tsx#L343), it maybe should accept both formats.

1. onScroll → function callback

<KeyboardAwareScrollView
  onScroll={(e) => console.log('FUNCTION', e)}
  scrollEventThrottle={16}
>
  {children}
</KeyboardAwareScrollView>

https://github.com/kirillzyusko/react-native-keyboard-controller/assets/1467712/fe86959d-8f00-49c5-8a6d-0c06b80f8c84

2.onScroll → scroll handler from Reanimated

const scrollHandler = useAnimatedScrollHandler({
  onScroll: (event) => {
    console.log('SCROLL HANDLER', event);
  },
});

<KeyboardAwareScrollView
  onScroll={scrollHandler}
  scrollEventThrottle={16}
>
  {children}
</KeyboardAwareScrollView>

https://github.com/kirillzyusko/react-native-keyboard-controller/assets/1467712/f59192ad-a6b4-464b-ae31-dc7f46ff948e

3. Implementation

The initial suggestion fails to consider the first point (a function callback). The final implementation might appear as:

const internalOnScroll = useAnimatedScrollHandler(
  {
    onScroll: (event) => {
      position.value = event.contentOffset.y;

      if (typeof onScrollProps === "function") {
        runOnJS(onScrollProps)({ nativeEvent: event });
      }
    },
  },
  [onScrollProps],
);

const onScroll = useComposedEventHandler([
  internalOnScroll,
  typeof onScrollProps === "object" ? onScrollProps : null,
]);

And here's the crash/error I encounter when passing a scroll handler (from Reanimated) to the onScroll prop:

https://github.com/kirillzyusko/react-native-keyboard-controller/assets/1467712/d5c4d407-c20d-4882-a14c-6b0a8a222d32

[kirillzyusko]

@mobily do I correctly understand that you want to attach your own reanimated scroll handler?

In this case probably you'll need to pass another property scrollHandler and implementation will look like:

const internalOnScroll = useAnimatedScrollHandler(
  {
    onScroll: (event) => {
      position.value = event.contentOffset.y;

      if (typeof onScrollProps === "function") {
        runOnJS(onScrollProps)({ nativeEvent: event });
      }
    },
  },
  [onScrollProps],
);

const onScroll = useComposedEventHandler([
  internalOnScroll,
 scrollHandler,
].filter(Boolean));

Another option would be to try to wrap KeyboardAwareScrollView into Reanimated.createAnimatedComponent and pass your instance of useAnimatedScrollHandler to your component returned by Reanimated.createAnimatedComponent (but I doubt it will work).

I hope I understand your intention properly, but if not - feel free to correct me and explain again what you are trying to achieve 😊

[mobily]

do I correctly understand that you want to attach your own reanimated scroll handler?

correct!

In this case probably you'll need to pass another property scrollHandler and implementation will look like

I would keep it as a single prop (onScroll) instead of having two (onScroll and scrollHandler). At least this is how it works when using Animated.ScrollView: you can pass either a function or a scroll handler to onScroll

Another option would be to try to wrap KeyboardAwareScrollView into Reanimated.createAnimatedComponent

I have tried this before, and it does work, but the performance seems slower, especially on Android devices