refine getTopic using RTKQ #458
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mshim03
reviewed
Nov 7, 2024
web/src/components/AnalysisTopic.jsx
Outdated
| const handleDetailOpen = () => setDetailOpen(!detailOpen); | ||
|
|
||
| /* block rendering until data ready */ | ||
| if (!ateam.ateam_id) return <Box sx={{ m: 2 }}>Loading...</Box>; |
There was a problem hiding this comment.
ateam.ateam_id が未定義の状況ってありますかね？ ( ateamはrequiredなpropsで渡されるので、存在しないパターンがあるのかが怪しい）
There was a problem hiding this comment.
required props でも undefined な可能性はあります（コンソールに警告は出ますが、それだけ）。
ただし、呼び出し側で if (!ateam) return ... してれば問題ない認識です。
どこまで fail safe にするべきかは全体的に揃えたいですね（props に undefined を含めないのは呼び出し側の責務とする、とか、逆に受け側で必ず undefined も想定しておく、とか）。
There was a problem hiding this comment.
私の案
・基本的に、props に undefined を含めないのは呼び出し側の責務とする
・例外的にprops に undefined を含めたい箇所のみ、呼ばれた側でチェックする。
その他の場合は呼ばれた側でのチェックは不要（呼び出し側でチェックされてることを確認するのが望ましいが）
There was a problem hiding this comment.
指摘の通り、ateamの存在チェックを削除しました。
また、その際気付きましたが、ateamなし時のretrunはBoxを返してるので、他のreturnもBoxを返すようにしました。
表示確認したところ、見栄えがよくなりました。
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PR の目的
従来、トピック編集時はpropsでpresetTopicIdを受け取りスライスからTopicを取得するが、トピック作成の時はこれを受け取らず、presetTopicIdが空かどうかで分岐していた。
修正後はpresetTopicIdの代わりにフェッチしたTopicデータをpresetTopicとして受け取り、これが空かどうかで分岐する。