You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Without adding a dependancy on decimal.js (or similar), any implementation of multipleOf() is unlikely going to pass all use-cases. Dealing with decimals is hard, and deferring that responsibility to a purpose-built library is the best solution.
I think that leaves the following options:
Add a dependancy on decimal.js and update the multipleOf validator for Number types.
Quick-win, and increases the reliability of typebox... but I can understand any reluctance you may have on this given that it would only be used to cater for a particular set of edge-cases.
Add an optional dependancy on decimal.js and use it if available, otherwise fallback to current approach.
Sounds a little messy, and conditional imports often cause build issues for consumers.
Create a separate package that exposes a custom Decimal type for those that want to opt-in to using it.
This could lead to complicated installs, and an uncontrolled marketplace of custom type packages could lead to the typebox equivalent of WordPress plugins. Perhaps TypeBox itself could be split into multiple packages, e.g.
@typebox/core
Base types, compiler, etc.
@typebox/json
JSON Schema-specific types; either a package for each spec version (e.g @typebox/json-draft2019-09), or named exports from a single package (e.g. import { JsonType } from '@typebox/json/draft2019-09')
@typebox/js
TConstructor, TFunction, etc.
@typebox/custom
Everything that falls out of scope of the other two, as well as anything that requires extra dependancies (TDecimal lives here). Hyperthetical problem: which JSON Schema version should TDecimal align to? Does it support a divisibleBy or multipleOf option? Maybe a future spec version will rename it again, perhaps to modulus. Possibly needs some additional thought.
Implement TDecimal as a local custom type.
This is what we're going with today; we already depend on decimal.js for other things, so this works nicely for us.
reacted with thumbs up emoji reacted with thumbs down emoji reacted with laugh emoji reacted with hooray emoji reacted with confused emoji reacted with heart emoji reacted with rocket emoji reacted with eyes emoji
-
Background: #754
Without adding a dependancy on decimal.js (or similar), any implementation of
multipleOf()
is unlikely going to pass all use-cases. Dealing with decimals is hard, and deferring that responsibility to a purpose-built library is the best solution.I think that leaves the following options:
Add a dependancy on decimal.js and update the
multipleOf
validator for Number types.Quick-win, and increases the reliability of typebox... but I can understand any reluctance you may have on this given that it would only be used to cater for a particular set of edge-cases.
Add an optional dependancy on decimal.js and use it if available, otherwise fallback to current approach.
Sounds a little messy, and conditional imports often cause build issues for consumers.
Create a separate package that exposes a custom
Decimal
type for those that want to opt-in to using it.This could lead to complicated installs, and an uncontrolled marketplace of custom type packages could lead to the typebox equivalent of WordPress plugins. Perhaps TypeBox itself could be split into multiple packages, e.g.
@typebox/core
Base types, compiler, etc.
@typebox/json
JSON Schema-specific types; either a package for each spec version (e.g
@typebox/json-draft2019-09
), or named exports from a single package (e.g.import { JsonType } from '@typebox/json/draft2019-09'
)@typebox/js
TConstructor
,TFunction
, etc.@typebox/custom
Everything that falls out of scope of the other two, as well as anything that requires extra dependancies (
TDecimal
lives here). Hyperthetical problem: which JSON Schema version shouldTDecimal
align to? Does it support adivisibleBy
ormultipleOf
option? Maybe a future spec version will rename it again, perhaps tomodulus
. Possibly needs some additional thought.Implement
TDecimal
as a local custom type.This is what we're going with today; we already depend on decimal.js for other things, so this works nicely for us.
Beta Was this translation helpful? Give feedback.
All reactions