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

Keep Refinements? #373

Closed
mikecann opened this issue Oct 20, 2019 · 18 comments
Closed

Keep Refinements? #373

mikecann opened this issue Oct 20, 2019 · 18 comments

Comments

@mikecann
Copy link

mikecann commented Oct 20, 2019

Hi,

Apologies if this doesn't follow the usual issue convention.

I noticed that refinements are now deprecated in favour of brands. Im just wondering the reason for this exactly as the new syntax for brands is considerably more verbose and is limited.

Previously I enjoyed writing nice terse validation such as:

const rxEmail = /^[^@\s]+@[^@\s]+\.[^@\s]+$/;

const Email = t.refinement(t.string, s => rxEmail.test(s), "Email");

const MaxLengthString = (len: number) =>
  t.refinement(t.string, s => s.length <= len, `MaxLengthString${len}`);

const EmailSubject = MaxLengthString(50);

export const GuestSenderDetails = t.strict({
  name: t.string,
  email: Email,
});

export const NewVideoMessageDetails = t.strict({
  subject: EmailSubject,
  sender: GuestSenderDetails,
});

Now with brands this would be considerably more verbose and im not even sure if MaxLengthString is even possible.

Thoughts?

@giogonzo
Copy link
Contributor

Hi @mikecann , the problem with "old" refinements is that they don't carry the information at the type level, so for instance, with your definition, the following is allowed:

const details: t.TypeOf<typeof NewVideoMessageDetails> = {
  subject: "", // length = 0! but allowed
  sender: {
    name: "name",
    email: "foo" // invalid! but allowed
  }
};

The cost you have to pay to use refinements is just adding one more interface per refinement you define, which looks legit if you consider the benefits. So for instance:

// this is the added brand
interface EmailBrand {
  readonly Email: unique symbol;
}
// this is the same as before + the t.Branded annotation
const Email = t.brand(
  t.string,
  (s): s is t.Branded<string, EmailBrand> => rxEmail.test(s),
  "Email"
);

// this is the added brand
interface MaxLengthString<N> {
  readonly MaxLengthString: unique symbol;
  readonly length: N;
}
const MaxLengthString = <N extends number>(len: N) =>
  // this is the same as before + the t.Branded annotation
  t.brand(
    t.string,
    (s): s is t.Branded<string, MaxLengthString<N>> => s.length <= len,
    "MaxLengthString"
  );

// ... 

const details: t.TypeOf<typeof NewVideoMessageDetails> = {
  subject: "", // correctly errors: Type 'string' is not assignable to type 'Branded<string, MaxLengthString<50>>'
  sender: {
    name: "name",
    email: "foo" // correctly errors: Type 'string' is not assignable to type 'Branded<string, EmailBrand>'
  }
};

im not even sure if MaxLengthString is even possible.

yes, bonus:

const EmailSubject = MaxLengthString(50);
const Another = MaxLengthString(20);

declare let one: t.TypeOf<typeof EmailSubject>;
declare let two: t.TypeOf<typeof Another>;

one = two; // Type '20' is not assignable to type '50'
two = one; // Type '50' is not assignable to type '20'

@mikecann
Copy link
Author

Ye.. Its interesting as I had never seen that unique keyword before..

So I guess its basically undoing the "structural" typing nature of Typescript and making it more "nominal" ? (not sure if I got the terminology right here) So it works more like other functional languages?

Im not 100% sold the extra typing here is worth the benefit but I guess io-ts or fp-ts was never going to be perfect while building ontop of TS :)

@giogonzo
Copy link
Contributor

So I guess its basically undoing the "structural" typing nature of Typescript and making it more "nominal" ? (not sure if I got the terminology right here) So it works more like other functional languages?

Correct. The TS team is currently investigating different ways to add support for nominal types directly in the language, so hopefully things are going to improve in the future.
For the time being, this "trick" is the best we have :)

@mmkal
Copy link
Contributor

mmkal commented Oct 20, 2019

@mikecann @giogonzo I was actually planning on opening an issue just like this this week. Would you consider reopening?

Even though you are right that branded types have better type safety, the extra boilerplate is often not worth it. io-ts doesn't need to perfectly correspond to the static type system in all cases to be useful. One other problem with branded types is they're not just a pain to set up, they're also harder to use, and sometimes require non-trivial refactors. In the team I'm on, if we didn't have refinements, I'd worry people would be disincentivised from adding the extra type safety of a predicate verifier at all.

