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

Support for rust_decimal::Decimal #351

Closed

Conversation

corvusrabus
Copy link
Contributor

Goal

Adds support for the currently most popular decimals crate rust_decimal (23m downloads on crates.io).

I think the crates rust_decimal and bigdecimal don't need serde themselves enabled as the users of this library will enable it by themselves if they need it.

Checklist

  • I have followed the steps listed in the Contributing guide.
  • If necessary, I have added documentation related to the changes made.
  • I have added or updated the tests related to the changes made.

@corvusrabus
Copy link
Contributor Author

@gustavo-shigueo sorry you were so fast with merging my last one that I had to open a new one for this :)

@corvusrabus corvusrabus force-pushed the support-rust-decimal branch 2 times, most recently from df87a0a to 82acfec Compare September 7, 2024 14:08
@corvusrabus
Copy link
Contributor Author

@gustavo-shigueo any chance this could be release 9.1 ?

@corvusrabus corvusrabus mentioned this pull request Sep 7, 2024
3 tasks
rename rustdecimal_impl and smolstr_impl
@gustavo-shigueo
Copy link
Collaborator

Don't worry about smol_str, I've updated the feature name on main

@NyxCode
Copy link
Collaborator

NyxCode commented Sep 7, 2024

How does this interact with the serde features of rust_decimal?

There's

We had some discussion about this a while back in #273. As far as I can tell, we cannot support this library here properly, and support within rust_decimal itself is needed.
Since there's no clear format to which a Decimal is serialized, I am not sure it's a good idea to generate string bindings for it. I think it might just be better to let users use #[ts(type = "..")] or #[ts(as = "..")] here.

@corvusrabus
Copy link
Contributor Author

How does this interact with the serde features of rust_decimal?

You're completely right about this suspicion. I didn't remember that rust_decimal has the weird architecture of being allowed to change the serde serialization via feature flag.

I don't see how to get arund this right now. We might in a hacky way serialize a Decimal via serde_json::to_value and see which type we get but I am not sure how to do that at compile time to deduce the types needed to implement TS.

Closing this for now and might reopen if I come up with a new idea.

Any chance you could release version 9.1 that includes the smolstr feature @NyxCode ?

@corvusrabus corvusrabus closed this Sep 9, 2024
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.

3 participants