기여 목록 / 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>;해결 내용
currentThreadSetting필드 삭제- 처음에는
Setting<number>로 타입을 지정했으나, 트리 전체에서 읽거나 쓰는 곳이 없었음 - 생성과 change listener 등록만 있고, 값을 바꾸는 곳이 없으니 listener 도 실행될 일이 없음
- 타입을 주는 대신 삭제
- 처음에는
groupBySetting에Setting<AggregatedTimelineTreeView.GroupBy>지정TimelineTreeView가 같은 설정을 이미 그 타입으로 선언하고 있어 일관성을 맞춤
TimelineFilmStripOverview.Padding→PADDING개명eslint-disable이 붙어 있던 유일한 건- 같은 파일 내 참조 2곳 함께 수정
networkSplitWidget의 TODO 주석 제거eslint-disable도 없는 맨 주석이고 타입도 이미 명시되어 있었음
TimelineHistoryManager.onClick의 @ts-expect-error 1건은 별도 CL(8513859, #645)로 처리했습니다.
테스트 방법
panels/timeline:timeline및:unittests빌드TimelineTreeView.test.ts+TimelineFlameChartView.test.ts+TimelineEventOverview.test.ts→ 47/47 통과- 동작 변경 없음 (타입 선언과 상수명 변경에 한정)
배운 점
- 같은 TODO 번호가 붙어 있어도 해결 방법은 제각각
- 네 건 중 둘은 타입 지정, 하나는 개명, 하나는 단순 삭제였고 나머지 하나는 런타임 가드가 필요해 분리 대상이었음
- 번호 단위로 묶어 한 번에 처리하려 하면 성격이 다른 변경이 한 CL에 섞임
any의 실제 타입은 대개 선언부가 아니라 값이 만들어지는 곳에 있음- 두
Setting모두createSetting()호출의 기본값이 타입을 알려주고 있었음
- 두
참고 자료
third_party/devtools-frontend/src/front_end/panels/timeline/TimelineTreeView.tsthird_party/devtools-frontend/src/front_end/panels/timeline/TimelineFlameChartView.tsthird_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 장애로 실패했고, 트리가 복구된 뒤 재실행으로 머지