customHoldingView を必須 props にして null 混入を型で防ぐ - #87
Open
ryuuji wants to merge 1 commit into
Open
Conversation
Index の defaultProps は値が undefined のときしか効かないため、サイト設定 (window.options)から明示的に null が渡ると Book の描画時に例外になっていた。 Index が customHoldingView を渡す際に DefaultHoldingView へ null 合体させ、 Results / Book 側は必須 props にすることで、この経路を型で保証する。
This file contains hidden or 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
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.
背景
src/js/view/book.tsxで差し替え可能なコンポーネントを3つ受け取っているが、扱いが揃っていなかった。customDetailViewとcustomNotFoundViewはifで存在を確かめてから描画するが、customHoldingViewだけ確かめずに描画していた(非 null アサーション頼み)。
IndexのdefaultPropsがDefaultHoldingViewを補うため通常は必ず値があるが、React のdefaultPropsは値がundefinedのときにしか効かない。サイト設定(window.options)がcustomHoldingViewに明示的にnullを渡した場合は既定値が入らず、Bookの描画時に<CustomHoldingView .../>がnullコンポーネントとして例外になる。対応方針
「他の2つと同じくガードして未指定時は何も描かない」か「Props で必須にして呼び出し側に保証させる」かの
どちらかを決める必要があった。所蔵リンクは検索結果の中核機能で、ガード側を選ぶとサイト設定の
不備が所蔵リンクの黙った消失として現れてしまう(気づきにくい)。そのため後者を採用し、
根本原因(
defaultPropsがnullを素通しする)そのものを塞いだ。変更内容
Index:Resultsへ渡す際にthis.props.customHoldingView ?? DefaultHoldingViewでnull 合体させ、
undefined/nullのどちらが来ても既定値を保証するResults/Book:customHoldingViewを必須 props(non-null)にし、Book側の非 null アサーション(
!)を撤去姉妹リポジトリ(unitrad-view / unitrad-kintone-plugin / littel-ui / unitrad-ui-nagano)にも
同じ修正を入れている。
Test plan
npm run typechecknpm test(190件全て成功)Claude Codeで作業: https://claude.ai/code/session_01XBWJb9rhvLz6Mz6rwySUwC