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

[RFC] TypeScript 2.8 and optional fields #140

Closed
gcanti opened this issue Feb 26, 2018 · 22 comments
Closed

[RFC] TypeScript 2.8 and optional fields #140

gcanti opened this issue Feb 26, 2018 · 22 comments

Comments

@gcanti
Copy link
Owner

gcanti commented Feb 26, 2018

from #138 (comment)

as 2.8 is around the corner, I would love to have a nicer solution than intersecting with partials to have optional members

There's a branch (optional) with a proposal that makes use of conditional types. The gist is

  • added an OptionalType and optional combinator
  • added RequiredKeys and OptionalKeys types (<= using conditional types)
  • changed TypeOfProps and OutputOfProps accordingly

Result

const Person = t.type({
  name: t.string,
  age: t.optional(t.number) // new optional combinator
})

type T = t.TypeOf<typeof Person>
/*
type T = {
    name: string;
} & {
    age?: number | undefined;
}
*/

Bonus point: is backward compatible (AFAIK)

/cc @sledorze

EDIT

Note that this is different from make a union with t.undefined

const Person = t.type({
  name: t.string,
  age: t.optional(t.number),
  foo: t.union([t.number, t.undefined])
})

type T = t.TypeOf<typeof Person>
/*
type T = {
    name: string;
    foo: number | undefined; // foo can be undefined but the key is required
} & {
    age?: number | undefined;
}
*/

const x: T = { name: 'Giulio' } // Property 'foo' is missing in type '{ name: string; }'.
@gcanti
Copy link
Owner Author

gcanti commented Feb 26, 2018

Also /cc @giogonzo, @mattiamanzati (mobx-state-tree), @pelotom (runtypes)

@giogonzo
Copy link
Contributor

nice! This code can be improved using the new combinator: https://github.com/gcanti/io-ts-codegen/blob/master/src/index.ts#L491-L505

@sledorze
Copy link
Collaborator

@gcanti is there a comparison speed wise between the new and historic way to encode an object via intersection and partial?

@gcanti
Copy link
Owner Author

gcanti commented Feb 27, 2018

@sledorze just added a perf suite, looks good

// optional
const T1 = t.type({
  a: t.string,
  b: t.optional(t.number)
})

// intersection
const T2 = t.intersection([t.type({ a: t.string }), t.partial({ b: t.number })])

const valid = { a: 'a', b: 1 }
const invalid = { a: 'a', b: 'b' }

Results

optional (valid) x 6,283,783 ops/sec ±0.77% (86 runs sampled)
intersection (valid) x 4,483,965 ops/sec ±0.75% (88 runs sampled)
optional (invalid) x 4,671,694 ops/sec ±0.91% (88 runs sampled)
intersection (invalid) x 1,613,067 ops/sec ±1.13% (83 runs sampled)

@sledorze
Copy link
Collaborator

sledorze commented Feb 27, 2018

@gcanti you made it so!

@sledorze
Copy link
Collaborator

sledorze commented Feb 27, 2018

@gcanti wouldn't it be interesting to reword it to t.maybe (and infer to undefined | T) and have another t.optional that infer to Option<T> (knowing it may be part of io-ts-types) ?

@gcanti
Copy link
Owner Author

gcanti commented Feb 28, 2018

@sledorze I prefer "optional" since

@gcanti
Copy link
Owner Author

gcanti commented Apr 30, 2018

Published as io-ts@next (v1.1.0, requires TypeScript 2.8.x)

Feedback much appreciated

@sledorze
Copy link
Collaborator

sledorze commented Apr 30, 2018

@gcanti can't promise any feedback in the very short term (maybe next week).
However very happy to see that addition!

@sledorze
Copy link
Collaborator

sledorze commented Apr 30, 2018

@gcanti actually gave a quick try: the problem I encountered can be seen here:

export const foo = <Type extends t.Type<A, O>, A = any, O = any>(
  _type: Type
) : t.RequiredKeys<{
  t: t.LiteralType<"v">;
  v: Type;
}> => 1 as any

foo type:

const foo: <Type extends t.Type<A, O, t.mixed>, A = any, O = any>(_type: Type) => "t" | (Type extends t.OptionalType<any, any, any, t.mixed> ? never : "v")

v being abstract, the inference cannot decide if it is optional or not at this point (hence the inferred type: (Type extends t.OptionalType<any, any, any, t.mixed> ? never : "v") ).

The strange consequence of that is that the compiler decides to not make it available but VSCode display does display it.


This implementation approach really looks like a show stopper for the use cases involving abstract runtime types.

@gcanti
Copy link
Owner Author

gcanti commented May 1, 2018

The strange consequence of that is that the compiler decides to not make it available but VSCode display does display it

@sledorze I'm sorry but I don't understand what you mean, could you please elaborate?

This implementation approach really looks like a show stopper for the use cases involving abstract runtime types

Why?

@sledorze
Copy link
Collaborator

sledorze commented May 1, 2018

Here's the full example:

export const ReadStatus = <Type extends t.Type<A, O>, A = any, O = any>(
  type: Type // abstract type param here
): t.Type<ReadStatus<t.TypeOf<Type>>, JSONReadStatus<t.OutputOf<Type>>> => {
  const JSONReadStatus = t.taggedUnion('t', [ // we instantiate a generic runtime using the abstract Type
    t.interface({
      t: t.literal('w')
    }),
    t.interface({
      t: t.literal('n')
    }),
    t.interface({
      t: t.literal('v'),
      v: type // v is defined here but rely on 'abstract type' (unkinded)
    })
  ])
  return new t.Type(
    `ReadStatus<${type.name}>`,
    (v): v is ReadStatus<A> =>
      v instanceof ReadNothing || v instanceof ReadValue || v instanceof ReadWaiting,
    (s, c) =>
      JSONReadStatus.validate(s, c).map(o => { // we try to a value validate against the instantiated schema
        switch (o.t) {
          case 'w':
            return ReadWaiting.value
          case 'n':
            return ReadNothing.value
          case 'v':
            return new ReadValue(o.v) // Errors here saying v does not exist at compile time
        }
      }),
    a => JSONReadStatus.encode(a)
  )
}

