Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Only mutate ManagedPlayerViewModel.loadingState on main thread #566

Merged
merged 2 commits into from
Jan 6, 2025

Conversation

ap-for-work
Copy link
Contributor

Published properties in SwiftUI models should not be mutated on background threads.

Refactor ManagedPlayerViewModel.next to ensure all paths that update loadingState do so from the main thread.

Add unit tests to verify loadingState changes arrive on the main thread when next is called.

Change Type (required)

Indicate the type of change your pull request is:

  • patch
  • minor
  • major
  • N/A

Does your PR have any documentation updates?

  • Updated docs
  • No Update needed
  • Unable to update docs

Published properties in SwiftUI models should not be mutated on
background threads.

Refactor `ManagedPlayerViewModel.next` to ensure all paths that update
`loadingState` do so from the main thread.

Add unit tests to verify loadingState changes arrive on the main thread
when next is called.
Copy link

codecov bot commented Dec 23, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.36%. Comparing base (16f2a50) to head (e7dfed2).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #566      +/-   ##
==========================================
+ Coverage   89.70%   94.36%   +4.66%     
==========================================
  Files         331      145     -186     
  Lines       19794     9160   -10634     
  Branches     1949        0    -1949     
==========================================
- Hits        17756     8644    -9112     
+ Misses       2024      516    -1508     
+ Partials       14        0      -14     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@nancywu1 nancywu1 left a comment

Choose a reason for hiding this comment

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

looks good to me, thanks for adding the tests!

@hborawski hborawski added the patch Increment the patch version when merged label Jan 3, 2025
@nancywu1 nancywu1 merged commit cb3ea0c into player-ui:main Jan 6, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
patch Increment the patch version when merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants