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

Add Float.toFixed and friends #142

Closed
wants to merge 2 commits into from
Closed

Conversation

jchavarri
Copy link
Contributor

@jchavarri jchavarri commented Jun 27, 2024

Previously based on asset-prefix (#140).

Adds some functions to Js.Float.

@jchavarri jchavarri force-pushed the add-float-tofixed-n-friends branch from 10047e7 to a468b75 Compare June 27, 2024 07:54
Base automatically changed from asset-prefix to main June 27, 2024 08:06
@jchavarri jchavarri marked this pull request as ready for review June 27, 2024 08:07
@jchavarri jchavarri requested a review from davesnx June 27, 2024 08:07
@jchavarri
Copy link
Contributor Author

I'm not convinced this is the way to go. Because the behavior in native will differ from the JS one in some cases, it kind of defeats one of the main purpose of server-reason-react (render same HTML in server with OCaml and client with Melange).

I think it's better to keep current not_implemented alerts and let users use alternatives from the stdlib, which guarantees the behavior is the same.

@davesnx feel free to reopen if anything's recoverable :)

@jchavarri jchavarri closed this Jun 27, 2024
@jchavarri jchavarri deleted the add-float-tofixed-n-friends branch June 27, 2024 17:07
@davesnx
Copy link
Member

davesnx commented Jun 27, 2024

Currently the Js module is marked as very unstable, so keeping a half-backed implementation sounds good to merge to me.

@davesnx davesnx restored the add-float-tofixed-n-friends branch June 27, 2024 17:58
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.

2 participants