Conversation
`this.videojs.trigger({ type: _event, eventData: data })` only passes
one argument to trigger(), so the payload never reaches listeners as
their second `data` argument. Every extended event (percentsplayed,
timeplayed, playerload, start, seek, pausenoseek, mute, unmute,
qualitychanged) fired with `data` undefined, breaking any listener
(including the analytics plugin) that destructures it.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
✅ Deploy Preview for cld-video-player ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for cld-vp-esm-pages ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
The prior assertion spied on trigger()'s arguments and expected the
buggy single-object shape ({ type, eventData }), so it passed against
the broken code and started failing once the fix corrected the call
to trigger(_event, data). Assert what a real listener receives instead
- the actual behavior that was broken.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This branch has not been deployed
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.
Summary
_setExtendedEvents()forwarded internal extended-event payloads to the public player event bus viathis.videojs.trigger({ type: _event, eventData: data }), passing only one argument totrigger().videojs.trigger(event, hash)delivershashas the second (data) argument to listeners — since it wasn't passed, every listener attached the normal way (player.on(type, (event, data) => ...)) receiveddata === undefined.percentsplayed,timeplayed,playerload,start,seek,pausenoseek,mute,unmute,qualitychanged— including the built-in analytics plugin, which destructuresdataand throws.Fix: pass the payload as the actual trigger hash —
this.videojs.trigger(_event, data).Found while manually verifying an unrelated analytics fix on
docs/analytics.html(percentsplayedenabled by default): page threwTypeError: Cannot destructure property 'percent' of 'data' as it is undefinedon load.Test plan
master: loadeddocs/analytics.html(default config,percentsplayedevents enabled), confirmed uncaughtTypeError: Cannot destructure property 'percent' of 'data' as it is undefined.percentsplayed/timeplayedfire correctly while playing/seeking.npm run test:unitfor regressions in extended-events / analytics coverage.🤖 Generated with Claude Code