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

Reexport Fraction types from fraction.js instead of using custom one #3330

Merged
merged 2 commits into from
Dec 11, 2024

Conversation

fchu
Copy link
Contributor

@fchu fchu commented Dec 3, 2024

This should fix #3317
FYI npm test passed but got issues with npm run test:types (prior to this PR that is), so an approver might want to double check

@josdejong
Copy link
Owner

Thanks Francois!

I was just thinking: maybe we should just remove exporting the type definition of Fraction, and let TypeScript pick the type defintions from the Fraction.js library itself. If we export Fraction from mathjs, as an end user, you can import Fraction from two different sources: mathjs or Fraction.js. That is a duplicate. What do you think?

@fchu
Copy link
Contributor Author

fchu commented Dec 6, 2024

I'm not an expert so I won't be able to give you the most informed answer.
I would think that if you remove the definition, the user will still have to import it from somewhere, which will either be their own direct import of fraction.js (which can be a different version), or have to find the exact version your package is depending on, which is an hassle. Maybe re-exporting it is just the user-friendly way to have access to the correct version of the type?

@josdejong
Copy link
Owner

Hm, yeah. If we change that we should be consistent and not export the type definition of Fraction nor BigNumber. Ok let's leave the export in then 👍.

@josdejong josdejong merged commit 97da410 into josdejong:develop Dec 11, 2024
8 checks passed
@josdejong
Copy link
Owner

Published now in v14.0.1, thanks again.

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.

fraction methods are not typed
2 participants