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

fix(types): Fix issues in exported types #8333

Merged
merged 1 commit into from
Sep 28, 2023
Merged

fix(types): Fix issues in exported types #8333

merged 1 commit into from
Sep 28, 2023

Conversation

boris-petrov
Copy link
Contributor

Description

Fixes #8301

Specific Changes proposed

Fix exported types.

Requirements Checklist

  • Feature implemented / Bug fixed
  • If necessary, more likely in a feature request than a bug fix
    • Change has been verified in an actual browser (Chrome, Firefox, IE)
    • Unit Tests updated or fixed
    • Docs/guides updated
    • Example created (starter template on JSBin)
  • Reviewed by Two Core Contributors

@codecov
Copy link

codecov bot commented Jun 28, 2023

Codecov Report

Merging #8333 (cecbb19) into main (2b0df25) will increase coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #8333      +/-   ##
==========================================
+ Coverage   82.68%   82.69%   +0.01%     
==========================================
  Files         113      113              
  Lines        7561     7578      +17     
  Branches     1818     1820       +2     
==========================================
+ Hits         6252     6267      +15     
- Misses       1309     1311       +2     
Impacted Files Coverage Δ
src/js/component.js 93.54% <ø> (ø)
src/js/event-target.js 100.00% <ø> (ø)
src/js/plugin.js 99.00% <ø> (ø)
src/js/tech/tech.js 83.71% <ø> (ø)
src/js/tracks/audio-track-list.js 96.77% <ø> (ø)
src/js/tracks/html-track-element.js 100.00% <ø> (ø)
src/js/tracks/text-track.js 94.87% <ø> (ø)
src/js/tracks/track-list.js 100.00% <ø> (ø)
src/js/video.js 94.16% <ø> (ø)

... and 3 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@mister-ben mister-ben changed the title Fix issues in exported types fix(types): Fix issues in exported types Jun 29, 2023
Copy link
Contributor

@mister-ben mister-ben 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, thanks

@mister-ben mister-ben added the needs: LGTM Needs one or more additional approvals label Jun 30, 2023
Copy link
Member

@misteroneill misteroneill left a comment

Choose a reason for hiding this comment

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

I don't think we can merge this because it adds a lot of unnecessary data to the dist file(s).

src/js/video.js Outdated Show resolved Hide resolved
src/js/tracks/audio-track-list.js Show resolved Hide resolved
@misteroneill misteroneill removed the needs: LGTM Needs one or more additional approvals label Jul 7, 2023
Copy link
Contributor

@mister-ben mister-ben left a comment

Choose a reason for hiding this comment

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

I think this is OK now, the unnecessary junk from package.json isn't being added. With this and #8218 we should be in a slightly better place wrt types.

@drallgood
Copy link

Is it possible to merge this soon?
Without it I can't use this library and I would really like to

@qstiegler
Copy link

Please merge this soon!

@andreifilip123
Copy link
Contributor

I'd also appreciate if this would be merged soon 🙏 Currently, we have to do a bunch of @ts-ignore for types we know are correct. Thank you 🙏

@mister-ben mister-ben added the needs: LGTM Needs one or more additional approvals label Sep 27, 2023
@dzianis-dashkevich
Copy link
Contributor

@boris-petrov
Thank you for your contribution!

@dzianis-dashkevich dzianis-dashkevich merged commit bad086d into videojs:main Sep 28, 2023
11 checks passed
@boris-petrov boris-petrov deleted the fix-exported-types branch September 28, 2023 15:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs: LGTM Needs one or more additional approvals
Projects
None yet
Development

Successfully merging this pull request may close these issues.

A number of TypeScript errors in the exported types
8 participants