-
Notifications
You must be signed in to change notification settings - Fork 516
Fix browser compatibility: guard requestIdleCallback and startViewTransition #1464
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,6 +22,11 @@ export default function ThemeToggle() { | |
| return; | ||
| } | ||
|
|
||
| if (typeof document.startViewTransition !== "function") { | ||
| setTheme(nextTheme); | ||
| return; | ||
| } | ||
|
Comment on lines
+25
to
+28
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The new Prompt To Fix With AIThis is a comment left during a code review.
Path: apps/dashboard/src/components/theme-toggle.tsx
Line: 25-28
Comment:
**Redundant guard — already covered by `supportsViewTransitions()`**
The new `typeof document.startViewTransition !== "function"` check is unreachable in practice. The existing `supportsViewTransitions()` call on line 20 uses `"startViewTransition" in document`, which already returns `false` whenever the API is absent, causing an early return before this block is ever evaluated. The only case where this new guard would fire is if `startViewTransition` is present on `document` but isn't a function — a scenario that doesn't exist in any real browser. Consider either updating `supportsViewTransitions()` to use `typeof document.startViewTransition === "function"` and removing this guard, or simply removing this redundant block.
How can I resolve this? If you propose a fix, please make it concise. |
||
|
|
||
| document.documentElement.classList.add("vt-disable-transitions"); | ||
|
|
||
| const transition = document.startViewTransition(() => { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Redundant startViewTransition check duplicates existing guard
Low Severity
The new
typeof document.startViewTransition !== "function"check on lines 25-28 is redundant dead code. The existingsupportsViewTransitions()function (line 8-10) already checks"startViewTransition" in document, and the early return on line 20 ensures execution only reaches line 25 when that property exists. In all real browser implementations, when"startViewTransition" in documentistrue, the property is always a function, making this second guard unreachable.Reviewed by Cursor Bugbot for commit b5232a1. Configure here.