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

1.0.0 #112

Merged
merged 8 commits into from
Feb 12, 2018
Merged

1.0.0 #112

merged 8 commits into from
Feb 12, 2018

Conversation

gcanti
Copy link
Owner

@gcanti gcanti commented Jan 23, 2018

This is a sneak view of a possible improvement on the serialization side.

Currently when we serialize a value of type A via a runtime type Type<S, A> we get a value of type S which can be pretty loose.
For example if I serialize a string via t.string I get mixed though I know is actually a string. This PR adds a third type parameter to Type

export class Type<A, O = A, I = mixed> implements Decoder<I, A>, Encoder<A, O> {
}

where

  • A (old) is the type represented by the runtime type
  • I (old) is the input type
  • O (new) is the output type

This change will allow to preserve informations about serialized values

const NumberFromString = new t.Type<number, string>(
  'NumberFromString',
  t.number.is,
  (m, c) =>
    t.string.validate(m, c).chain(s => {
      const n = parseFloat(s)
      return isNaN(n) ? t.failure(s, c) : t.success(n)
    }),
  String
)

const Person = t.type({
  name: t.string,
  age: NumberFromString
})
/*
const Person: t.InterfaceType<{
    name: t.StringType;
    age: t.Type<number, string, t.mixed>;
}, {
    name: string;
    age: number;
}, {
    name: string;
    age: string;
}, t.mixed>
*/

const person = { name: 'Giulio', age: 44 }
const v = Person.serialize(person)
/*
v: {
    name: string;
    age: string;
}

currently is mixed which is a shame
*/

Also it will allow for fine grained (static) constraints

type JSONObject = { [key: string]: JSONType }
interface JSONArray extends Array<JSONType> {}
type JSONType = null | string | number | boolean | JSONArray | JSONObject

type SerializableTo<O> = t.Type<any, O, any>

interface JSONableArray extends t.ArrayType<JSONable> {}
type JSONable =
  | SerializableTo<string>
  | SerializableTo<number>
  | SerializableTo<boolean>
  | SerializableTo<null>
  | JSONableArray
  | t.InterfaceType<{ [key: string]: JSONable }>

function stringify<A>(a: A, type: t.Type<A, any, any> & JSONable): string {
  return JSON.stringify(type.serialize(a))
}

const s1 = stringify(person, Person) // OK

const Bad = t.type({
  f: t.Function
})

const s2 = stringify({ f: () => 1 }, Bad) // compiler error
/*
error: Type '{ f: FunctionType; }' is not assignable to type '{ [key: string]: JSONable; }'.
*/

/cc @sledorze @giogonzo

@gcanti
Copy link
Owner Author

gcanti commented Jan 24, 2018

For reference, an actual use case for this feature is fp-ts-routing where I'm using runtime types both for validations and serializations.
Last change (t.mixed) raised 3 compilation errors. The cause is some unsafe code I wrote

https://github.com/gcanti/fp-ts-routing/blob/1d77496efd00b2f6df3fd12b0662f89bb96205ed/src/index.ts#L142

I would like to add a static constraint on the type argument, so that only types that serialize to { [key: string]: string } will be accepted

@sledorze
Copy link
Collaborator

@gcanti I had similar needs after the upgrade (and found 3 places where some bugs could have bitten us).

@gcanti gcanti force-pushed the output-type-parameter branch 3 times, most recently from 963d4a3 to 46232c4 Compare February 6, 2018 15:10
@gcanti gcanti changed the title add output type parameter 1.0.0 Feb 10, 2018
@gcanti gcanti added WIP and removed discussion labels Feb 10, 2018
@sledorze
Copy link
Collaborator

sledorze commented Feb 12, 2018

@gcanti I miss the ability to define the Output type with Tagged and that's currently a blocker.

t.Type<A, Output, I> // <- that `Output` type

I would like to be able to write:

t.Tagged<'type', MyInnerType, MyOutputType> 

The reason is that I have some transforming decoders and current definition of Tagged rely on the fact that Tagged io-ts types have the same Inner and Output types.

