fix: make disableCookies reactive - #1349
Open
R-Delfino95 wants to merge 4 commits into
Open
Conversation
…s after hydration
|
@R-Delfino95 is attempting to deploy a commit to the Mux Team on Vercel. A member of the Team first needs to authorize it. |
✅ 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. |
R-Delfino95
marked this pull request as ready for review
August 4, 2026 14:50
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
decepulis
requested changes
Aug 6, 2026
decepulis
left a comment
Contributor
There was a problem hiding this comment.
This seems ok to me, but I think it should include mux-audio, too!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related to #1193 — that one made
disableTrackingreactive,disableCookieswas left behind.Description
disableCookiesgates the persistentmuxDatacookie that players write through their built-in Mux Data. Today it only takes effect when a player initialises. Changing it afterwards doesn't work in either direction:mux_viewer_idnever comes back for the rest of the page.That's a problem for consent flows. A consent manager answers after the page has rendered, so the value is almost always set after the player exists. The only way around it today is to not render the player until consent is known, which means it can't be server-rendered.
This PR makes the prop reactive, so a player can be rendered up front and told about consent later.
Why it wasn't reactive
Mux Data reads
disableCookiesonce, when its monitor is created, and there's no setter for it afterwards. So the only way to apply a change is to create a new monitor.videojs/v10reaches the same conclusion in itsMuxDatacomponent, and this follows its shape — including its guards, so the two implementations stay comparable.How it works
When
disableCookieschanges, Mux Data is re-attached: the old monitor is torn down and a new one is created with the new value. The<video>element and the hls.js instance are left alone, so playback isn't interrupted and anything attached to the element imperatively survives — chapters, caption tracks, listeners. That's the difference from thedisable-trackingbranch right above it, which reloads the media and loses all of that.Turning cookies back on reuses the existing
muxDatacookie if it's still there, so a returning visitor keeps their identity.Two behaviours worth being explicit about:
A change while a video is playing splits the Mux Data view in two — one
viewend, one newviewstart. It's inherent to re-creating the monitor, and it's the same behaviour asvideojs/v10. Applying the change before playback starts costs nothing, because Mux Data only closes a view that had already started, and that's the common case for a consent banner.A player that initialises with cookies disabled no longer deletes an existing cookie. It used to, and that turned out to be harmful: a server-rendered page can't read consent, so it always renders the cookie-less state, and deleting there wiped a returning visitor's
mux_viewer_idmoments before the client granted consent. Mux Data then minted a new identity on every page load. The element can't tell "the visitor declined" from "consent isn't known yet" — same markup, opposite meaning — so it now leaves the cookie alone and never writes to it. Apps that need a stale cookie cleared should do it themselves, where the real consent state is known.The React side
Making the element reactive isn't enough for server-rendered pages, because React never reconciles attributes during hydration — it warns that the mismatch "won't be patched up" and leaves the server's value in the DOM. So a server-rendered
disable-cookieswould stay there forever and the player would be stuck in the denied state.mux-player-reactand the@mux/mux-video/reactwrapper now also applydisableCookiesas a property after mount, which is whatmux-player-reactalready does forplaybackId,metadataand friends. That's the piece that makes a server-rendered, consent-gated player actually work.@mux/mux-video-reactis a separate implementation with no custom element, so it got its own equivalent: an effect that watches the prop.Testing
13 unit tests in
packages/mux-video/test/index.test.js(46 in the file now), covering both directions, no media reload, the cookie left alone at init, several changes in one tick collapsing into a single monitor, a change reverted in the same tick, and the cookie being cleared even when a monitor flushes its final beacon late or when several players are disabled at once.Verified manually with real playback on
<mux-video>,<mux-player>,@mux/mux-video/reactand@mux/mux-video-react, plus a server-render-and-hydrate harness on React 18 and 19:mux_viewer_idback within a second, reusing the existing one.emptied/loadstart/waitingevents on any change, and the playhead never stops.TextTrackobject identity survive every change.mux_viewer_idacross reloads.viewstartand zeroviewendper player when consent resolves before playback starts.Note
Medium Risk
Touches Mux Data lifecycle, shared
muxDatacookies, and analytics view boundaries on mid-playback toggles; behavior is well-tested but affects privacy/consent and viewer identity.Overview
Makes
disableCookiesreact to updates after the player loads, so consent managers can flip cookie behavior without tearing down playback.playback-core adds
applyDisableCookies,reinitMuxData, andclearMuxDataCookies: when the value actually changes, Mux Data is torn down and re-monitored with the new setting (hls.js and the<video>stay put). Same-tick bursts coalesce to one re-init; enabling cookies at init no longer wipes an existingmuxDatacookie (SSR “unknown consent” case). Runtime disable clears cookies after monitor teardown flushes, including multi-player races.<mux-video>swaps inline cookie deletion forapplyDisableCookieson thedisable-cookiesattribute.React:
mux-player-reactsyncsdisableCookiesviauseObjectPropEffect;@mux/mux-video/reactre-applies the property after mount to fix hydration;mux-video-reactwatches the prop in auseEffect.Adds a large
disable-cookiesunit test suite on<mux-video>.Reviewed by Cursor Bugbot for commit ac0fc79. Bugbot is set up for automated code reviews on this repo. Configure here.