fix: improve video loading compatibility

[ChengYi996]

原因：只调用 video.load() 而不调用 play()，canplay 事件可能不会触发

Summary by CodeRabbit

Release Notes

• Bug Fixes
  • Improved video loading reliability with enhanced event handling and automatic cleanup mechanisms
  • Better detection and notification of autoplay restrictions, displaying warnings when browsers block automatic video playback
  • Strengthened error handling to prevent multiple promise resolutions during video load operations

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

📝 Walkthrough

Walkthrough

The loadVideo function in downloader.ts has been refactored to improve promise handling reliability. Event listener logic was restructured to use multiple success triggers (loadeddata and canplay), error handling, and a cleanup mechanism. The function now calls video.play() instead of video.load() and includes autoplay-blocking detection.

Changes

Cohort / File(s)	Summary
Video Loading Promise Refactoring packages/effects-core/src/downloader.ts	Reworked promise resolution logic with settled flag to prevent multiple resolutions; replaced canplay/once listener with dual success triggers (loadeddata, canplay); added error listener; introduced cleanup function for event listener removal; changed from video.load() to video.play() with autoplay-blocked warning logging

Sequence Diagram(s)

sequenceDiagram
    participant Caller
    participant Promise as loadVideo Promise
    participant Video as Video Element
    participant Handler as Event Handlers
    
    Caller->>Promise: Call loadVideo(url)
    Note over Promise: Create promise, set settled = false
    Promise->>Video: Set src attribute
    Promise->>Video: Attach loadeddata listener
    Promise->>Video: Attach canplay listener
    Promise->>Video: Attach error listener
    Promise->>Video: Call video.play()
    
    alt Autoplay Blocked
        Video-->>Promise: (play promise rejects)
        Note over Promise: Log autoplay blocked warning
    end
    
    alt Success Path
        Video->>Handler: loadeddata event fires
        Handler->>Handler: Check settled flag
        Handler->>Promise: handleSuccess()
        Note over Promise: settled = true
        Promise->>Video: Remove all listeners (cleanup)
        Promise->>Caller: Resolve with video element
    else or
        Video->>Handler: canplay event fires
        Handler->>Handler: Check settled flag
        Handler->>Promise: handleSuccess()
        Note over Promise: settled = true
        Promise->>Video: Remove all listeners (cleanup)
        Promise->>Caller: Resolve with video element
    else Error Path
        Video->>Handler: error event fires
        Handler->>Handler: Check settled flag
        Handler->>Promise: handleError()
        Note over Promise: settled = true
        Promise->>Video: Remove all listeners (cleanup)
        Promise->>Caller: Reject with error message
    end

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

🐰 A little bunny hops with glee,
Video promises settled they'll be!
With listeners trimmed and cleanup so neat,
The autoplay tale is bittersweet! 🎬✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix: improve video loading compatibility' directly and clearly describes the main change: fixing a video loading compatibility issue by improving how video loading is handled.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/downloader

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

packages/effects-core/src/downloader.ts (1)

314-316: Video may remain playing after promise resolves.

Calling video.play() to trigger loading (instead of video.load()) is a reasonable fix for the compatibility issue. However, if play() succeeds, the video will be actively playing when the promise resolves. Since the function is named loadVideo, callers might expect a preloaded but paused video.

Consider pausing after successful load if the intent is purely to preload:

🔎 Suggested fix

     const handleSuccess = () => {
       if (settled) {return;}
       settled = true;
       cleanup();
+      video.pause();
       resolve(video);
     };

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 350e586 and 2bf74af.

📒 Files selected for processing (1)

• packages/effects-core/src/downloader.ts

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: Analyze (javascript-typescript)

🔇 Additional comments (1)

packages/effects-core/src/downloader.ts (1)

288-308: Well-structured promise handling with proper cleanup.

The settled flag pattern correctly prevents multiple resolutions, and the cleanup function properly removes all event listeners. Using both loadeddata and canplay events improves compatibility across different browsers.