gcanti / tcomb-validation

Validation library based on type combinators
MIT License
400 stars 23 forks source link

pass context to predicate #58

Closed chirag04 closed 7 years ago

chirag04 commented 7 years ago

allows handling dependent fields better. Person struct can be complex and having to set Person as a refinement on struct and then handle all dependent field in one predicate is not ideal imo.

isValidConfirmPassword = (confirmPassword, context) => context.password === confirmPassword;

const ConfirmPassword = t.refinement(t.String, isValidConfirmPassword);

const Person = t.struct({
  name: t.String,
  password: t.String,
  confirmPassword: ConfirmPassword,
  // ...can have multiple dependent fields here.
}, 'Person');

const formValues = { name: 'a', password: 'b', confirmPassword: 'c' };

validate(formValues, Person, { context: formValues });

cc @itajaja

gcanti commented 7 years ago

Hi @chirag04, thanks for this PR.

Not sure what you gain from this change though.

Currently refinements are type-safe

function refinement<A, T extends Type<A>>(type: T, predicate: (a: A) => boolean))

context can be anything, hence inherently unsafe:

function refinement<A, T extends Type<A>>(type: T, predicate: (a: A, context: any) => boolean))

Also it's not clear what's context in this case

isValidConfirmPassword = (confirmPassword, context) => context.password === confirmPassword;

const ConfirmPassword = t.refinement(t.String, isValidConfirmPassword);

const A = t.struct({
  profile: t.struct({
    username: t.String
    password: t.String,
  }),
  comfirm: t.struct({
    confirmPassword: ConfirmPassword
  })
})
chirag04 commented 7 years ago

@gcanti thanks for the comment.

So the problem i'm trying to address here is to better handle dependent fields. Refer your comment here: https://github.com/gcanti/tcomb-validation/issues/29#issuecomment-141194483. From your suggestion, it seems you recommend nesting dependent fields and having a refinement/subtype on them. taking the example from that suggestion:

function isValid(x) {
  if (t.Dat.is(x.endDate)) {
    return x.endDate.getTime() >= x.startDate.getTime();
  }
  return true;
}

var MyType = t.subtype(t.struct({
  startDate: t.Dat,
  endDate: t.maybe(t.Dat),
  foo: t.Num,
}), isValid);

The problem with above is the top level isValid. In above example i have only one dependent relation between startDate and endDate. Incase of a struct with multiple such dependent relations, my isValid will have to account for all such relations. So instead of having that one giant isValid, i want to break it down on individual fields like endDate, or confirmPassword. eg:

function isValidEndDate(endDate, context) {
  if (t.Dat.is(endDate)) {
    return endDate.getTime() >= context.startDate.getTime();
  }
  return true;
}

var EndDate = t.refinement(t.Dat, isValidEndDate);

var MyType = t.struct({
  startDate: t.Dat,
  endDate: t.maybe(EndDate),
  foo: t.Num,
});

var values = { startDate: 'x', endDate: 'y' };

validate(values, MyType, { context: { ...values } });

About type safety, i'm not an expert here but predicate is a user provided function and can be type checked in userland? This is same as how getValidationErrorMessage works?

About the value of context in your example:

isValidConfirmPassword = (confirmPassword, context) => context.profile.password === confirmPassword;

const ConfirmPassword = t.refinement(t.String, isValidConfirmPassword);

const A = t.struct({
  profile: t.struct({
    username: t.String
    password: t.String,
  }),
  comfirm: t.struct({
    confirmPassword: ConfirmPassword
  })
})
const values = { 
   profile: { username: 'x', password: 'y' }, 
   confirm: { confirmPassword: 'z' },
}
validate(values, A, { context: { ...values } });

Idea is predicate have access to context. How to use the context is one the user. In my case, i want to pass the entire form state in context so that my predicates know about other fields.

Let me know if i need to do a better job of explaining this.

gcanti commented 7 years ago

In my case, i want to pass the entire form state in context so that my predicates know about other fields

I'd do that as well, but if you end up passing in the entire state, what's the benefit of this change with respect to a top level predicate?

So instead of having that one giant isValid, i want to break it down on individual fields like endDate, or confirmPassword

Predicates are highly composable, another option would be to break it down on individual predicates and then compose them.

Example (written in TypeScript)

export type Predicate<A> = (a: A) => boolean

export function and<A, B>(p1: Predicate<A>, p2: Predicate<B>): Predicate<A & B> {
  return a => p1(a) && p2(a)
}

// Lens getter
export type Get<S, A> = (s: S) => A

export function fromGet<S, A>(get: Get<S, A>, p: Predicate<A>): Predicate<S> {
  return s => p(get(s))
}

// Usage

type Range = {
  startDate: Date,
  endDate: Date,
}

type Passwords = {
  password: string,
  confirmPassword: string
}

function isValidRange(x: Range): boolean {
  return x.endDate.getTime() >= x.startDate.getTime()
}

function isValidPasswords(x: Passwords): boolean {
  return x.password === x.confirmPassword
}

type Person = {
  startDate: Date,
  endDate: Date,
  profile: {
    username: string,
    password: string
  },
  confirm: {
    confirmPassword: string
  }
}

const isValidPerson = and(
  fromGet(
    (s: Person) => ({
      password: s.profile.password,
      confirmPassword: s.confirm.confirmPassword
    }),
    isValidPasswords
  ),
  isValidRange
)

const person: Person = {
  startDate: new Date(1000),
  endDate: new Date(2000),
  profile: {
    username: 'foo',
    password: 'bar'
  },
  confirm: {
    confirmPassword: 'baz'
  }
}

console.log(isValidPerson(person)) // false
chirag04 commented 7 years ago

Predicates are highly composable, another option would be to break it down on individual predicates and then compose them.

sure. This is one way of doing it. For me it just convenient to have access to context when working on the predicate vs composing. Also, wondering if there is a downside to passing context into predicate. I'm not sure about type safety.

Feel free to close this if you think composing the the right way to do it.

gcanti commented 7 years ago

Also, wondering if there is a downside to passing context into predicate. I'm not sure about type safety

Indeed my point is type safety: a context that you can only type with any can be convenient but likely leads to runtime errors, while predicates are type-safe (even if you are using JavaScript instead of a statically typed language, a any type is a smell: more room for mistakes)