OSSCA Chromium

기여 목록 / 8514037

Remove jsdoc-migration suppressions from panels/timeline

DevToolsrefactordevtools/devtools-frontendMerged

panels/timeline에 남아 있던 crbug.com/1172300 주석 4건을 제거한 컨트리뷰션입니다. 직접 찾은 이슈입니다.

문제 설명

  • jsdoc → ts 마이그레이션 당시 임시로 넣은 lint/타입 체크 예외가 그대로 남아 있었음
  • 같은 crbug 번호를 공유하지만 성격이 네 가지로 갈림
// TODO(crbug.com/1172300) Ignored during the jsdoc to ts migration
// eslint-disable-next-line @typescript-eslint/no-explicit-any
private currentThreadSetting?: Common.Settings.Setting<any>;

해결 내용

  1. currentThreadSetting 필드 삭제
    • 처음에는 Setting<number> 로 타입을 지정했으나, 트리 전체에서 읽거나 쓰는 곳이 없었음
    • 생성과 change listener 등록만 있고, 값을 바꾸는 곳이 없으니 listener 도 실행될 일이 없음
    • 타입을 주는 대신 삭제
  2. groupBySetting 에 Setting<AggregatedTimelineTreeView.GroupBy> 지정
    • TimelineTreeView 가 같은 설정을 이미 그 타입으로 선언하고 있어 일관성을 맞춤
  3. TimelineFilmStripOverview.Padding → PADDING 개명
    • eslint-disable이 붙어 있던 유일한 건
    • 같은 파일 내 참조 2곳 함께 수정
  4. networkSplitWidget의 TODO 주석 제거
    • eslint-disable도 없는 맨 주석이고 타입도 이미 명시되어 있었음

TimelineHistoryManager.onClick의 @ts-expect-error 1건은 별도 CL(8513859, #645)로 처리했습니다.

테스트 방법

  1. panels/timeline:timeline 및 :unittests 빌드
  2. TimelineTreeView.test.ts + TimelineFlameChartView.test.ts + TimelineEventOverview.test.ts → 47/47 통과
  3. 동작 변경 없음 (타입 선언과 상수명 변경에 한정)

배운 점

  • 같은 TODO 번호가 붙어 있어도 해결 방법은 제각각
    • 네 건 중 둘은 타입 지정, 하나는 개명, 하나는 단순 삭제였고 나머지 하나는 런타임 가드가 필요해 분리 대상이었음
    • 번호 단위로 묶어 한 번에 처리하려 하면 성격이 다른 변경이 한 CL에 섞임
  • any의 실제 타입은 대개 선언부가 아니라 값이 만들어지는 곳에 있음
    • 두 Setting 모두 createSetting() 호출의 기본값이 타입을 알려주고 있었음

참고 자료

  • third_party/devtools-frontend/src/front_end/panels/timeline/TimelineTreeView.ts
  • third_party/devtools-frontend/src/front_end/panels/timeline/TimelineFlameChartView.ts
  • third_party/devtools-frontend/src/front_end/panels/timeline/TimelineEventOverview.ts

리뷰 대응

  • Jack Franklin 님, Mathias Bynens 님 +1 후 머지
  • 사용하지 않는 필드에 타입을 붙이던 것을 삭제로 바꾼 버전이 바로 승인됨
  • Bug: 를 Monorail ID(1172300)에서 Buganizer ID(40166562)로 수정
  • 첫 CQ 는 main 의 E2E 장애로 실패했고, 트리가 복구된 뒤 재실행으로 머지