Skip to content

refactor: don't re-export playback-core items#204

Closed
gkatsev wants to merge 1 commit into
muxinc:mainfrom
gkatsev:limit-exports
Closed

refactor: don't re-export playback-core items#204
gkatsev wants to merge 1 commit into
muxinc:mainfrom
gkatsev:limit-exports

Conversation

@gkatsev
Copy link
Copy Markdown
Contributor

@gkatsev gkatsev commented Apr 29, 2022

This is a follow-up from #191. One of the issues is that older bundlers have some trouble with esm that does both default export and named exports. In addition, we no longer need to re-export some of these since we can import directly from playback-core without worrying about duplication of modules in bundles.

This doesn't necessarily have to make it in, particularly since it's a potentially breaking change, though, unlikely that anyone depends on these changes.

One thing that this PR doesn't address is the fact that native CJS users still need to use require('@mux-elements/mux-player).default. Unfortunately, while we can fix the actual export, fixing the types so that they work for both esm and cjs without any more work is complicated. We should consider whether we should be doing default exports and if we should only do named exports.

@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 29, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
elements-demo-create-react-app ✅ Ready (Inspect) Visit Preview Apr 29, 2022 at 6:43PM (UTC)
elements-demo-nextjs ✅ Ready (Inspect) Visit Preview Apr 29, 2022 at 6:43PM (UTC)
elements-demo-svelte-kit ✅ Ready (Inspect) Visit Preview Apr 29, 2022 at 6:43PM (UTC)
elements-demo-vanilla ✅ Ready (Inspect) Visit Preview Apr 29, 2022 at 6:43PM (UTC)
elements-demo-vue ✅ Ready (Inspect) Visit Preview Apr 29, 2022 at 6:43PM (UTC)

There shouldn't be a need to re-export items from playback-core through
mux-video. mux-player can import them directly. Don't re-export in
mux-audio either.

Additionally, don't export getVideoProperties from mux-player.
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 29, 2022

Codecov Report

Merging #204 (0d8730f) into main (2a6f8d1) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #204   +/-   ##
=======================================
  Coverage   82.00%   82.01%           
=======================================
  Files          37       37           
  Lines        3863     3858    -5     
  Branches      129      129           
=======================================
- Hits         3168     3164    -4     
+ Misses        688      687    -1     
  Partials        7        7           
Impacted Files Coverage Δ
packages/mux-audio/src/index.ts 60.88% <ø> (-0.25%) ⬇️
packages/mux-video/src/index.ts 64.03% <ø> (-0.18%) ⬇️
packages/mux-player/src/errors.ts 91.54% <100.00%> (ø)
packages/mux-player/src/index.ts 77.80% <100.00%> (-0.03%) ⬇️
.../mux-player/src/media-theme-mux/media-theme-mux.ts 98.88% <0.00%> (+0.37%) ⬆️

@vercel vercel Bot temporarily deployed to Preview – elements-demo-create-react-app April 29, 2022 18:43 Inactive
@gkatsev gkatsev closed this May 31, 2022
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.

1 participant