"validConfig" is incorrectly expected to be an Object in "allValid" test util
le0nik opened this issue · 6 comments
This is the code in question: https://github.com/js-dxtools/webpack-validator/blob/v2.2.6/test/utils/allValid.js#L11
It's expected here, that input is always of type Object, although in tests very often strings are passed into it. Like here: https://github.com/js-dxtools/webpack-validator/blob/v2.2.6/src/properties/devtool/index.test.js#L13
Is this check for validConfig needed here at all? If yes, shouldn't it be checked for undefined instead?
I'm asking because I was trying to allow false value to be passed into devtool, because it's a valid value(see here: https://github.com/webpack/webpack/blob/40aac864fd88495cd56dbec9bab522f8f34a47ea/lib/WebpackOptionsDefaulter.js#L11).
I add it on this line: https://github.com/js-dxtools/webpack-validator/blob/v2.2.6/src/properties/devtool/index.test.js#L13 like this:
[
//...
{ input: false },
]So false is obviously a falsy value and is not considered to be a validConfig, so the test throws.
I'm kind of stuck with the PR for devtool because of it. I suppose this check is a legacy code. Am I wrong?
Is this check for validConfig needed here at all? If yes, shouldn't it be checked for undefined instead?
I think you're correct 👍
@kentcdodds
Would you accept a PR for devtool where I'd also change this check to if (typeof validConfig === undefined)?
Yup!
Ok, will do it soon.
Merged