feat: add new options to string scheme

bitkidd commented 3 years ago

Proposed changes

Add case (lowerCase, upperCase, camelCase, snakeCase, dashCase, pascalCase, capitalCase, sentenceCase, dotCase, titleCase, noCase) options.
Add pluralize, singularize and condenseWhitespace options

Types of changes

What types of changes does your code introduce?

[ ] Bugfix (non-breaking change which fixes an issue)
[x] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist

[x] I have read the CONTRIBUTING doc
[x] Lint and unit tests pass locally with my changes
[x] I have added tests that prove my fix is effective or that my feature works.
[ ] I have added necessary documentation (if appropriate)

thetutlage commented 3 years ago

Looks great. However, I suggest converting it to a rule. It can be one rule that is changeCase or can be multiple rules. I think one rule is fine.

Why convert it to a rule?

I am also going to convert the trim and escape options to rules, so that the end user can decide when to apply them. For example:

Right now, the minLength and the maxLength rules validate the original value and not the trimmed value (if the option is used). For some, this behavior is fine and for others this behavior is problematic.

To solve this, if I convert the trim option to a rule, then the end user can decide when to apply based upon their scenario.

schema.string({}, [
  rules.trim()
  // Apply trim before
  rules.minLength(4)
])

schema.string({}, [
  rules.minLength(4)
  // Apply trim after
  rules.trim()
])

I know that your rule is not prone to this behavior. But I will love to unify the API and get rid of the options all together

bitkidd commented 3 years ago

It is a great idea to convert options to a separate set of rules, but should it be really a rule?

Feels like a rule is a logic operator, it could be a complex one, but this operator semantically should answer a single question, for example for rule.email it asks a question if the string is email and then answers true or false.

Wouldn't it be better to implement something like modifiers that can be used in the same array with rules and basically will just mutate the values, but semantically you will know what to wait from them, they get a value and modify, the naming is just an example, it can be anything really, sanitizers, modifiers, mutators.

The actual use may look like this:

schema.string([
  modifiers.trim()
  // Apply trim before
  rules.minLength(4)
])

schema.string([
  rules.minLength(4)
  // Apply trim after
  modifiers.trim()
])

What do you think @thetutlage?

thetutlage commented 3 years ago

Yup, I like the modifiers approach. Now this brings me to an internal discussion I had with @targos and @RomainLanz.

I think the validator API needs some love. The most radical approach will be to use a chainable API like the following.

schema
  .string()
  .email()
  .trim()
  .minLength(10)
  .changeCase('camelCase')

I call it radical, coz it is a completely different API and I am not interested in introducing a breaking change at all.

But, then I have other ideas in mind like the ability to define a custom field name which is different from the schema property name. For example:

firstName: schema.string().field('first_name')

With the chainable API it is simple, coz each function is not part of any group. Whereas with existing API, what will we call this method? Like we call rules for validation rules, modifiers for mutating the field value. What is this 3rd thing called?

firstName: schema.string({}, [
  what.field('first_name')
])

Any suggestions or ideas?

bitkidd commented 3 years ago

Current validator implementation is one of my beloved functionalities in adonis really, as it allows in a very structured and predictable manner build data schemes.

If speaking about changing field name, we can easily add this to modifiers group, as it semantically modifies/mutates data, the approach is not ideal but it will not lead to a breaking change. The new functionality can be added leaving old options api that will allow developers to adapt and prepare.

As for chainable API, this looks good too, but I feel it a bit less structured in comparison to what we have now.

bitkidd commented 3 years ago

The problem with modifiers approach rises with scheme.file where there are real options, that cannot be categorised as a rule or modifier, for example:

schema.file({
  size: '2mb',
  extnames: ['jpg', 'gif', 'png'],
})

In this case chainable API resolves the problem.

scheme.file().size('2mb').extnames(['jpg', 'gif', 'png'])

thetutlage commented 3 years ago

Great. @RomainLanz also thinks the current validator API is more logical than the chainable API. So let's not change too many things and add support for modifiers.

Now, if you need this feature immediately, then we can go ahead and implement it. Otherwise, I will suggest going the RFC route to solidify the API even further.

If you want, I can initiate the RFC this weekend and then we can exchange ideas and finalize it

bitkidd commented 3 years ago

I don't need the change right away. Let's initiate RFC and discuss the change there, so maybe other ideas arise.

bitkidd commented 3 years ago

If you want, I can do the RFC tomorrow and submit it, the repo is: https://github.com/adonisjs/rfcs Right?

thetutlage commented 3 years ago

Sure, that will be great. Just checkout the websocket rfc as an inspiration

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

RomainLanz commented 3 years ago

Here is the link to the RFC: https://github.com/adonisjs/rfcs/pull/40.

Closing in the meanwhile.

adonisjs / validator