[POC] partial partial

[mmkal]

@gcanti @sledorze

Addresses #140

I'm not totally sure I understand what the issue mentioned in #140 (comment) is, I tried pasting that code into dtslint/index.ts and it didn't seem to cause problems I could see.

I also don't know if this implementation is the same as the POC from the optional branch mentioned in the OP because I can't find the branch (I guess it's been deleted).

I implemented it as a separate helper, partialPartial rather than updating the type export, just for the sake of a proof-of-concept. @gcanti @sledorze if this has the same issue as the previous POC, could you give a code sample that will fail dtslint?

[gcanti]

I also don't know if this implementation is the same as the POC from the optional branch

Yes, the gist was trying to use conditional types. Indeed I see the same problem

export const shrinked = <Type extends t.Type<A, O>, A = any, O = any>(
  type: Type
): t.Validation<{ t: 'v'; v: t.TypeOf<Type> }> => {
  const T = t.partialPartial({
    t: t.literal('v'),
    v: type
  })
  const res = T.validate(1 as any, [])
  res.map(x => {
    x.t // Ok
    x.v // Error: Property 'v' does not exist on type 'OptionalPropsTypes<{ t: LiteralC<"v">; v: Type; }> & RequiredPropsTypes<{ t: LiteralC<"v">; v: Type; }>'
  })
  return res // Errors as v is missing
}

[mmkal]

@gcanti ah yes, I copy and pasted the example without changing t.interface to t.partialPartial 🤦‍♂️

Updated to fix for that problem, and add a test for it to dtslint to ensure no regressions. The only change needed to shrinked (aside from a more explicit name) was to make v in the return type optional, since it might be. However I also added an identical example with no return type (i.e. inferred), because the compiler is then smart enough to do the right thing - make v optional when an optional type is passed in, required otherwise. There's an $ExpectError covering this second scenario too.

[mmkal]

(the solution to the problem in the snippet above was to use keyof P rather than any fancy conditionals for the optional keys. Fancy conditionals are still used for required properties. Then it relies on the compiler being able to figure out that

{ foo?: string; bar?: string } & { foo: string }

is the same as

{ foo: string; bar?: string }

)

[mmkal]

@gcanti any thoughts on this? The issue mentioned in #140 (comment) is now addressed, and tested in https://github.com/gcanti/io-ts/pull/266/files#diff-a40ed4e571c91fc09060cdd4d991ff2aR609

[gcanti]

@mmkal that's an interesting approach but I wouldn't touch the core type combinator, using an intersection is slightly more verbose but also seems more future proof

[mmkal]

I agree, that's why I put it in a separate helper to make sure it doesn't interfere with existing code. It uses an intersection type as an implementation detail. Currently called partialPartial as a placeholder but there's probably a better name for it?

[gcanti]

@mmkal I mean, if it doesn't affect the core type combinator then such a combinator should be external to io-ts

[mmkal]

@gcanti it solves an issue that comes up a lot: #196 #140 #83 #56

You yourself said "I wish" in that latest one. And it seems at least with the latest and greatest typescript versions it's now possible. In the RFC issue you created @sledorze pointed out that it wouldn't work using abstract types passed into a function. In this PR I believe that scenario is handled successfully. Unless I've missed something (@sledorze - what do you think?) - now we know that it can be done, why not include it in the library since it's such a highly-requested feature, and significantly improves the syntax?

This PR is only a POC to figure out if the compiler can be persuaded to make the static type match the runtime type. The API design could be a follow-up step. This PR is really just the (potential) implementation detail.

Questions over whether it should it go into the type combinator, or be its own thing are a little above my pay-grade as I'm not a maintainer of this project.

I'd be happy to give my opinion (my slight preference would be that it does stay in the type combinator, so there isn't a separate function that needs to be used when types have optional properties).

[VanTanev]

@gcanti @mmkal Maybe this can become a part of https://github.com/gcanti/io-ts-types?

[mmkal]

It could. But having thought about it more since the original conversation and had a look at some of the other changes that have gone in, I definitely feel it'd be better as a first-class citizen. i.e. instead of a new combinator partialPartial the implementation of type should be updated. This could ensure that we'd still get expected behaviour with some of the other combinators like union, intersection, strict, etc.

@gcanti would you be open to merging an update to this PR that did that? I'm happy to do the work but wouldn't want it to sit in review for a long time. The change would affect existing code, meaning merge conflicts would be very likely if it's waiting for too long.

[gcanti]

would you be open to merging an update to this PR that did that?

@mmkal thanks for this POC but no, I confirm my previous comments

[mmkal]

I assume you're talking about the future-proof comment. That's the only one that applies to this suggestion. If so, are you saying you think the types could be broken by future typescript versions - I'd be interested to hear how/why.

