Skip to content

Commit

Permalink
fix: avoid shared value reads during render (#662)
Browse files Browse the repository at this point in the history
## 📜 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
#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
  • Loading branch information
kirillzyusko authored Oct 28, 2024
1 parent d16908c commit e31a62a
Showing 1 changed file with 10 additions and 4 deletions.
14 changes: 10 additions & 4 deletions src/components/KeyboardAvoidingView/hooks.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,20 @@
import { useState } from "react";
import { useSharedValue } from "react-native-reanimated";

import { useKeyboardContext } from "../../context";
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(
{
Expand Down

0 comments on commit e31a62a

Please sign in to comment.