fix: <vimeo-video> element race condition#220
fix: <vimeo-video> element race condition#220spuppo-mux wants to merge 4 commits intomuxinc:mainfrom
<vimeo-video> element race condition#220Conversation
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
|
@spuppo-mux is attempting to deploy a commit to the Mux Team on Vercel. A member of the Team first needs to authorize it. |
|
Closes #219 |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
| this.#loadRequested = null; | ||
|
|
||
| if (this.#hasLoaded) this.loadComplete = new PublicPromise(); | ||
| this.#hasLoaded = true; // TODO: Identify how hasLoaded differs from isInit |
There was a problem hiding this comment.
Reordered loadComplete refresh enables null api dereference
Medium Severity
Moving if (this.#hasLoaded) this.loadComplete = new PublicPromise() from before the 1-tick await to after it introduces a timing regression on second+ loads. When src and loop change in the same synchronous block (common in React/Next.js), the loop handler's await this.loadComplete resolves immediately on the stale promise. Its continuation then runs after load() sets this.api = null, causing this.api.setLoop(...) to throw a TypeError. Previously, loadComplete was refreshed before the await, so the loop handler would correctly suspend until the new load completed.


By checking logs I saw that when the bug happened, we were getting into the
isInit === falsecase (and load was running just one time), so the options weren't being applied.Tested this mainly on nextjs example. Could originally reproduce semi-consistently by switching to TikTok player and then back to vimeo. When the bug happens we see the Vimeo default color (blueish)


When the config applies correctly we should see a pinkish color
To address the race condition:
await (this.#loadRequested = Promise.resolve());to the top of the functiondisconnectedCallbackto clear state on unmount.Extra:
#setupApiListenersfunction#onLoadedlistener to a new method since it's now used from two functionsloadfunction so it's clearer to read.isInitandhasLoadedare pretty similar semantically, was wondering if one of them can be removed.optionsto new VimeoPlayerAPI call (did not seem to have effect even though it's specified in the JSdocs)Note
Medium Risk
Changes the
vimeo-video-elementload sequencing and lifecycle state reset, which can affect initialization timing and event ordering across mounts/unmounts.Overview
Fixes a race in
VimeoVideoElement.load()by moving the one-tick#loadRequestedgate earlier, so attribute/config updates settle before initialization and options are reliably applied.Refactors load completion handling by extracting
#onLoaded()and listener wiring into#setupApiListeners(), passingoptionsintonew VimeoPlayerAPI(...), and addingdisconnectedCallback()to clear internal init/load flags and resetloadCompletewhen the element unmounts.Written by Cursor Bugbot for commit 359a7fc. This will update automatically on new commits. Configure here.