To use the example above, Branded types might disallow an invalid email but they also disallow a valid email.

const details: t.TypeOf<typeof NewVideoMessageDetails> = {
  subject: "Hi", // incorrectly errors: Type 'string' is not assignable to type 'Branded<string, MaxLengthString<50>>'
  sender: {
    name: "name",
    email: "a@b.com" // incorrectly errors: Type 'string' is not assignable to type 'Branded<string, EmailBrand>'
  }
};

(in fact, even in the example given, subject is incorrectly erroring - an empty string does have length <50)

To be clear, I 100% agree that t.brand is a necessary addition to io-ts. The issue here is over whether t.refinement has a role anyway. I believe it does - so I'm hoping it's deprecation can change to a link to some documentation about the difference between t.brand and t.refinement, and why t.brand may be preferable. Maybe a simple way to consider it is that it's use-case specific - would the developer prefer false negatives, or false positives?

@giogonzo
Copy link
Contributor

To use the example above, Branded types might disallow an invalid email but they also disallow a valid email.

Yes, to clarify, they disallow values of the "carrier" type (e.g. string here), avoiding a whole set of errors at compile time (which is the point of having brands).

The example was a bit contrived (and wrong in some sense, as you point out) but, generally speaking, you'll obtain branded values from:

  1. the IO boundary (HTTP, HTML forms, file system, database, ...)
  2. in the core of the app, e.g. in tests or for some hardcoded value

For 1., they should be validated anyway, no overhead in usage
For 2., the cost is usually exporting a utility function able to create e.g. an Email out of the blue:

export function unsafeEmail(s: string): Email {
  return s as any
}

Usages of this function are in my experience very rare, usually for testing purposes or developing with mock values - something that is not part of the final source code anyway.


io-ts doesn't need to perfectly correspondent to the static type system in all cases to be useful

Actually I believe this is one of the main goals of io-ts, being a precise mapping to the TS type system.

I understand this:

I'd worry people would be disincentivised from adding the extra type safety of a predicate verifier at all.
The issue here is over whether t.refinement has a role anyway. I believe it does

and it makes me think sometimes refinements are just used to e.g. provide nice errors to the users of your system, but not really to obtain more type-safety throughout the codebase.
If this is the case, I believe there are other libraries out there (off the top of my head: joi, class-validator, runtypes) that are much less concerned with static type safety, and could be a better match for your use case.

For more context, please also have a look at #265 and #304 where Giulio does a great job at explaining the whys behind brands and "smart constructors"

Hope this helps :)

@mmkal
Copy link
Contributor

mmkal commented Oct 20, 2019

Usages of this function are in my experience very rare, usually for testing purposes or developing with mock values - something that is not part of the final source code anyway.