@sledorze sledorze mentioned this pull request Feb 12, 2018
@gcanti
Copy link
Owner Author

gcanti commented Feb 12, 2018

@sledorze the "input" type comes last

// O = output type (encode to)
// I = input type (decode from)
export class Type<A, O = A, I = mixed> implements Decoder<I, A>, Encoder<A, O> {
  ...
}

so not sure I understand the issue, have you got a self contained example I can try?

@sledorze
Copy link
Collaborator

@gcanti I messed with the naming, it's really the 'Output' type.

@gcanti
Copy link
Owner Author

gcanti commented Feb 12, 2018

@sledorze so you want to be able to define the O type right? i.e. from

export type Tagged<Tag extends string, A = any>

to

export type Tagged<Tag extends string, A = any, O = A>

@sledorze
Copy link
Collaborator

sledorze commented Feb 12, 2018

@gcanti here's the repro:

let x: t.Type<string, t.mixed, t.mixed> = <any>1

interface Bar {
  type: 'foo'
  x: string
}
const Bar: t.Tagged<'type', Bar> = t.interface({
  type: t.literal('foo'),
  x
})

The compiler complains:

[ts]
Type 'InterfaceType<{ type: LiteralType<"foo">; x: Type<string, mixed, mixed>; }, TypeOfProps<{ type: L...' is not assignable to type 'Tagged<"type", Bar>'.
  Type 'InterfaceType<{ type: LiteralType<"foo">; x: Type<string, mixed, mixed>; }, TypeOfProps<{ type: L...' is not assignable to type 'InterfaceType<TaggedProps<"type">, Bar, Bar, mixed>'.
    Types of property 'encode' are incompatible.
      Type 'Encode<TypeOfProps<{ type: LiteralType<"foo">; x: Type<string, mixed, mixed>; }>, OutputOfProps<{...' is not assignable to type 'Encode<Bar, Bar>'.
        Type 'OutputOfProps<{ type: LiteralType<"foo">; x: Type<string, mixed, mixed>; }>' is not assignable to type 'Bar'

I think it's self explanatory :)

If you change x definition to:

let x: t.Type<string, t.mixed, t.mixed> = <any>1

Then everything types check as expected.

Yes, your proposal would suit my needs (I'm not in need to specify the Input type ATM btw, but that can't hurt).

@gcanti
Copy link
Owner Author

gcanti commented Feb 12, 2018

Ok thanks. So this should do the trick (?)

export type TaggedProps<Tag extends string> = { [K in Tag]: LiteralType<any> }
export interface TaggedRefinement<Tag extends string, A, O = A> extends RefinementType<Tagged<Tag>, A, O> {}
export interface TaggedUnion<Tag extends string, A, O = A> extends UnionType<Array<Tagged<Tag>>, A, O> {}
export type TaggedIntersectionArgument<Tag extends string> =
  | [Tagged<Tag>]
  | [Tagged<Tag>, Mixed]
  | [Mixed, Tagged<Tag>]
  | [Tagged<Tag>, Mixed, Mixed]
  | [Mixed, Tagged<Tag>, Mixed]
  | [Mixed, Mixed, Tagged<Tag>]
  | [Tagged<Tag>, Mixed, Mixed, Mixed]
  | [Mixed, Tagged<Tag>, Mixed, Mixed]
  | [Mixed, Mixed, Tagged<Tag>, Mixed]
  | [Mixed, Mixed, Mixed, Tagged<Tag>]
  | [Tagged<Tag>, Mixed, Mixed, Mixed, Mixed]
  | [Mixed, Tagged<Tag>, Mixed, Mixed, Mixed]
  | [Mixed, Mixed, Tagged<Tag>, Mixed, Mixed]
  | [Mixed, Mixed, Mixed, Tagged<Tag>, Mixed]
  | [Mixed, Mixed, Mixed, Mixed, Tagged<Tag>]
export interface TaggedIntersection<Tag extends string, A, O = A>
  extends IntersectionType<TaggedIntersectionArgument<Tag>, A, O> {}
