Skip to content

Bugfix: console errors on unmount - #88

Merged
KoolADE85 merged 7 commits into
mainfrom
bugfix/74-unmount-errors
Sep 14, 2026
Merged

KoolADE85 merged 7 commits into
mainfrom
bugfix/74-unmount-errors

Conversation

@KoolADE85

@KoolADE85 KoolADE85 commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

This PR fixes a flood of Javascript errors thrown when the player is unmounted while a video is playing.

While I'm in the neighbourhood:

  • move CircleCI job to a Github action
  • update dev dependencies to a modern stack
  • fix trivial console errors around invalid React syntax:
    • use className instead of invalid class
    • use React.createRef() for refs instead of hard-coded string player

Fixes #74
Fixes https://github.com/plotly/ddk-dash-docs/issues/4081

- Use modern dash[testing] version
- Change component to use `className` instead of `class`
- Change component to use real ref objects rather than invalid string refs
@AnnMarieW

AnnMarieW commented Aug 31, 2026

Copy link
Copy Markdown

Looks good!
Just curious - why is react-player not listed as a dependency in package.json

@AnnMarieW

Copy link
Copy Markdown

Also:
closes #78

@KoolADE85

Copy link
Copy Markdown
Contributor Author

Looks good! Just curious - why is react-player not listed as a dependency in package.json

but it is! 😃 :

@AnnMarieW

Copy link
Copy Markdown

Looks good! Just curious - why is react-player not listed as a dependency in package.json

but it is! 😃 :

Oh, haha - What's it doing way up there? I always look for it near the devDependancies. I totally missed it! 🙈

@camdecoster camdecoster left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good except for the few things we discussed.

Comment thread .github/workflows/ci.yml
Comment thread .github/workflows/ci.yml Outdated
Comment thread requirements-dev.txt
Comment thread src/lib/fragments/DashPlayer.react.js
Comment thread dash_player/DashPlayer.py

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As we discussed, this file got regenerated as part of the build process, but it doesn't need to be updated for the bug fix. You could revert this change or update the Dash version in dev requirements to 4.4.1 and update the setup.py Dash version floor. There's also an issue with newer Dash that resulted in the style prop getting dropped from the docstring.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All done in 7cb037b
I reverted the dev requirements back to v2.18 of Dash just to keep the scope from creeping up here.
We can do the extra work to bump everything else as part of a major version later.

@KoolADE85
KoolADE85 merged commit dfb9178 into main Sep 14, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Error at Chrome Console

3 participants