Here's another use case. We have a system which builds a server with an io-ts schema. The schema is used to decode incoming requests, but it's also used to generate a client which encodes the request before sending with an http client. This gives very nice type safety (the kind joi, class-validator, runtypes etc. can't give) when used with types like DateFromISOString:

schema:

...
search: {
  request: t.type({
    query: t.partial({ before: DateFromISOString, after: DateFromISOString })
  }),
  response: t.type({ ... }),
}
...

The library we use decodes requests for us, so server implementors only have to deal with Date objects which they already know are valid.

The nice thing is, clients can now also only deal with Date objects, which the corresponding client system encodes for them. This meaning it becomes hard to send a bad request:

client:

await myClient.search({ query: { before: new Date('2020'), after: new Date('2010') } })
// library uses io-ts to encode this and send `GET /search?before=2020&after=2010`

The benefit is that invalid/unsupported formats are now a compile-time error on the client side - this gives error: string not assignable to Date:

await myClient.search({ query: { before: 'tomorrow', after: 'yesterday' } })

Branded types make things tricky. Let's say we want to add a limit query param to constrain search result size:

new schema:

...
search: {
  request: t.type({
    query: t.partial({ limit: t.Int, before: DateFromISOString, after: DateFromISOString })
  }),
  response: t.type({ ... }),
}
...

client:

await myClient.search({ query: { limit: 10, before: new Date('2020') } })

This will error, because typescript doesn't know that 10 is an integer. To get around this, client developers will be incentivised to make weird workarounds. Here, it'll probably either be decoding the value 10, which is silly, or casting to any, or abandoning the strongly-typed library, which are dangerous. Since the value is being decoded on the server-side anyway, we'd gain very little by decoding it on the client side before immediately re-encoding to a raw number.

I would argue this is an example of typescript and io-ts not corresponding perfectly already. t.brand is out of sync with typescript for encode use cases, t.refinement is out of sync for decode use cases. The decode use cases are more common, so it makes sense to promote brand as the usually-preferred solution. But again, I do feel the use of t.refinement here is legitimate. Tests, as you say, are another use case, so it seems extreme to say we should move to another library for this. The other libraries you suggest provide validators rather than codecs.

Right now, the solution is easy: we use t.Integer, but that's deprecated since it's a refinement rather than a brand. It'd hurt us if it went away.

Is the plan to keep refinements around at least until a solution for microsoft/TypeScript#202 lands? Re-evaluating then may be a good compromise, since likely branding will change at that point anyway.

@giogonzo
Copy link
Contributor

(sorry I'm from mobile)

Thanks for the explanation, yes we use similar setups and it's just a joi (😂) to work with it fullstack

To get around this, client developers will be incentivised to make weird workarounds

I'd argue that client developers should validate the limit if it's user input (basically my case 1. from last comment). If it's not, then I'd say this is case 2. and values of type Int are hardcoded - in this scenario they can be hardcoded using the cast once in the project as described above

@mikecann
Copy link
Author

Im reopening this because I think I agree with the comments of @mmkal personally im not sure I would want to use io-ts if we have to use t.brand with the current syntax.

For me (and probably a lot of other TS developers) Typing is about pragmatism and im always looking for a way to increase safely without extra typing overhead and t.brand really does add a lot more syntax and clutter, confusing the code.

If the solution is to pull in another library (joi, class-validator or whatever) then maybe I will go with just one of those in the future.

Having said that I do appreciate that io-ts really is trying to be something different and is trying to add nominal types on Typescript, thus anything that weakens that goal goes against the library's purpose.

Its a tough one.

@mikecann mikecann reopened this Oct 21, 2019
@osdiab
Copy link
Contributor

osdiab commented Nov 8, 2019

I've run into this as well and it's kept me from actually using t.Int and stuff like that in my company's codebase - I personally don't mind it but I know the rest of my team would find it painful to have to specify things that way, and I really don't want to encourage them to do unsafe casts as workarounds.

as a workaround for the cases where I really do want a branded codec, in the case of calling a REST API, for example, I try to include one-off convenience versions of functions that replace branded input types with unbranded ones, and then decode in the helpers before actually firing off the requests. but this is very one-off as of now.

For anyone interested these types will strip brands from a given branded type, if any exists (the resulting types on hover in VSCode are pretty messy, though):

// These are not tested thoroughly so use at your own risk

/**
 * Remove all brands from a type, if any are present
 */
type Unbranded<T> = T extends Branded<infer Orig, any>
  ? { recurse: Unbranded<Orig>; end: Orig }[Orig extends Branded<any, any>
      ? "recurse"
      : "end"]
  : T;

/**
 * Remove all brands from children of an object
 *
 * Doesn't remove the brand from T if T itself is branded
 */
type DeepUnbrandedChildren<T> = {
  [key in keyof T]: {
    recurse: DeepUnbrandedChildren<Unbranded<T[key]>>;
    end: Unbranded<T[key]>;
  }[Unbranded<T[key]>[keyof Unbranded<T[key]>] extends Branded<any, any>
    ? "recurse"
    : "end"];
};

/**
 * Remove brands from a type and any of its children
 */
type DeepUnbranded<T> = DeepUnbrandedChildren<Unbranded<T>>;

@mnn
Copy link

mnn commented Nov 13, 2019

We chose this library, because it ensures TS types and validation are in sync and we can choose where finer validations take place. We actually want support of potentially invalid data (e.g. form state, store state) to be allowed, we only want that basic TS types (e.g. string, number) are correct. We don't want twice or more types and a lot of other boilerplate (decode+encode) on every place something is being changed.

If this feature gets removed, we either stop updating or move to a fork which still supports it, or modify OpenAPI -> io-ts schema generator to generate joi+TS types.

@osdiab
Copy link
Contributor

osdiab commented Nov 20, 2019

Another issue where it turned out to be annoying for my teammate was that we have a safeIntCodec which basically wraps the Int brand and ensures that the value doesn't go above what's safely expressible in a JavaScript number. but if we want to do some math with it, adding and subtracting values from it, we have to keep applying the codec repeatedly on every intermediate value. It felt really clunky.

Maybe we can make an abstraction (doesn't have to be in io-ts) that can encapsulate the application of the codec for scenarios like that - like by overriding the = method on a class member, or with a specific setter method to implicitly apply the codec on operations like that but without having to unbox it each time, to make the ergonomics nicer.

@mlegenhausen
Copy link
Contributor

@osdiab what you want is a Ring for the Int. Then you can do the math with it. It also ensures that only the operations are available that are allowed for Int. Like div should not be allowed for Int cause the result is not always a whole number.

@osdiab
Copy link
Contributor

osdiab commented Nov 24, 2019

Ah, that looks useful! Thanks for the tip, I’ll figure out later how I can use that to make an easily usable API for that particular use case (ideally not involving manual application of the codec to values repeatedly).

@christianbradley
Copy link

I don't hate the idea of deprecating refinement, but I do think some work could be done to address the amount of boilerplate necessary. This is a runtime type library, not a validation library per se.

@ArmorDarks
Copy link

Another issue with the brands that they use a unique symbol for branding. Now, try npm link package that uses such TS construct, and you will be unpleasantly surprised by type errors because for TS unique symbol carrier must always literally match with an import path (otherwise it's another symbol), which isn't the case in case of linking.

Unfortunately, because of the t.brand types, it's impossible to use some string instead of the Symbol :(

@gcanti
Copy link
Owner

gcanti commented Jan 28, 2021

In the experimental module Decoder I came up with an agnostic API refine

import { Refinement } from 'fp-ts/function'

export declare const refine: <A, B extends A>(
  refinement: Refinement<A, B>, // <= this is `(a: A) => a is B`
  id: string
) => (<I>(from: Decoder<I, A>)

with "agnostic" I mean that, based on the refinement argument, it's up to the user to choose both the level of type-safety and the encoding.

So you can have a branded type (which is advised) with a custom encoding...

import { pipe } from 'fp-ts/function'
import * as D from 'io-ts/Decoder'

type Int = number & { __brand__: 'Int' } // <= choose your preferred encoding here

// const Int: D.Decoder<unknown, Int>
const Int = pipe(
  D.number,
  D.refine((n): n is Int => Number.isInteger(n), 'Int')
)

...or a much simpler resulting type (if you really want to)

// const Integer: D.Decoder<unknown, number>
const Integer = pipe(
  D.number,
  D.refine((n): n is number => Number.isInteger(n), 'Integer')
)

I could revert the refinement deprecation and add an overload:

import { Predicate, Refinement } from 'fp-ts/function'
import * as t from 'io-ts'

// new overloading
export declare function refinement<C extends t.Any, B extends t.TypeOf<C>>(
  codec: C,
  refinement: Refinement<t.TypeOf<C>, B>,
  name?: string
): t.RefinementC<C, B>

// old signature
export declare function refinement<C extends t.Any>(
  codec: C,
  predicate: Predicate<t.TypeOf<C>>,
  name?: string
): t.RefinementC<C>

type Int = number & { __brand__: 'Int' }

// const Int: t.RefinementC<t.NumberC, Int>
const Int = refinement(t.number, (n): n is Int => Number.isInteger(n), 'Int')

// const Integer: t.RefinementC<t.NumberC, number>
const Integer = refinement(t.number, Number.isInteger, 'Integer')

@JalilArfaoui
Copy link

Please @gcanti bring the new overloading refinement definition !

I have many brand types defined with { __brand__: 'Int' } style … It’s the only thing that keep io-ts incompatible but my codebase …

I don’t understand why io-ts should enforce how developers type their refinements, as long as we pass a Refinement

@JalilArfaoui
Copy link

Thank you so much @gcanti

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

10 participants