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: specify an explicit return type for uint8Array() #128

Merged
merged 2 commits into from
Feb 10, 2025

Conversation

eltigerchino
Copy link
Contributor

@eltigerchino eltigerchino commented Feb 10, 2025

fixes #127

An alternative to #126

By explicitly specifying the return type as Uint8Array, TypeScript won't infer the return type for us and compile it as the generic form Uint8Array<ArrayBufferLike> which is only available on TS >5.7. This allows us to maintain compatibility with libraries using TypeScript <5.7 while still returning the same type.

@benmccann
Copy link

It looks like this needs the corepack changes from #126 to pass the CI

@pi0
Copy link
Member

pi0 commented Feb 10, 2025

there are some other methods in same class using inferred types.

I will be back at this later today + updating CI issue.

@LukeHagar
Copy link
Collaborator

Inferred types are not necessarily an issue, I believe just that one generic is. I can fix the CI

@pi0
Copy link
Member

pi0 commented Feb 10, 2025

yes but inferred types can cause issues like this.

Copy link

codecov bot commented Feb 10, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Please upload report for BASE (main@9ef29ad). Learn more about missing BASE report.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #128   +/-   ##
=======================================
  Coverage        ?   76.55%           
=======================================
  Files           ?        9           
  Lines           ?      708           
  Branches        ?      146           
=======================================
  Hits            ?      542           
  Misses          ?      164           
  Partials        ?        2           

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

@pi0 pi0 merged commit 92e2653 into unjs:main Feb 10, 2025
4 checks passed
@pi0
Copy link
Member

pi0 commented Feb 10, 2025

Thanks! (might release with little delay to include some additional fixes + isolated types migration if possible)

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.

Compatibility with TypeScript <5.7
4 participants