Do you at least agree it'd be better to enable optional properties in some form, as you've suggested in the past? And in that case, is the issue with this PR about the implementation/maintainability, or did you just change your mind (or is there something else)?

[gcanti]

are you saying you think the types could be broken by future typescript versions

Yes it's possible (and it wouldn't be the first time IME) but I'm more concerned by the future directions of io-ts which I can't completely forsee now.

Do you at least agree it'd be better to enable optional properties in some form, as you've suggested in the past?

Optional properties are possible from the beginning via intersection and partial. The current way of defining optional properties proved to be reliable during the last two years, so at least we have a solid solution.

My "I wish" is more like "I wish it would exist an official type-level operator that allows us to selectively make some key optional". The only official operator (the ? modifier of mapped types) is applied on all keys and that's what partial is using under the hood.

Now back to the main topic, since there's already a way to encode optional properties, I think we are talking about API ergonomics here.

This is what you can do today

import * as t from 'io-ts'

interface Person {
  name: string
  age?: number
}

const Person = t.intersection([
  t.type({
    name: t.string
  }),
  t.partial({
    age: t.number
  })
])

Another option which avoids some boilerplate is to define an helper combinator

function type2<R extends t.Props, O extends t.Props>(
  required: R,
  optional: O,
  name?: string
): t.IntersectionC<[t.TypeC<R>, t.PartialC<O>]> {
  return t.intersection([t.type(required), t.partial(optional)], name)
}

// usage
const Person2 = type2(
  {
    name: t.string
  },
  { age: t.number }
)

Now since we are not talking about core features but.. taste? I can't judge for other people.
Personally I'm fine with both versions. The following syntax is certainly nice (IMO)

const Person3 = type3({ 
  name: t.string,
  age: t.optional(t.number)
})

// versus

const Person2 = type2(
  {
    name: t.string
  },
  { age: t.number }
)

However I'm worried to put a possible constraint on the future development or undermine the stability of io-ts for a small syntax change.

Maybe I'm too cautious but better safe than sorry, I had bad track record with conditional type in the past. /cc @sledorze

[mmkal]

Thanks for the response. You are right that it would increase the possibility of a bug creeping into the implementation somewhere. So if this were to make it into the library it would have to be done thoughtfully and carefully.

I do believe it goes a little further than taste though - I think it'd be hard to find anyone who prefers the current way of doing things from an API point of view (of course implementation is a separate matter), so taste only really comes into it in terms of how much individual people care. But there are consequences of having the "correct" way of doing things be non-intuitive/unnatural. Which is that the library tends to get used wrongly. The reason I'd love for this to make it in is that I've used io-ts on many projects, and in lots of places, people have implemented their own optional combinator which just does a union with t.null and t.undefined. This is 1) duplicated code and 2) can be quite dangerous, and has actually caused an outage for us in production. Here's an example of how:

const optional = <T extends t.Type<any, any, any>>(type: T) => t.union([type, t.null, t.undefined])
const Person = t.interface({
  name: t.string,
  age: optional(t.number)
})

Person.decode({name: 'bob'}) // returns right({name: 'bob'})
Person.is({name: 'bob'}) // returns false

That very subtle semantic difference between decode and is may be correct but it's surprising, and a user basically has to get bitten by it to discover it. Our solution was to warn against this in documentation and try to catch it in code review. Even so, new team members would constantly accidentally try to do the same thing.

With the change to allow optional properties, the most natural and intuitive way to define partially-optional types (which in my experience, the majority of type definitions are), becomes the correct and safe way to do it too.

This is all to say, I think it's really worth the additional maintenance overhead! Please let me know if there's anything you can think of that I could help with which would make this change feel less risky. One thing that comes to mind (aside from more tests) is to mark the optional combinator as experimental until it's battle tested, maybe in v2 of the library, so anyone using it can assume the risk.

[gcanti]

That very subtle semantic difference between decode and is may be correct but it's surprising, and a user basically has to get bitten by it to discover it

I'd say that the difference is quite big, starting from the signatures:

while decode can change the result type

decode: (input: I) => Either<Errors, A>

is is just a custom type guard

is: (u: unknown) => u is A

Indeed you can't derive is from decode since, in general, A can be different from I/O

This example shows the difference

NumberFromString.decode('1') // returns right(1)
NumberFromString.is('1') // returns false
NumberFromString.is(1) // returns true

which leads to

Our solution was to warn against this in documentation and try to catch it in code review. Even so, new team members would constantly accidentally try to do the same thing.

this is a good point (and likely a documentation failure from my part), would you be willing to send a PR to improve io-ts's docs, based on your experience?

[Lonli-Lokli]

@gcanti maybe there is a chance/option that typescript itself can be updated to allow more simpler declaration in this library?