BUG: string with pattern and empty not set: intentional breaking change?

[JMS-1]

Our rule is (simplified here) {type:"string", pattern:"^\d+$"} which in 1.4.2 rejects empty strings. Now with 1.5.0 empty strings are allowed which breaks our app!

... if (${schema.empty} !== false && len === 0) { // Do nothing } else { ${patternValidator} } ...

For a pure string validation not setting empty will allow empty strings which is a good (an non-breaking) default. But maybe as soon as a pattern comes into play it may be better to default empty to false to allow for the pattern to decide. If empty is explicitly set the test makes sense.

Although currently I can see no perfect behaviour - all can be wrong in some situations.

[erfanium]

@JMS-1 You can use default configs for fix this issue quickly now:

const v = new Validator({
   defaults: {
      string: {
         empty: false
      }
   }
})

[erfanium]

@icebob As I said before, it's better to not accept empty strings by default, because In most cases, empty strings are not desirable

[JMS-1]

@JMS-1 You can use default configs for fix this issue quickly now:

const v = new Validator({
   defaults: {
      string: {
         empty: false
      }
   }
})

well as you might imagine there a couple of other type: "string" validations so a global default is not an option. but yes: this is what I did for all validations using pattern: "..." - nasty point is that in case of an empty string there are two ValidationError entries which messes a bit with the generic edit form and error reporting therein. But for the moment it's good enough - better than writing invalid entities into the database.

[JMS-1]

@icebob As I said before, it's better to not accept empty strings by default, because In most cases, empty strings are not desirable

In general I would I agree but especially in our case we have a couple of strings which have fallbacks. So not setting a string at all which gives the fallbacks differs a lot from having an empty string.

Besides this would be a major breaking change which you should not put in a minor update - to say something that may automatically update on a fresh npm install.

[icebob]

@JMS-1 Thanks for the report. The problem is that, if we change the empty: false by default, it means breaking change again. We can solve it if we revert this commit: 3fd9275

[icebob]

Fix released in 1.5.1