🐛

rememberはバグの元

2024/11/18に公開

Composable関数の中で状態を維持したい時や、毎回計算するのが大変な処理を軽くするために remember を使いたくなることがあります。
しかし、使い方を誤ると思ったように動いてくれなかったり、不具合の原因となったりします。

今回は、私が見かけた「よくやりがちなrememberの誤った使い方」について紹介します。

リストの要素でユーザー操作による状態変更をする

ユーザーの一覧を表示していて、各セルにユーザーの情報に加えてフォローボタンがあるようなUIを思い浮かべてください。TwitterXとかでよくある、ユーザー検索結果の一覧みたいなものをイメージしていただけるとわかりやすいかと思います。

フォローボタンは、対象ユーザーを自分がフォローしていなければ「フォローする」というテキストになり、対象ユーザーを自分がフォロー中であれば「フォロー解除」というテキストになるものとします。タップすると状態に応じたアクションが実行され、新しい状態に切り替わります。つまり、フォローボタンを押すと状態が反転するということです。

こんな感じ

リストの要素にあたる部分だけを抜き出してみるとこんな感じです。

@Composable
fun UserListItem(
    user: User, // サーバーから取得する、ユーザーの状態
    modifier: Modifier = Modifier,
) {
    Row(
        modifier.padding(12.dp),
        verticalAlignment = Alignment.CenterVertically
    ) {
        Text(
            user.name,
            fontSize = 24.sp,
            modifier = Modifier.weight(1f)
        )
        Button(
            onClick = { /*TODO*/ },
        ) {
            Text(
                if (user.isFollowing) { // 自分がこのユーザーをフォローしているか
                    "フォロー解除"
                } else {
                    "フォローする"
                }
            )
        }
    }
}

※上記はまだボタンのタップ時に何もしていないのでフォロー状態は変化しません。

さて、ここに「ボタンを押したらフォロー状態を変化させる」機能を追加するにはどうしますか?

期待される挙動から考えてみると、リスト全体を再読み込みすることなく、フォローボタンの状態だけが変化するということになるでしょうから、このように記述したくなるかもしれません。

@Composable
fun UserListItem(
    user: User,
    modifier: Modifier = Modifier,
) {
    // リストを表示した時点のフォロー状態を覚えておくが、フォローボタンの操作により切り替わる
    var isFollowing by remember { mutableStateOf(user.isFollowing) }

    Row(
        modifier.padding(12.dp),
        verticalAlignment = Alignment.CenterVertically
    ) {
        Text(
            user.name,
            fontSize = 24.sp,
            modifier = Modifier.weight(1f)
        )
        Button(
            onClick = {
                isFollowing = !isFollowing
                // TODO: サーバーへリクエストしたりする
            },
        ) {
            Text(
                if (isFollowing) {
                    "フォロー解除"
                } else {
                    "フォローする"
                }
            )
        }
    }
}

これで一応、見た目上はフォローボタンがタップするたびに「フォローする」→「フォロー解除」→「フォローする」→…というふうに切り替わるようになります。

…が、このように実装してはいけません。

なにがいけないのか

このコードの明らかな問題点は、UserListItemに渡されるuserが別のユーザーに変わった際に isFollowing が適切なフォロー状態にならなくなることです。

LazyColumnを使って上記のUserListItemを縦に並べるとします。

@Composable
fun UserList(
    users: List<User>,
    modifier: Modifier = Modifier
) {
    LazyColumn(modifier) {
        items(users) { user ->
            UserListItem(user, modifier = Modifier.fillMaxWidth())
        }
    }
}

LazyColumnは、スクロールするリストを効率的に処理します。リスト全件分のUserListItemの計算をするのではなく、画面内に表示される分だけUserListItemを処理します。

そして、UserListItemが画面外にスクロールされると、次に下から追加のUserListItemが出てくることになるのですが、その際に毎回新しいUserListItemが作成されるのではなく、画面外に追いやられたUserListItemが別のユーザー用に再利用されます。そして、このような再利用が起きる時にrememberの内容は保持されます。

たとえば、ユーザーA, ユーザーB, ユーザーC, ユーザーD, ユーザーE, ... と長いユーザーリストを表示しようとしていて、一画面に収まるのは表示されるのはユーザーDまでだったとします。
画面をスクロールしていくとユーザーAは画面の外に追いやられ、下からユーザーEが出てきます。
この時、最初にユーザーAが渡されてUserListItemのUIが作られたあと、同じUserListItemに対してユーザーEが渡されることになります。

var isFollowing by remember { mutableStateOf(user.isFollowing) }

は、最初に渡されたユーザーAのフォロー状態を覚えたままになるので、後でユーザーEが渡されたとしてもisFollowingの値はユーザーAのフォロー状態のままになります。

