Conversation
mattcosta7
temporarily deployed
to
github-pages-preview
GitHub Actions
Inactive
There was a problem hiding this comment.
Pull request overview
Updates dependency versions across the monorepo (website + storybook addon) and makes small internal adjustments in the performance panel code to align with the upgraded toolchain/runtime.
Changes:
- Bump website build deps (Vite + plugin-react) and UI/router deps (@primer/react, @tanstack/react-*).
- Bump storybook addon dev tooling (Vitest, tsdown, plugin-react) and update internal profiler/input collector code.
- Update repo-level tooling (TypeScript, typescript-eslint) and adjust npm overrides/devDependency aliasing for
h3.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/website/package.json | Dependency bumps for the website build/runtime stack (Vite/plugin-react, Primer, TanStack). |
| packages/storybook-addon-performance-panel/react/react-profiler-wrapper.tsx | Refactor metrics ref initialization and add null-guards around store updates. |
| packages/storybook-addon-performance-panel/package.json | Dev dependency bumps for building/testing the addon. |
| packages/storybook-addon-performance-panel/collectors/input-collector.ts | Adjust interactionCount retrieval logic in Event Timing processing. |
| package.json | Update TS/tooling versions and modify h3 override strategy via h3-v2 alias. |
| package-lock.json | Lockfile updates reflecting dependency/tooling upgrades. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| }: ReactProfilerWrapperProps) { | ||
| // Store metrics in a ref to avoid re-renders on metric updates | ||
| const metricsRef = useRef<ProfilerMetricsRef>(createMetricsRef()) | ||
| const metricsRef = useRef<ProfilerMetricsRef>(null) |
There was a problem hiding this comment.
useRef<ProfilerMetricsRef>(null) is not type-safe under strict/strictNullChecks (null isn’t assignable to ProfilerMetricsRef). Change the ref type to include null (or initialize with a non-null value) so this compiles without relying on unsound casts, and then you can avoid repeatedly guarding metricsRef.current at each use-site.
| const metricsRef = useRef<ProfilerMetricsRef>(null) | |
| const metricsRef = useRef<ProfilerMetricsRef | null>(null) |
| // Update interaction tracking - prefer browser's count if available | ||
| const perfWithEventTiming = performance as PerformanceWithEventTiming | ||
| this.#interactionCount = perfWithEventTiming.interactionCount ?? this.#interactionMap.size | ||
| const perfWithEventTiming = performance | ||
| this.#interactionCount = perfWithEventTiming.interactionCount | ||
| addToWindow(this.#interactionLatencies, duration, INTERACTION_LATENCIES_WINDOW) |
There was a problem hiding this comment.
performance.interactionCount is not part of the standard Performance type (and may be undefined at runtime). As written, this will fail TypeScript compilation under strict, and it can also assign undefined into #interactionCount even though it’s typed as number. Consider reintroducing a narrow type assertion/augmentation for interactionCount?: number and keep a numeric fallback (e.g., to this.#interactionMap.size) when the browser value is unavailable.
No description provided.