fix(web): add requestIdleCallback fallback for Safari/iOS#9094
Conversation
|
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds a reusable idle-task helper and integrates it into RenderIfVisible: the prop was renamed to ChangesIdle task scheduling and observer fixes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
apps/web/core/components/core/render-if-visible-HOC.tsx (2)
26-32: ⚡ Quick winHarden fallback path to avoid direct
windowusage.Line 31 still assumes
windowexists. If this helper is reused in a non-browser context, it can throw. PreferglobalThis.setTimeoutfor a truly safe fallback.Proposed change
const runIdleTask = (callback: () => void) => { if (typeof window !== "undefined" && typeof window.requestIdleCallback === "function") { window.requestIdleCallback(callback, { timeout: 300 }); return; } - window.setTimeout(callback, 0); + globalThis.setTimeout(callback, 0); };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/core/components/core/render-if-visible-HOC.tsx` around lines 26 - 32, The fallback in runIdleTask still calls window.setTimeout which can throw in non-browser contexts; update the fallback to use globalThis.setTimeout (or guard for typeof globalThis.setTimeout === "function") instead of window.setTimeout, keeping the existing check for window.requestIdleCallback and leaving requestIdleCallback usage unchanged; reference the runIdleTask function and the window.requestIdleCallback branch when making this change.
79-79: ⚡ Quick winRemove
childrenfrom observer effect dependencies.Line 79 unnecessarily ties observer lifecycle to
childrenidentity, which can recreate observers on normal rerenders. Keep dependencies to values that affect observer setup.Proposed change
- }, [intersectionRef, children, root, verticalOffset, horizontalOffset, useIdletime]); + }, [intersectionRef, root, verticalOffset, horizontalOffset, useIdletime]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/core/components/core/render-if-visible-HOC.tsx` at line 79, The effect that creates the IntersectionObserver in RenderIfVisibleHOC (the useEffect that references intersectionRef, root, verticalOffset, horizontalOffset, useIdletime) should not include `children` in its dependency array because that causes unnecessary observer teardown/recreation on normal rerenders; update the dependency array to remove `children` so it only depends on stable values that affect observer setup (intersectionRef, root, verticalOffset, horizontalOffset, useIdletime) and ensure any linter warnings are addressed by keeping referenced variables stable or explicitly documenting why `children` is excluded.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@apps/web/core/components/core/render-if-visible-HOC.tsx`:
- Around line 26-32: The fallback in runIdleTask still calls window.setTimeout
which can throw in non-browser contexts; update the fallback to use
globalThis.setTimeout (or guard for typeof globalThis.setTimeout === "function")
instead of window.setTimeout, keeping the existing check for
window.requestIdleCallback and leaving requestIdleCallback usage unchanged;
reference the runIdleTask function and the window.requestIdleCallback branch
when making this change.
- Line 79: The effect that creates the IntersectionObserver in
RenderIfVisibleHOC (the useEffect that references intersectionRef, root,
verticalOffset, horizontalOffset, useIdletime) should not include `children` in
its dependency array because that causes unnecessary observer
teardown/recreation on normal rerenders; update the dependency array to remove
`children` so it only depends on stable values that affect observer setup
(intersectionRef, root, verticalOffset, horizontalOffset, useIdletime) and
ensure any linter warnings are addressed by keeping referenced variables stable
or explicitly documenting why `children` is excluded.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 16539ca9-e76f-4879-bca1-7bd08e2c45ad
📒 Files selected for processing (1)
apps/web/core/components/core/render-if-visible-HOC.tsx
bubacho
force-pushed
the
fix/safari-request-idle-callback
branch
from
62134f5 to
d048ed2
Compare
|
@bubacho A few small suggestions while we're already touching this area:
|
Thanks for the suggestions — addressed in 73c0d10. Renamed useIdletime to useIdleTime. |
bubacho
force-pushed
the
fix/safari-request-idle-callback
branch
from
73c0d10 to
eb0edcd
Compare
3 participants
Summary
Fixes a runtime crash on Safari/iOS where
window.requestIdleCallbackis not available.Type of change
Problem
On iPhone Safari, opening a project issues page can crash with:
TypeError: window.requestIdleCallback is not a function.The crash happens in
RenderIfVisibleand can break project/issue views.Changes
runIdleTaskhelper:window.requestIdleCallbackwhen availableglobalThis.setTimeout(..., 0)otherwisewindow.requestIdleCallbackcalls withrunIdleTasktypeof window !== "undefined"IntersectionObservercleanup to unobserve a stabletargetreferencechildrenfrom the observer effect dependency list to avoid unnecessary observer recreationTest scenarios
Screenshots / media
N/A (runtime compatibility fix).
Why this is safe
requestIdleCallbackSummary by CodeRabbit
Performance
Bug Fixes