Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

React研修資料へのコメント #73

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

aDaisukeHirano
Copy link

@aDaisukeHirano aDaisukeHirano commented Dec 25, 2024

更新内容

React研修資料で改善した方が良さそうなところにコメントをしました。
間違っているコメントやもっとよい改善方法が見つかると思うので、レビューをいただいたあとに清書する予定です。

![仮想DOMのイメージ](./02_lesson2-2.png)

計算が終わると、リアル DOM は実際に変更されたものだけが更新されます。

// "実際に変更されたもの"という文言が少し気になるので
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

「算出した、『実際に変更のある部分』のみ、リアル DOM を更新します。」

という表現はどうでしょう。

| `useImperativeHandle` | **親** コンポーネント内のDOM要素の参照を取得する |
| `useDebugValue` | React DevTools でカスタムフックのラベルを表示する |
// useSyncExternalStoreやuseTransitionなど、React18,19で追加されたHooksを追加するか考えたいです。個人的には、初学者をターゲットとしているので追加しなくても良いかなと思いました。追加しないのであれば、使用頻度が低いuseImperativeHandleとuseDebugValueは削除して良いのかなと思いました。
Copy link
Contributor

@aTomohiroIto aTomohiroIto Dec 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

初学者向けに内容を小さくまとめるなら、useCallback、useMemo も同様に外して良いのでは、と思いました。
いずれ自動最適化が働くことを見込んでいます。

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

useCallback, useMemo は実案件での使用頻度が高いので残した方が良いです。
useImperativeHandleuseDebugValue は僕は見たこと無いので削除して良いでしょう。
新しい hooks についてはどっちでも良いと思います。講師次第ですが、口頭で紹介するくらいがちょうどいいのかも?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

useCallback, useMemoに関しては同じ理由で自分も残したいです。

@@ -9,11 +9,36 @@ React では、「状態」のことを言葉通りに「state」と呼びます

`props`は、コンポーネントの外から渡すもので、かつ、読み取り専用です。
一方、`state`は、コンポーネント自身が持つ情報で、かつ、書き換えができます。
// レンダリング中は書き換えは出来ないですし、書き換えるにしてもmutableな変更は出来ないので、「書き換えができます」とは書きたくないです。

関数型言語では、**純粋関数** と **不純関数** という用語があります。
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(今回のPRのスコープ外ですが)「不純関数」ではなく「非純粋関数」ですね。

Comment on lines +18 to +20
// stateは副作用ですが、immutableなので、ある意味「引数のようなもの」と考えることもできます。
公式Docの[純粋性が重要である理由](https://ja.react.dev/reference/rules/components-and-hooks-must-be-pure#why-does-purity-matter) には、「コンポーネントの入力とは props と state とコンテクスト。フックの入力とはその引数。」ともあります。
stateを副作用の代表として取り上げたくないです。
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

state を引数として解釈すれば確かにそうなりますが、「拡大解釈し過ぎでは?」とも思います。

普通に解釈すると、関数の引数である props のみが引数であり、引数が同じであれば結果が同じ(参照透過性)というのが純粋であるための必要条件なので、propsが同じでもstateが変わると返り値が変わるというのは純粋ではないです。

あまりこういったおかしな解釈を広めたくないという思いはありますが、Reactの公式ドキュメントと矛盾するのも良くないですね。純粋云々の記述自体削除してしまったほうが良いと思います。

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

そうですね

まるまる消してしまってもよいと思いますし、「useStateは副作用か」という見出しを作って、一般的な純粋関数の定義に沿った説明と、公式ドキュメントの説明を両方載せてもよいのかなと思いました。

Comment on lines +37 to +40
stateは、レンダリング間で値を保持したいもの、かつ、値が書き変わったときに際レンダリングしたい値を保持するもの
refは、レンダリング間で値を保持したいもの、かつ、値が書き変わったときに際レンダリングしたくない値を保持するもの

と説明したいです。
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

良い説明だと思います。

|Commit Phase | DOM操作を含めた、副作用の実行。react-dom ライブラリの仕事 |

// 描画と書かれていますが、ブラウザが画面を描画する「ブラウザレンダリング」とは別なので、公式Docのように「コミット」という単語を使いたいです。
Copy link
Author

@aDaisukeHirano aDaisukeHirano Dec 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

すみません、間違っていたので以下のように訂正します。

「Updating」を「再描画」と訳するのは、ブラウザレンダリングやコミットと混同しやすいので、単に「更新」でよいのかなと思いました。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants