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

Pass the field/key when errorMessage is a function #89

Merged
merged 4 commits into from
Jul 23, 2017

Conversation

Emilios1995
Copy link
Contributor

It is very common to include the name of the field in the error message.
I thought it convenient to include pass it when errorMessage is a function.

Here's how I'm using this feature (from my fork):

const isType = R.curry((type, field) => R.type(field) === type);
const typeMessage = (type, field) => `${field} has to be a ${type}`;
const typeRule = (type) => [
  isType(type),
  (val, field) => typeMessage(type, field)
];

const validationRules = {
  name: [typeRule("String")]
}

@busypeoples
Copy link
Owner

I like the approach, very nice example of how to create a [pred, errorMsg] tuple.

const typeRule = (type) => [
  isType(type),
  (val, field) => typeMessage(type, field)
];

Just so I fully understand, this:

const isType = R.curry((type, field) => R.type(field) === type);

should be:

const isType = R.curry((type, val) => R.type(val) === type);

right?

@Emilios1995
Copy link
Contributor Author

Exactly! Sorry, my bad.

@busypeoples
Copy link
Owner

Yes, I think this is very cool!

@busypeoples
Copy link
Owner

We should add this to the 0.6 release. Will probably merge this tomorrow.
I also like the example

const validationRules = {
  name: [typeRule("String")]
}

shows how nicely one can create rules.

@busypeoples
Copy link
Owner

@Emilios1995 Great work!

@Emilios1995
Copy link
Contributor Author

Thanks, I'm glad you liked it!

@Emilios1995
Copy link
Contributor Author

Emilios1995 commented Jul 20, 2017

We should also document this in the API docs. I'll probably make a commit for that later.

@busypeoples
Copy link
Owner

busypeoples commented Jul 20, 2017

We should als add a test, but I can write that one tomorrow, if you have no time for that.

@Emilios1995
Copy link
Contributor Author

@busypeoples I'll try to do both things tonight. I'll ping you tomorrow if I have any trouble.

@Emilios1995
Copy link
Contributor Author

Emilios1995 commented Jul 21, 2017

Done! But I'm not sure if I left that test too complex or the docs too verbose. What do you think?

P.S. I would also love to see something like my typeRule function somewhere in the readme, but I'll leave that to your consideration.

@busypeoples
Copy link
Owner

@Emilios1995 Thank you very much!
I'll look into it in more detail in a couple of hours. But looks good after quickly going through the commits. I will merge later on today.

@busypeoples
Copy link
Owner

busypeoples commented Jul 21, 2017

Regarding the typeRule, definitely. That example you have is more than elegant, and displays nicely how one can compose rules to bigger rules etc.

@busypeoples busypeoples merged commit e7546e1 into busypeoples:master Jul 23, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants