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

Core package organization, and renaming to ReactPlayer #93

Merged
merged 6 commits into from
Oct 25, 2022

Conversation

adierkens
Copy link
Member

@adierkens adierkens commented Oct 21, 2022

The majority of this PR merges most of the sub-packages inside the core/ folder under the core/player package. Most of the API of the core/player remains unchanged (as many of these modules were exported under @player-ui/player anyway).

The big motivation for reducing the number of transitive dependencies is to reduce the churn for updating versions. Many times the transitive dependencies don't get properly resolved/shared without manual editing of the lockfiles generated by yarn/npm. This should help to reduce that variability down to just @player-ui/player and @player-ui/react.

This PR also does the same thing for @player-ui/react, which now includes the export for the ReactAsset component to render in the player tree. In order to avoid conflict w/ the Asset interface from @player-ui/types, the Asset component was renamed to ReactAsset, along with the entire WebPlayer to ReactPlayer to better describe it's platform dependence. @player-ui/react no longer requires the use of react-dom


Fixes #87

Version

Published prerelease version: 0.3.0-next.4

Changelog

🚀 Enhancement

🐛 Bug Fix

📝 Documentation

Authors: 7

@codecov
Copy link

codecov bot commented Oct 21, 2022

Codecov Report

Base: 77.01% // Head: 78.57% // Increases project coverage by +1.55% 🎉

Coverage data is based on head (556f812) compared to base (88bfa54).
Patch coverage: 68.42% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #93      +/-   ##
==========================================
+ Coverage   77.01%   78.57%   +1.55%     
==========================================
  Files         129      130       +1     
  Lines        4499     4504       +5     
  Branches     1114     1114              
==========================================
+ Hits         3465     3539      +74     
+ Misses        754      688      -66     
+ Partials      280      277       -3     
Impacted Files Coverage Δ
core/player/src/binding-grammar/ast.ts 100.00% <ø> (ø)
core/player/src/binding-grammar/custom/index.ts 94.77% <ø> (ø)
core/player/src/binding-grammar/ebnf/index.ts 80.00% <ø> (ø)
core/player/src/binding-grammar/parsimmon/index.ts 100.00% <ø> (ø)
core/player/src/binding/binding.ts 100.00% <ø> (ø)
core/player/src/binding/index.ts 93.22% <ø> (ø)
core/player/src/binding/resolver.ts 80.95% <ø> (ø)
core/player/src/binding/utils.ts 100.00% <ø> (ø)
core/player/src/controllers/constants/index.ts 100.00% <ø> (ø)
core/player/src/controllers/constants/utils.ts 100.00% <ø> (ø)
... and 90 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@adierkens adierkens added the minor Increment the minor version when merged label Oct 25, 2022
@adierkens adierkens merged commit 774a3cf into main Oct 25, 2022
@adierkens adierkens deleted the js-package-organization branch October 25, 2022 16:48
@sugarmanz
Copy link
Member

I know I'm late to this PR, but it probably still makes sense to keep separate BUILD targets for the sub-modules to help maintain incremental build performance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
minor Increment the minor version when merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Simplify package layout for JS libraries
2 participants