Skip to content
This repository has been archived by the owner on Jul 16, 2024. It is now read-only.

Add BigDecimal #597

Merged
merged 21 commits into from
Nov 23, 2023
Merged

Add BigDecimal #597

merged 21 commits into from
Nov 23, 2023

Conversation

jessekelly881
Copy link
Contributor

@jessekelly881 jessekelly881 commented Nov 23, 2023

Closes #576

Copy link

changeset-bot bot commented Nov 23, 2023

🦋 Changeset detected

Latest commit: 6f64153

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@effect/schema Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@jessekelly881
Copy link
Contributor Author

jessekelly881 commented Nov 23, 2023

@fubhy What do you think the "default" decoder should be here? I.e. what should Schema.BigDecimal decode from? Currently I've defined it as Schema.BigDecimal :: Schema<string, BigDecimal> which seems to make the most sense. But I'm not 100% sure if this is the best option.

I've played around with Schema.BigDecimal :: Schema<[value: bigint, scale: number], BigDecimal> as well, similar to Duration, but I'm not a huge fan of that.

@fubhy
Copy link
Member

fubhy commented Nov 23, 2023

I think it should be Schema<string, BigDecimal> where both values are the normalized representation (optimized for scaled).

src/Schema.ts Outdated Show resolved Hide resolved
src/Schema.ts Outdated Show resolved Hide resolved
src/Schema.ts Show resolved Hide resolved
@jessekelly881
Copy link
Contributor Author

jessekelly881 commented Nov 23, 2023

Sounds good! I added normalization when decoding to/from a string as well.

@jessekelly881 jessekelly881 marked this pull request as ready for review November 23, 2023 14:45
test/BigDecimal/greaterThanBigDecimal.test.ts Outdated Show resolved Hide resolved
test/BigDecimal/BigDecimalFromSelf.test.ts Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
src/Schema.ts Outdated Show resolved Hide resolved
@gcanti gcanti merged commit caeed29 into Effect-TS:main Nov 23, 2023
1 check passed
@github-actions github-actions bot mentioned this pull request Nov 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for BigDecimal
3 participants