fix(mux-video,mux-audio): reload core on DOM connect #765
Conversation
This file contains 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
…ently unload core when disconnected.
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #765 +/- ##
==========================================
+ Coverage 82.12% 82.18% +0.05%
==========================================
Files 40 40
Lines 7848 7862 +14
Branches 458 456 -2
==========================================
+ Hits 6445 6461 +16
+ Misses 1397 1395 -2
Partials 6 6
|
|
Unrelated to code, the title needs a |
cjpillsbury
changed the title
cjpillsbury
changed the title
AdamJaggard
approved these changes
Sep 5, 2023
Yeah that's derived from my branch name, not my intended squash commit msg. That said, since we've moved to squash merges instead of rebase merges, I've def unintentionally merged with the wrong commit msg before. |
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.
Currently, we do an "unload"/teardown of core (and everything that happens in it/is created by it "under the hood") whenever our custom media elements are disconnected from the DOM. Ideally, this would not be the way we handle cleanups, as we're currently treating disconnect as a "
destroy()" method. This causes issues with more advanced UIs/use cases, especially common in app frameworks where an element may be moved around the DOM for UI/UX purposes without the intention of destroying it. Unfortunately, it looks like we have some dangling references that result in a fairly large memory footprint per media element if we don't do this teardown. As a "middle ground" solution until we can identify a reasonable and reliable resolution of this larger memory management GC issue, we will simply (conditionally) "reload"/re-initialize core when the media element is (re)connected to the DOM.