export type Tagged<Tag extends string, A = any, O = A> = // <= added O here
  | InterfaceType<TaggedProps<Tag>, A, O>
  | StrictType<TaggedProps<Tag>, A, O>
  | TaggedRefinement<Tag, A, O>
  | TaggedUnion<Tag, A, O>
  | TaggedIntersection<Tag, A, O>

Could you please try it out overriding your local definition file? If it's ok I'll push a commit accordingly

@sledorze
Copy link
Collaborator

sledorze commented Feb 12, 2018

@gcanti Yes that's exactly what was needed.
I've checked; it works.

@sledorze
Copy link
Collaborator

Side note: the default signature are nice as they're conservative regarding the type and preserve more information.
However, in realworld, we often use transforming types; like Dates or other custom representations.
Maybe it would be interesting to offer a shortcut alias like so:

 WondefulAliasName<Tag, A> = Tagged<Tag, A, t.mixed>

@gcanti
Copy link
Owner Author

gcanti commented Feb 12, 2018

gcanti/io-ts#next updated

Maybe it would be interesting to offer a shortcut alias like so

mixed in output position is lossy though and should be an exception not the norm IMO

// why mixed in output position here?
let x: t.Type<string, t.mixed, t.mixed> = <any>1

For example in case of Dates you could define

declare const DateFromISOString: t.Type<Date, string, mixed>

declare const DateFromMillis: t.Type<Date, number, mixed>

which are way better than t.Type<Date, mixed, mixed>

@sledorze
Copy link
Collaborator

@gcanti you are right, but the issue is that it introduces two types to build and maintain (think those dates can be deeply nested into structures)
This is seen as too-much-work-as-we-dont-care-about-output-type for end user (library) which then rely on t.mixed.

@sledorze
Copy link
Collaborator

@gcanti it's a side note; it is really not required to be in the lib.

@gcanti
Copy link
Owner Author

gcanti commented Feb 12, 2018

the issue is that it introduces two types to build and maintain

Right, but only if you cast manually (like you do with Tagged), usually the standard combinators will take care of such nesting automatically


On another point, one thing that currently concerns me is the readability of the derived types and compiler erros. Just pushed a commit that should improve the current situation:

Example:

From

'{ c: number; }' is not assignable to type 'TypeOfDictionary<KeyofType<{ a: boolean; }>, NumberType>'
Type 'true' is not assignable to type 'TypeOfProps<{ type: LiteralType<"a">; }> | TypeOfProps<{ type: LiteralType<"b">; }>'

to

Type '{ c: number; }' is not assignable to type '{ a: number; }'.
Type 'true' is not assignable to type '{ type: "a"; } | { type: "b"; }'

@sledorze
Copy link
Collaborator

@gcanti actually we cast types to interfaces in order to keep error messages lean AND work with meaningful types following our business requierements.
I will try to override only the first type param and so how it goes.

@sledorze
Copy link
Collaborator

sledorze commented Feb 12, 2018

@gcanti all code base migrated.
no major issues encountered (but the need to expose more type params to Tagged - which you enabled timely).
I would rate the lib as ready for consumption! :)

@gcanti
Copy link
Owner Author

gcanti commented Feb 12, 2018

Awesome, thanks a lot for your help.

For what concerns the issue above, I'm interested in finding a solution, however I can't do much more right now. I propose to postpone the discussion after the release, I'll open an issue after some thoughts.

@sledorze
Copy link
Collaborator

@gcanti that's great!
I would love to have all this released indeed so that I'll not loose all the conversion work :)

@gcanti gcanti merged commit 5e504f6 into master Feb 12, 2018
@gcanti gcanti deleted the output-type-parameter branch February 12, 2018 14:53
@gcanti
Copy link
Owner Author

gcanti commented Feb 12, 2018

I would love to have all this released

@sledorze done!

@sledorze
Copy link
Collaborator

@gcanti Awesome! Thanks a lot!!!!!!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants