OSSCA Chromium

기여 목록 / 8513498

Type the visibleView check in ProfilesPanel.searchableView

DevToolsrefactordevtools/devtools-frontendMerged

panels/profiler/ProfilesPanel.ts에 남아 있던 crbug.com/1172300 주석 1건을 제거한 컨트리뷰션입니다. #715 의 후속입니다.

문제 설명

override searchableView(): UI.SearchableView.SearchableView|null {
  // eslint-disable-next-line @typescript-eslint/no-explicit-any
  const visibleView = (this.visibleView as any);
  return visibleView?.searchableView ? visibleView.searchableView() : null;
}
  • visibleView 는 UI.Widget.Widget|undefined 이고 searchableView() 를 선언하지 않음
  • 프로필 뷰마다 이 메서드의 유무가 다름
    • 있음: HeapProfileView:637, HeapSnapshotView:562
    • 없음: launcher view
  • 즉 메서드 존재 여부를 런타임에 확인하는 덕 타이핑 코드

해결 내용

탐색 대상 형태를 인터페이스로 선언하고 캐스팅했습니다.

interface MaybeSearchableView {
  searchableView?(): UI.SearchableView.SearchableView|null;
}

const visibleView = this.visibleView as MaybeSearchableView | undefined;
  • 메서드를 선택적으로 선언해 Partial<> 없이 처리
  • 가드 함수를 따로 만들지 않고 캐스팅 한 줄로 끝냈음

의미 보존을 위해 피한 것

typeof === 'function' 가드를 쓰지 않았습니다.

  • 원본은 프로퍼티의 truthiness 만 검사하므로, 호출 불가능한 truthy 값이 있으면 호출하다 던짐
  • typeof 가드를 넣으면 그 경우 조용히 null 을 반환하게 되어 동작이 바뀜
  • 더 "올바른" 코드로 보이지만 동작 변경에 해당해서 범위 밖으로 두었음

테스트 방법

  1. panels/profiler:unittests 빌드
  2. panels/profiler 테스트 → 11/11 통과
  3. 동작 변경 없음

배운 점

  • 타입 주석만으로는 안 되고 캐스팅이 필요했음
    • MaybeSearchableView 는 멤버가 전부 선택적인 weak type 이라, Widget 과 공통 프로퍼티가 없어 TS 가 대입을 거부함(TS2559)
  • 포매터가 캐스팅 위치의 union 과 타입 주석 위치의 union 을 다르게 다룸
    • git cl format 은 A|undefined, presubmit 포매터는 A | undefined 를 요구
    • presubmit 이 게이트이므로 그쪽 출력을 따랐음

참고 자료

  • third_party/devtools-frontend/src/front_end/panels/profiler/ProfilesPanel.ts

리뷰 대응

  • Simon Zünd 님, Wolfgang Beyer 님 +1 후 Simon 님 CQ+2 로 머지
  • 처음 올린 패치셋은 Partial<ViewWithSearchableView> 였음
    • presubmit 포매터가 Partial<X>| undefined 라는 어색한 출력을 요구
    • Partial<> 을 없애고 인터페이스에서 메서드를 선택적으로 선언하는 쪽으로 바꿔 해소
  • 커밋 메시지 72자 초과 경고를 처음 받음
    • Chromium 은 커밋 메시지 줄을 72자로 제한
    • 메시지만 수정하면 Code-Review 표가 유지됨 (copy condition 에 NO_CODE_CHANGE 포함)
목록으로