-
-
Notifications
You must be signed in to change notification settings - Fork 10.5k
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
deps: [email protected] closes #6242 coerce input using input + '' #6472
Conversation
@@ -17,19 +17,19 @@ var schema = require('../schema').tables, | |||
// Provide a few custom validators | |||
// | |||
validator.extend('empty', function empty(str) { | |||
return _.isEmpty(str); | |||
return _.isEmpty(str+''); |
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
Hey @ajays91, thanks for contributing! First off, can you fix up the commit as per the outline in #6462 (add the line breaks)? Also, it closes 6462 not 6242 😉 Also, the tests are failing because you don't have spaces between the changes you added. See my line comment. Lastly, is this the best way to fix the deprecation notices? It seems like a bit of a hack. I'm not saying it isn't, but to me it looks odd. |
Hey @acburdine My bad for 6242 mistake. 😅 |
deps: [email protected] closes TryGhost#6462 coerce input using input + ''
@ajays91 you have to use git rebase to squash the commits down into one 😄 something like Also, I missed this earlier, but your commit doesn't actually update validator.js 😄 Update it to version 4.7.1 (4.7.2 doesn't exist 😉). |
I agree with @acburdine's original assessment, that this is not the best way to fix the deprecation notices. Coercing the type is something that should be done as a last resort, in a controlled fashion when you know it's the best option. By doing this in all of our custom validators, all underlying issues in the code base are being actively masked, rather than solved. Further, it only fixes issues where they use our custom validators. Deprecation notices are still being thrown where non-strings are being passed to other validator functions see example. I would expect the result of this PR to be an investigation into where the deprecations are coming from, with a set of fixes. Those fixes should only be coercions where absolutely necessary. |
Closing in favour of #6645 |
No description provided.