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

Missing types in currency.Any #446

Open
jonaspohren opened this issue May 30, 2023 · 3 comments
Open

Missing types in currency.Any #446

jonaspohren opened this issue May 30, 2023 · 3 comments

Comments

@jonaspohren
Copy link

The currency.d.ts should be updated to also include undefined and null on currency.Any, because when passing those values it defaults to 0.

This code works fine, but TypeScript complains:
currency(null).add(undefined).value; // 0

@scurker
Copy link
Owner

scurker commented May 31, 2023

What's the value in having a less restrictive type? It's intended for currency.js to try and parse the value and it seems like passing in null or undefined should raise flags as that might be accidental.

@jonaspohren
Copy link
Author

In my project I have a lot of possible undefined fields so i need to always check the value (prop ?? 0) , but that is only a problem when using with TypeScript, if I was only using JS I could just ignore this check.

If you thing passing null or undefined is not ideal, then maybe adding a warning log would alleviate the problem, something like 'you passed an undefined value but we used 0 instead'

@scurker
Copy link
Owner

scurker commented Jun 1, 2023

There is an errorOnInvalid option if you want to catch a runtime error, but of course that doesn't help you in typescript land:

https://currency.js.org/#erroroninvalid-default-false

currency(undefined, { errorOnInvalid: true }); // throws an error

I think I have a little better understanding of where you're coming from, it technically is allowed from javascript - so that does seem like something we might want to consider making more loose in typescript. Let me think about what the implications of that change might be.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants