From e31a62a3ab59825cf5da616e50bb8d82d95e1579 Mon Sep 17 00:00:00 2001
From: Kirill Zyusko <zyusko.kirik@gmail.com>
Date: Mon, 28 Oct 2024 18:41:55 +0100
Subject: [PATCH] fix: avoid shared value reads during render (#662)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

## 📜 Description

Fixed popping up warnings when re-render happens if
`KeyboardAvoidingView` is used.

## 💡 Motivation and Context

Reading `.value` during render is a bad approach because it violates
rules of react, so we shouldn't use it anymore.

Fortunately we have only one place in the codebase - it's
`KeyboardAvoidingView`. Technically I don't violate rules of

Initially I had 3 approaches in my head:

### 1️⃣  Precompute initial value via `useMemo`

The first approach I was thinking of was this:

```tsx
const {height, progress} = useMemo(() => ({
  height: context.reanimated.height,
  progress: context.reanimated.progress,
}), []);

const heightWhenOpened = useSharedValue(-height);
const height = useSharedValue(-height);
const progress = useSharedValue(progress);
const isClosed = useSharedValue(progress === 0);
```

However later on in react docs I found this:

<img width="918" alt="image"
src="https://github.com/user-attachments/assets/68d698ac-afb8-4677-8cf7-392936b4c0bb">

So I thought that in future **potentially** `useMemo` may be
re-evaluated, so we'll get this warning again. Which is not desirable.

### 2️⃣ Memoize the initial value via `useState`

Another approach was the usage of `useState`:

```tsx
  const [initialHeight] = useState(() => -reanimated.height.value);
  const [initialProgress] = useState(() => reanimated.progress.value);

  const heightWhenOpened = useSharedValue(initialHeight);
  const height = useSharedValue(initialHeight);
  const progress = useSharedValue(initialProgress);
  const isClosed = useSharedValue(initialProgress === 0);
```

In this case we derive value (kind of preparing them) and we are sure
that those values will not be accidentally re-calculated (because in
such way state will be initialized only one time).

The only downside is that we are creating additional variables and we
use state not for its purpose (such values will never be updated
actually).

### 3️⃣ create `useLazySharedValue` hook

The implementation could look like:

```tsx
export function useLazySharedValue<Value>(initialValue: () => Value): SharedValue<Value> { // function instead of value
  const [mutable] = useState(() => makeMutable(initialValue()));
  useEffect(() => {
    return () => {
      cancelAnimation(mutable);
    };
  }, [mutable]);
  return mutable;
}

// and usage...

const progress = useLazySharedValue(() => context.reanimated.progress);
```

However I have next concerns in my head:
- `makeMutable` is publicly exported, but I don't want to copy internal
implementation of `useSharedValue` (even though this hook is a very
simple). If it changes over time I'll have to replicate these changes in
this package as well.

<hr>

Keeping all possible ways in my head I decided to go with approach
**2**. In future I can adjust the logic if needed.

Closes
https://github.com/kirillzyusko/react-native-keyboard-controller/issues/649

## 📢 Changelog

<!-- High level overview of important changes -->
<!-- For example: fixed status bar manipulation; added new types
declarations; -->
<!-- If your changes don't affect one of platform/language below - then
remove this platform/language -->

### JS

- prepare init values from shred value via `useState(() => `;

## 🤔 How Has This Been Tested?

Tested locally on iPhone 16 Pro iOS 18.0.

## 📸 Screenshots (if appropriate):

|Before|After|
|-------|-----|
|<video
src="https://github.com/user-attachments/assets/6fe02687-683a-4988-a370-3f1fba864c04">|<video
src="https://github.com/user-attachments/assets/c7a79568-8705-49f8-97fd-19540291b2a9">|

## 📝 Checklist

- [x] CI successfully passed
- [x] I added new mocks and corresponding unit-tests if library API was
changed
---
 src/components/KeyboardAvoidingView/hooks.ts | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/src/components/KeyboardAvoidingView/hooks.ts b/src/components/KeyboardAvoidingView/hooks.ts
index a183ef98d1..c0416d1134 100644
--- a/src/components/KeyboardAvoidingView/hooks.ts
+++ b/src/components/KeyboardAvoidingView/hooks.ts
@@ -1,3 +1,4 @@
+import { useState } from "react";
 import { useSharedValue } from "react-native-reanimated";
 
 import { useKeyboardContext } from "../../context";
@@ -5,10 +6,15 @@ import { useKeyboardHandler } from "../../hooks";
 
 export const useKeyboardAnimation = () => {
   const { reanimated } = useKeyboardContext();
-  const heightWhenOpened = useSharedValue(-reanimated.height.value);
-  const height = useSharedValue(-reanimated.height.value);
-  const progress = useSharedValue(reanimated.progress.value);
-  const isClosed = useSharedValue(reanimated.progress.value === 0);
+
+  // calculate it only once on mount, to avoid `SharedValue` reads during a render
+  const [initialHeight] = useState(() => -reanimated.height.value);
+  const [initialProgress] = useState(() => reanimated.progress.value);
+
+  const heightWhenOpened = useSharedValue(initialHeight);
+  const height = useSharedValue(initialHeight);
+  const progress = useSharedValue(initialProgress);
+  const isClosed = useSharedValue(initialProgress === 0);
 
   useKeyboardHandler(
     {