As for the field display that works and the compiler reporting an error, that was a wrong statement; that's because the display of a TypeOf is not simplified and the field v is not discarded at this point.

The problem is due to the way field are filtered to discard OptionalType, hence the inferred type of 'v' key:

Type extends t.OptionalType<any, any, any, t.mixed> ? never : "v"

@gcanti
Copy link
Owner Author

gcanti commented May 1, 2018

@sledorze Not sure I have an exact repro without some types / definitions, could you please add the missing bits?

  • ReadStatus
  • JSONReadStatus
  • ReadNothing
  • ReadValue
  • ReadWaiting

@sledorze
Copy link
Collaborator

sledorze commented May 1, 2018

@gcanti Here's a shrinked down, self contained version:

import * as t from 'io-ts'

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

@gcanti
Copy link
Owner Author

gcanti commented May 1, 2018

@sledorze Thanks. Looks like changing the signature fixes the error

export const shrinked1 = <Type extends t.Type<A, O>, A, O>(
  type: Type
): t.Validation<{ t: 'v'; v: t.TypeOf<typeof type> }> => {
  const T = t.interface({
    t: t.literal('v'),
    v: type
  })
  return T.decode({}) // error
}

export const shrinked2 = <A, O>(type: t.Type<A, O>): t.Validation<{ t: 'v'; v: A }> => {
  const T = t.interface({
    t: t.literal('v'),
    v: type
  })
  return T.decode({}) // ok
}

Unfortunately, instead of being an actual fix, it highlights another problem

const T = shrinked2(t.optional(t.string))
/*
T should be a Either<Errors, { t: 'v', v?: string }>
but is a Either<Errors, { t: 'v', v: string | undefined }> instead
*/

So it seems that defining an optional key through a combinator applied to its value, despite the syntax being nice, is not feasible

@sledorze
Copy link
Collaborator

sledorze commented May 1, 2018

@gcanti maybe defining the optional combinator to not directly return a subtype of type but a separated class only for the purpose of optionality would work (no more type ambiguity with abstract type).

I mean it should be done via an encoding on AnyProps, AnyProps having a signature like.

type AnyProps = {
   [k : string] : Any | Optional<Any>
}

Every implementation (Strict, Interface, Partial(?)) should adapt.

@MastroLindus
Copy link

This is currently my main issue with this otherwise fantastic project, I noticed that the related io-ts-codegen project has some work in the area recently, does that have any positive impact on the resolution for this issue for io-ts?

Or is there any kind of typescript issue waiting-to-be-fixed that will allow this? Something to look forward to?
Meanwhile thanks for the good work so far!

@gcanti
Copy link
Owner Author

gcanti commented Sep 8, 2018

@MastroLindus no it doesn't, io-ts-codegen still uses intersection and partial in order to implement optional properties.

@anilanar
Copy link

I assume this RFC is dead now?

@gcanti
Copy link
Owner Author

gcanti commented Apr 11, 2019

Yup

@gcanti gcanti closed this as completed Apr 11, 2019
@lostintime
Copy link
Collaborator

@gcanti @MastroLindus I'm not sure this was solved in the comments above, but here is a trick I found which allows to workaround this. I was also struggling for a while with optionals encoding as type() & partial(), as most of the time for me there is no difference with T | undefined union.

First you can define a type lambda which will mark all fields of given type as optional:

/**
 * Type lambda returning a union of key names from input type P having type A
 */
type FieldsWith<A, P> = { [K in keyof P]-?: (A extends P[K] ? K : never) }[keyof P]

/**
 * Dual for FieldsWith - returns the rest of the fields
 */
type FieldsWithout<A, P> = Exclude<keyof P, FieldsWith<A, P>>

/**
 * Typa lambda returning new type with all fields within P having type U marked as optional
 */
type MakeOptional<P, U = undefined> = Pick<P, FieldsWithout<U, P>> & Partial<Pick<P, FieldsWith<U, P>>>

Then having these combinators:

/**
 * Fix signature by marking all fields with undefined as optional
 */
const fixOptionals = <C extends t.Mixed>(c: C): t.Type<MakeOptional<t.TypeOf<C>>, t.OutputOf<C>, t.InputOf<C>> => c

/**
 * Just an alias for T | undefined coded
 */
const optional = <C extends t.Mixed>(c: C): t.Type<t.TypeOf<C> | undefined, t.OutputOf<C>, t.InputOf<C>> =>
  t.union([t.undefined, c])

You can use it like:

const Profile = fixOptionals(t.type({
  name: t.string,
  age: optional(t.number)
}))

type Profile = t.TypeOf<typeof Profile>

/**
 * the type can now be initialized ignoring undefined fields
 */
const someProfile: Profile = { name: "John" }

console.log(Profile.decode(someProfile))

// prints:
// 
// right({
//   "name": "John"
// })

This seems to work with io-ts v1.8.6 and typescript 3.5.1.

One drawback is how type signature looks:

Screen Shot 2019-06-02 at 11 14 42

Maybe you can come with something better out of this.

@zhaoyao91
Copy link

is there any hope for this RFC nowadays?

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

No branches or pull requests

7 participants