Guard window access for non-browser runtimes (#3735)#3751
Conversation
Several motion-dom functions accessed `window` without a `typeof window` guard, throwing in non-browser JS runtimes — or when `window` is shadowed to undefined by an IIFE wrapper (e.g. Lynx's web runtime). The immediate crash reported was `animateTarget` reading `window.MotionHandoffAnimation`, but the same unguarded access existed across projection, keyframe resolution, computed-style reads, gestures, resize handling and the scroll-timeline support checks. Add `typeof window !== "undefined"` guards at each site, matching the existing pattern in render/utils/reduced-motion. `typeof` never throws on an undefined binding, and every guard evaluates to true in a browser, so existing behaviour is unchanged. Fixes #3735 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Greptile SummaryThis PR adds
Confidence Score: 4/5Safe to merge for browser consumers; gesture handlers in hover.ts and press/index.ts have a minor state-stuck edge case that only affects non-browser runtimes with active pointer events All guards are additive no-ops in browsers. The only non-trivial behavioural gap is in hover.ts and press/index.ts: when window is undefined but pointer events can still fire (e.g., Lynx's UI layer), the window-level pointerup/pointercancel cleanup listeners are never registered, so isPressed and isPressing can get stuck after a pointerdown. This doesn't affect any browser path and is a degraded-but-non-crashing state in the exact exotic environments this PR targets. gestures/hover.ts and gestures/press/index.ts — the isPressed/isPressing cleanup gap is worth a follow-up if Lynx (or similar UI-capable runtimes) is a first-class target Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[animateTarget called] --> B{typeof window !== 'undefined'?}
B -- No --> C[Skip MotionHandoffAnimation check\nReturn no-op animation]
B -- Yes --> D[Read window.MotionHandoffAnimation\nProceed normally]
E[hover / press pointerdown] --> F{typeof window !== 'undefined'?}
F -- No --> G[Set isPressed=true\nSkip window pointerup/cancel listeners\nstate may get stuck]
F -- Yes --> H[Register cleanup on window\nNormal gesture flow]
I[createWindowResizeHandler] --> J{typeof window !== 'undefined'?}
J -- No --> K[Return early\nwindowResizeHandler stays undefined]
J -- Yes --> L[Register resize listener\nNormal resize flow]
M[supportsScrollTimeline] --> N{typeof window !== 'undefined'?}
N -- No --> O[Return false / memoize false]
N -- Yes --> P[Check window.ScrollTimeline\nMemoize result]
Reviews (1): Last reviewed commit: "Guard window access for non-browser runt..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const onPointerDown = () => { | ||
| isPressed = true | ||
| window.addEventListener( | ||
| "pointerup", | ||
| onPointerUp as EventListener, | ||
| eventOptions | ||
| ) | ||
| window.addEventListener( | ||
| "pointercancel", | ||
| onPointerUp as EventListener, | ||
| eventOptions | ||
| ) | ||
| if (typeof window !== "undefined") { | ||
| window.addEventListener( | ||
| "pointerup", | ||
| onPointerUp as EventListener, | ||
| eventOptions | ||
| ) | ||
| window.addEventListener( | ||
| "pointercancel", | ||
| onPointerUp as EventListener, | ||
| eventOptions | ||
| ) | ||
| } |
There was a problem hiding this comment.
isPressed = true
When window is undefined (e.g., Lynx's main-thread runtime), onPointerDown sets isPressed = true but skips registering the pointerup/pointercancel cleanup on window. The only path that resets isPressed = false is onPointerUp, which is only registered on window. Subsequent pointerleave events will therefore always take the deferred-hover-end branch (deferredHoverEnd = true; return) and never call endHover, leaving the element permanently stuck in a "hovering while pressed" state for the rest of the element's lifetime. The same pattern exists in press/index.ts where isPressing can retain the target indefinitely once a pointerdown fires without a corresponding window-level cleanup.
1 participant
Bug
In non-browser JS runtimes — or when
windowis shadowed toundefinedby an IIFE wrapper (e.g. Lynx's web runtime, which wraps main-thread scripts in(function(){ const window = void 0; ... })()) —motion-domthrew:The immediate crash was in
animateTarget(animation/interfaces/visual-element-target.ts), which readwindow.MotionHandoffAnimationwithout atypeof windowguard. The same unguardedwindowaccess existed across a number of other modules, so fixing onlyanimateTargetwould just move the crash one step downstream.Cause
Bare
window.<prop>accesses assume a browser global. Whenwindowisundefined(or an undeclared binding) the property read throws. Other modules already do this correctly — e.g.render/utils/reduced-motion/index.tsusestypeof window !== "undefined".Fix
Add a
typeof window !== "undefined"guard before each unguardedwindowaccess.typeofnever throws, even on an undefined binding. Crucially, every guard evaluates totruein a browser, so existing behaviour is unchanged — these are no-ops outside non-browser runtimes.Guarded sites:
animation/interfaces/visual-element-target.ts—window.MotionHandoffAnimation(the reported crash)projection/node/create-projection-node.ts—MotionHasOptimisedAnimation,MotionCancelOptimisedAnimation,innerWidthprojection/node/HTMLProjectionNode.ts—mount(window),getComputedStylerender/dom/style-computed.ts&render/html/HTMLVisualElement.ts—getComputedStyleanimation/keyframes/DOMKeyframesResolver.ts—pageYOffset,getComputedStyleanimation/keyframes/KeyframesResolver.ts—scrollToanimation/utils/css-variables-conversion.ts—getComputedStylegestures/hover.ts&gestures/press/index.ts—addEventListener/removeEventListener(the [BUG]When using the drag gesture and calling stopPropagation inside onPointerUp, the scroll element starts following the cursor. #2794 capture-phase logic is preserved)resize/handle-window.ts—innerWidth/innerHeight,addEventListenerutils/supports/scroll-timeline.ts—ScrollTimeline/ViewTimelinerender/VisualElement.ts(MotionCheckAppearSync) was already guarded, so it's untouched.projection/node/types.tswidensdefaultParentto() => IProjectionNode | undefined(the sole caller already handlesundefined).Test
animation/interfaces/__tests__/visual-element-target.test.tsruns in thenodetest environment (wherewindowis genuinelyundefined, matching the reported runtime) and assertsanimateTargetno longer throws. It fails against the unmodified code withwindow is not definedat the exact crash line, and passes with the guard.Verification
motion-dom: builds clean (tsc + rollup + bundle-size check), all unit suites pass incl. the new regression testframer-motion: builds clean (tsc --noEmitOnError), all 94 client suites + SSR suites pass — confirms no public-API or behavioural regressionFixes #3735
🤖 Generated with Claude Code