busypeoples / spected

Validation library
MIT License
703 stars 32 forks source link

Pass the top-level inputs object to the predicate functions as a third argument. #102

Open boyko opened 6 years ago

boyko commented 6 years ago

Hello and thanks for this great library! A while ago I needed to validate a structure like this one:

const obj = {
    person: { id: 'someId'},
    car: {
        owner: 'someId'
    }
}

where the car.owner needs to match person.id in order to pass validation. Maybe I got it totally wrong but my first attempt was:

const validationRules = {
    car: {
        owner: [
            [
                (value, inputs) => {
                        if (value === inputs.person.id) {
                                return true;
                        }
                        return false;
                },
                'Car not owned by person.'
            ]
        ]
    }
}

It failed because the predicate function is only passed the car object and not the whole input.

This PR changed the validation function so that it passes the top-level input as a third argument to the predicate functions so that the above validation problem can be tackled like this:

const validationRules = {
    car: {
        owner: [
            [
                (value, input, allInputs) => {
                        if (value === allInputs.person.id) {
                                return true;
                        }
                        return false;
                },
                'Car not owned by person.'
            ]
        ]
    }
}
busypeoples commented 6 years ago

Thanks for the great input @boyko I think this is very similar to https://github.com/25th-floor/spected/issues/100 from @davidchase Which would also solve the problem mentioned in above issue more elegantly. I will have a look at your implementation tomorrow or Friday and either provide feedback or straight merge.

davidchase commented 6 years ago

@boyko @busypeoples 👋 this looks really great! i havent tested it out in my scenario just yet (busy @ work) but this could be it thanks a bunch 😄

busypeoples commented 6 years ago

Didn't have the time to run this. Will try to check it out today or tomorrow and then merge.

benneq commented 5 years ago

The 2nd argument of the predicate is the "parent" object. I don't think that "allInputs" as 3rd argument is a good idea in general.

I'd go for some kind of "composable validation specs". What I mean: Let's say, that you want to reuse your personEqualsCarOwnerRule in some other context, then you cannot know where inside the allInputs your personEqualsCarOwnerRule is.

Example:

const data1 = {
  person: { id: 'someId' },
  car: { owner: 'someId' }
};

const data2 = {
  foo: {
    person: { id: 'someId' },
    car: { owner: 'someId' }
  }
};

const personEqualsCarOwnerRule = {
  person: [[...]]
  car: {
    owner: [[
      (input, allInputs) => ???, // for data1 it would check "allInputs.person.id"
      // but for data2 it would check "allInputs.foo.person.id"
      // this means: you cannot reuse this rule that easy.
      'Car not owned by person.'
    ]]
  }
}

spected(data1, personEqualsCarOwnerRule);
spected(data2, { foo: personEqualsCarOwnerRule });

Of course you could use something like this with allInputs:

const personEqualsCarOwnerRule = (path) => ({
  person: [[...]]
  car: {
    owner: [[
      (input, allInputs) => ???, // then use "allInputs[path].person.id"
      'Car not owned by person.'
    ]]
  }
})

spected(data2, { foo: personEqualsCarOwnerRule("foo") });

But this doesn't look nice for deeply nested stuff. And it gets really hard when it comes to arrays:

const data3 = {
  bar: [
    {
      foo: {
        person: { id: 'someId' },
        car: { owner: 'someId' }
      }
    },
    ...
  ]
}

Now you've got to access allInputs.bar[0].foo.person.id.


I think a much better solution would be if we were able to access parent, parent.parent, parent.parent.parent, ... within the predicate.

Then you could simply write:

const personEqualsCarOwnerRule = () => ({
  person: [[...]]
  car: {
    owner: [[
      (input, parent, parentParent) => ???, // use "parentParent.person.id"
      'Car not owned by person.'
    ]]
  }
})

And this rule can now be applied to all objects with a { person: { id }, car: { owner } } structure.

Though I'm not sure if passing parent, parentParent, parentParentParent, ... as arguments to the predicate is the way to go.

Another possibility would be something like this:

const personEqualsCarOwnerRule = () => ({
  person: [[...]]
  car: {
    owner: [[
      (input, ancestors) => ???, // where ancestors is an array
      // and each index contains one level deeper parent:
      // ancestors[0] === car object === { owner }
      // ancestors[1] === car's parent object === { person: { id }, car: { owner } }
      // So you would use `ancestors[1].person.id`
      'Car not owned by person.'
    ]]
  }
})

Maybe there are other solution, too, that are even nicer to use.


EDIT: Another solution could be some kind of ref. I don't know if this could be implemented somehow. It's just an idea:

const personEqualsCarOwnerRule = () => {
  const ref = { actualRef: null };
  return {
    person: compose(createRef(ref), [[...]]) // compose will just somehow (magically)
    // run the "createRef" and then return the validations array.
    // "createRef" will simply set the "actualRef" within the "ref" object.
    car: {
      owner: [[
        (input) => ???, // use "ref.actualRef.id"
        'Car not owned by person.'
      ]]
    }
  }
}
busypeoples commented 5 years ago

Thanks @benneq and @boyko for all the input! Let's think about the possible ways to tackle this.

benneq commented 5 years ago

For maximum flexibility, I really like the ref approach. This way, you don't have to rely on the object structure at all. Because you can reference everything you like.

But for refs, we need #104 / #106 , I guess.

Using ref would be pretty similar to using allInputs: You could simply create a ref of the root and call it allInputs. But additionally you are also able to create a ref for a smaller part of the object tree.


The parent, parent.parent, ... stuff is nice, too. Because it's simple. But it will become pretty ugly when you have deep objects trees. And: You lose some flexibility which you could have with refs.

Example (pseudo code):

const fooRule = {
  bar: ...,
  baz: ...
}

const rootRule = {
  a: ...,
  b: ...,
  foo: fooRule,
  nested: {
    xyz: fooRule
  }
}

Now you want to create some rule for fooRule.bar:

const rootRule = () => { const aRef = ...; return { a: ..., // createRef here b: ..., foo: fooRule(aRef), nested: { xyz: fooRule(aRef) } ) }



---

Providing `allInputs`, as I already said above, doesn't feel nice. Yes, it will work, if you have a single static validation object and you are 100% sure about its structure. But it will fail if you want to build small composable rules, and then combine them.

--- 

Looking at other validation libs: For example `yup` is using string refs. 1. I don't like to use strings for matching field by name. 2. it only supports siblings (and their children). So it's basically the same as the current 2nd arg for the predicate of spected.