-
Notifications
You must be signed in to change notification settings - Fork 329
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
Filter extra fields in interface
decoder
#147
Comments
@lostintime thanks for opening this issue. The following cast is unsound const prof: UserProfileWithTime = profile though is allowed by TypeScript (which is unfortunate, AFAIK Flow complains here). I think that there are (at least) two solutions to this problem. (1) Never cast when optional fields are involved, refine instead For maximum type safety const UserProfileWithTime = t.intersection([UserProfile, t.partial({ time: t.number })])
...
// ... deeper in the call stack
if (UserProfileWithTime.is(profile)) {
if (profile.time !== undefined) {
// ...
}
} (2) You are willing to pay for the additional cost of stripping the extra fields Then you could define a export function strip<P extends t.Props, A, O>(type: t.InterfaceType<P, A, O>): t.InterfaceType<P, A, O> {
const keys = Object.keys(type.props)
const len = keys.length
return new t.InterfaceType(
type.name,
type.is,
(m, c) =>
type.validate(m, c).map((o: any) => {
const r: any = {}
for (let i = 0; i < len; i++) {
const k = keys[i]
r[k] = o[k]
}
return r
}),
type.encode,
type.props
)
}
// our type definition for service response
const UserProfile = strip(
t.interface({
username: t.string
})
) p.s.
|
The problem I see with solution this is that I should care about it "deeper in the call stack", and it also adds a runtime cost by calling I personally prefer to cover my back at inputs and safely play with plain types after. Also you may not be able to control that assignment of structurally compatible types - when happens in 3rd parties. Using just
This looks like a reasonable solution! Maybe it can be added to the library? Also it may be worth mentioning this case somewhere near Thanks! |
I mean, that kind of cast is always unsound, whether you are using When using (contrived example) type UserProfile = {
username: string
}
type UserProfileWithTime = {
username: string
time?: number // timestamp (seconds)
}
type UserProfileWithISOTime = {
username: string
time?: string // timestamp (ISO)
}
UserProfile.decode(JSON.parse(serviceResponse)).fold(
e => {
console.log('Something went wrong', e)
},
profile => {
console.log('User profile is valid:', profile)
// ... deeper in the call stack
const x: UserProfileWithISOTime = { ...profile, time: 'foo' }
// ... deeper in the call stack
const y: UserProfile = x
// ... deeper in the call stack
const prof: UserProfileWithTime = y // opsss...
if (prof.time !== undefined) {
console.log('1 hour later:', prof.time + 3600) // 1 hour later: foo3600
}
}
) Obviously you can judge better than me if stripping at the boundaries is safe enough in your use case, I just wanted to point out that there's always a risk ;)
Yes, maybe to io-ts-types though
Sure, would you like to send a PR? Btw for a discussion about a type safe pattern matching, check out runtypes/runtypes#36 (comment) |
You're absolutely right! it doesn't matter where that hidden fields comes from, io-ts or other cast - it can break things anyway, and mutability makes it even worse #13347, must be always careful. Maybe this will help #12936 - i it still applies to optional fields. This is probably what you pointed to flow about: https://flow.org/en/docs/types/objects/#toc-exact-object-types - I never used flow before, you have to use a special syntax, but there is a solution at least. It would be great if
Yes! I will Thanks again for opening my eyes ) |
@lostintime will #277 fix the original issue? |
@gcanti yep, looking at the tests from #277 seems like Thanks for the great job! 👍 |
Hello @gcanti, thanks for writing this great piece of software!
Here is an issue I discovered while solving similar problems:
Passing extra fields, which are not not defined by
t.interface
throughdecode
method may turn into unexpected behavior at runtime when it's output assigned to other types with optional fields.Here is a example:
Output:
Using strict interfaces is not a desired behavior, for given example, as our client will start to fail when external service deploys new
time
field in the response.Filtering extra fields will of course have a performance impact, but to me - current behavior is very dangerous to leave as default.
While
decode
name implies a change/transformation of the input - changing it this way may notcount as "breaking".
validate
name is trickier, but it leads to same issue.The text was updated successfully, but these errors were encountered: