Skip to content

refactor(DropdownTrigger): childrenを依存配列から削除しMutationObserverを使用#6408

Open
AtsushiM wants to merge 1 commit into
masterfrom
refactor/dropdown-trigger-remove-children-from-deps
Open

refactor(DropdownTrigger): childrenを依存配列から削除しMutationObserverを使用#6408
AtsushiM wants to merge 1 commit into
masterfrom
refactor/dropdown-trigger-remove-children-from-deps

Conversation

@AtsushiM

Copy link
Copy Markdown
Member

関連URL

なし

概要

eslint-config-smarthr@14.7.0で追加されたsmarthr/best-practice-for-unstable-dependenciesルールにより、useEffectの依存配列に不安定な参照であるchildrenが含まれていることが警告されるようになりました。

childrenは参照が頻繁に変わるため、依存配列に含めると不要な再実行や無限ループの原因となる可能性があります。

変更内容

1. childrenを依存配列から削除しMutationObserverを使用

  • useEffectの依存配列からchildrenを削除
  • MutationObserverを追加してtrigger要素のDOM構造の変更を監視
  • 内容が変わった際も、MutationObserverが検知して適切に再設定

2. onClickTriggerをmemoizedOnClickTriggerにリネーム

  • Dropdown.tsxonClickTriggermemoizedOnClickTriggerにリネーム
  • この関数がuseCallbackでメモ化されており、安定していることを明示
  • 将来のメンテナンス時に、この関数がメモ化されていることが期待されている設計意図を明確化

Before

// Dropdown.tsx
const onClickTrigger = useCallback((rect: Rect) => {
  // ...
}, [])

// DropdownTrigger.tsx
const { active, onClickTrigger, contentId, triggerElementRef } = useContext(DropdownContext)

useEffect(() => {
  // ...
  const callback = (e: MouseEvent) => {
    onClickTrigger((e.currentTarget! as HTMLButtonElement).getBoundingClientRect())
  }
  
  button.addEventListener('click', callback, CAPTURE_OPTION)
  
  return () => {
    button.removeEventListener('click', callback, CAPTURE_OPTION)
  }
}, [children, onClickTrigger, triggerElementRef]) // childrenが不安定な参照

After

// Dropdown.tsx
const memoizedOnClickTrigger = useCallback((rect: Rect) => {
  // ...
}, [])

// DropdownTrigger.tsx
const { active, memoizedOnClickTrigger, contentId, triggerElementRef } = useContext(DropdownContext)

useEffect(() => {
  let currentCleanup: (() => void) | undefined

  const setupButton = () => {
    currentCleanup?.()
    const button = triggerElement.querySelector<HTMLButtonElement>('button')
    // ...
    button.addEventListener('click', callback, CAPTURE_OPTION)
    currentCleanup = () => {
      button.removeEventListener('click', callback, CAPTURE_OPTION)
    }
  }

  setupButton()

  const observer = new MutationObserver(setupButton)
  observer.observe(triggerElement, {
    childList: true,
    subtree: true,
  })

  return () => {
    currentCleanup?.()
    observer.disconnect()
  }
}, [memoizedOnClickTrigger, triggerElementRef]) // childrenを削除、メモ化が明示的

プロダクト側で対応が必要な事項

なし

確認方法

  • 既存のテストが通ることを確認
  • Storybookでドロップダウンの動作を確認
  • 内容が動的に変わる場合の動作を確認

useEffectの依存配列にchildrenが含まれていると、不安定な参照のため
頻繁に再実行される問題がありました。

MutationObserverを使用してtrigger要素のDOM構造の変更を監視することで、
内容が変わった際も適切に検知しつつ、不要な再実行を防ぎます。

また、onClickTriggerをmemoizedOnClickTriggerにリネームすることで、
この関数がuseCallbackでメモ化されていることを明示し、
将来のメンテナンス時に設計意図を明確にしました。

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@pkg-pr-new

pkg-pr-new Bot commented Jun 18, 2026

Copy link
Copy Markdown

Open in StackBlitz

npm i https://pkg.pr.new/@smarthr/smarthr-ui-charts@6408
npm i https://pkg.pr.new/smarthr-ui@6408

commit: b3b525b

@AtsushiM AtsushiM marked this pull request as ready for review June 19, 2026 05:16
@AtsushiM AtsushiM requested a review from a team as a code owner June 19, 2026 05:16
@AtsushiM AtsushiM requested review from masa0527 and yt-ymmt and removed request for a team June 19, 2026 05:16
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.

1 participant