なので、このようなコードはユーザー件数が数件で画面内に収まる程度なら問題なく動作するが、スクロールが必要な程度にユーザー件数が増えると正しい状態を表示しないという不具合の原因となります。

この不具合のたちの悪いところは、リスト内の要素の状態がほぼ全部同じになるような画面だったら気づかれにくいというところです。
たとえば、「自分がフォローしているユーザーの一覧画面」にこのコードがあったとしたら、表示されるユーザーは初期状態では全員 isFollowing = true なので、リストを一目見ただけでは不具合の存在に気付けません。
1,2名をフォロー解除してみた上で、さらにいくらか画面をスクロールしてみると違和感に気付けるはずなのですが、だいたいこういう機能を開発している時点ではフォローボタンを押した時の動作にしか注目していないため、他のセルに不整合な状態が表示されていても見落とす可能性が高いです。また、開発中はスクロールが必要なだけのデータ件数を用意するのが面倒でサボりがちということもあり、QAでも不具合が検出されなくてそのまま製品版として世の中にリリースされる可能性は割と高いです。

どうすれば良いか

今回の例であれば、UserListItemが別のユーザーに対して再利用されたときにはisFollowingの値がuser.isFollowingに再設定されて欲しいので、rememberのキーにuserを指定します。

@Composable
fun UserListItem(
    user: User,
    modifier: Modifier = Modifier,
) {
    // リストを表示した時点のフォロー状態を覚えておくが、フォローボタンの操作により切り替わる
    var isFollowing by remember(user) { mutableStateOf(user.isFollowing) }

    Row(
        modifier.padding(12.dp),
        verticalAlignment = Alignment.CenterVertically
    ) {
        Text(
            user.name,
            fontSize = 24.sp,
            modifier = Modifier.weight(1f)
        )
        Button(
            onClick = {
                isFollowing = !isFollowing
                // TODO: サーバーへリクエストしたりする
            },
        ) {
            Text(
                if (isFollowing) {
                    "フォロー解除"
                } else {
                    "フォローする"
                }
            )
        }
    }
}

これで、isFollowinguserが別のユーザーに変わった際にmutableStateOf(user.isFollowing)が再計算されて正しい状態を表示するようになります。

本当に?

ですが、そもそもrememberを使わないように書けるならそれに越したことはないです。
今回のような例であれば、フォローボタンタップのイベントを呼び出し元(ViewModelなど)に伝搬し、ユーザーリスト全体の状態を更新してUserList全部を再コンポーズさせるのが適切な実装です。

@Composable
fun UserListItem(
    user: User,
    onClickFollow: () -> Unit,
    modifier: Modifier = Modifier,
) {
    Row(
        modifier.padding(12.dp),
        verticalAlignment = Alignment.CenterVertically
    ) {
        Text(
            user.name,
            fontSize = 24.sp,
            modifier = Modifier.weight(1f)
        )
        Button(
            onClick = onClickFollow,
        ) {
            Text(
                if (user.isFollowing) { // 自分がこのユーザーをフォローしているか
                    "フォロー解除"
                } else {
                    "フォローする"
                }
            )
        }
    }
}

@Composable
fun UserList(
    users: List<User>,
    onClickFollow: (User) -> Unit,
    modifier: Modifier = Modifier
) {
    LazyColumn(modifier) {
        items(users) { user ->
            UserListItem(
                user = user,
                onClickFollow = { onClickFollow(user) },
                modifier = Modifier.fillMaxWidth(),
            )
        }
    }
}

@Composable
fun UserListScreen() {
    val users by viewModel.users.collectAsState()
    UserList(
        users = users,
        onClickFollow = viewModel::onClickFollow,
    )
}

class UserListViewModel: ViewModel() {
    val users: StateFlow<List<Users>> ...

    fun onClickFollow(user: User) {
        // TODO: user.isFollowingに応じてフォローまたはフォロー解除をする処理
        // TODO: フォロー状態の変更を反映した後のユーザーリスト全体をusersに流す
    }
}

※ViewModelの中でどうやってusers全体を更新させるかは様々な方法が考えられますが、本記事のスコープ外になるので説明は割愛します。

上記のUserListItemは、渡されたuserの状態を常に正として表示するようになっているので、スクロールがあってもなくても同じように動作するということが明らかです。

一般にComposable関数はステートレスであるべきとされていますが、rememberを使うとComposable関数が状態を持つことになり、ステートレスではなくなります。
そうすると扱いが難しくなり、不具合の原因となります。
実際、今回のような「リストの表示件数が多いときに実際の状態とUIの表示内容が食い違う」という不具合は、何度も目にしました。

なので、rememberはバグの元と言っても過言ではないと思います。

rememberを書こうと思った時や、コードレビューでrememberを使ったコードを見かけた際には
「本当にrememberを使う必要があるのか?」
「このrememberは意図せず古い状態を持ち続けてしまわないか?」
というのを気にするようにしてみてください。

Discussion