Add array validators #20
Conversation
|
Perfect so far. |
SamVerschueren
force-pushed
the
array-predicate
branch
from
4fed6e3
to
96da5c3
Compare
source/lib/predicates/array.ts
Outdated
| * | ||
| * @param searchElement The value that should be the start of the array. | ||
| */ | ||
| startsWith(searchElement: any) { |
There was a problem hiding this comment.
This is what you meant, right? Also endsWith.
Currently, this only works for primitive values. Should we use deepStrictEqual as validator function?
There was a problem hiding this comment.
No, it should just test identity, so primitive values or same instance of objects, like it does now. We should mention this though.
Like:
The value is tested by identity, not structure
Or something better.
There was a problem hiding this comment.
Can't really come with anything better for now.
source/lib/predicates/array.ts
Outdated
| * | ||
| * @param searchElement The value that should be the end of the array. | ||
| */ | ||
| endsWith(searchElement: any) { |
There was a problem hiding this comment.
Same comment as startsWith
| includes(...searchElements: any[]) { | ||
| return this.addValidator({ | ||
| message: value => `Expected array to include all elements of \`${JSON.stringify(searchElements)}\`, got \`${JSON.stringify(value)}\``, | ||
| validator: value => searchElements.every(el => value.indexOf(el) !== -1) |
There was a problem hiding this comment.
If we want deep equality, we should write a custom validator.
There was a problem hiding this comment.
Same as my other comment. Should test identity.
| includesAny(...searchElements: any[]) { | ||
| return this.addValidator({ | ||
| message: value => `Expected array to include any element of \`${JSON.stringify(searchElements)}\`, got \`${JSON.stringify(value)}\``, | ||
| validator: value => searchElements.some(el => value.indexOf(el) !== -1) |
There was a problem hiding this comment.
Same comment as includes.
source/lib/predicates/array.ts
Outdated
| deepEqual(expected: any[]) { | ||
| return this.addValidator({ | ||
| message: value => `Expected array to be deeply equal to \`${JSON.stringify(expected)}\`, got \`${JSON.stringify(value)}\``, | ||
| validator: value => deepStrictEqual(value, expected) |
There was a problem hiding this comment.
Used deepStrictEqual. Should we use deepEqual instead?
There was a problem hiding this comment.
Let's use https://www.npmjs.com/package/lodash.isequal instead. It's better.
| * | ||
| * @param predicate The predicate that should be applied against every individual item. | ||
| */ | ||
| ofType<T>(predicate: Predicate<T>) { |
There was a problem hiding this comment.
Not sure if the implementation is correct.
ofType(type, …)(Accepts anyis.xtype) (TODO: Would be useful if this could also support any ow predicate for each element, so you could do:ow.array.ofType(ow.string.minLength(5)))
I think I implemented what you described as TODO. Not sure what the initial goal was of ofType because it looks like it accepts multiple arguments.
There was a problem hiding this comment.
This is exactly what I had in mind :)
| @@ -0,0 +1,69 @@ | |||
| import test from 'ava'; | |||
| import * as m from '..'; | |||
There was a problem hiding this comment.
Why are we doing * as m? Shouldn't ow be a default export?
There was a problem hiding this comment.
Because of interoperability with CommonJS. Otherwise, CJS import should be done as const ow = require('ow').default. I didn't find a way yet in TS to support both.
There was a problem hiding this comment.
There was a problem hiding this comment.
The thing is that the type definition marks it as default export. Meaning, if you would write the following JavaScript
const ow = require('ow');Only ow.default will show up as intellisense, because it is a default export. Currently, webpack just exports this as module.exports, even when you would mark it as export default, so that's not really a problem. The problem lies in the type definitions.
There was a problem hiding this comment.
I didn't realize the type definitions worked even with JS, but I think it's an ok compromise (to do default export). This way it's correct for TS and ES2015 modules, and for CommonJS it still works, but Intellisense outputs the incorrect thing. Better to optimize for the future. Thoughts?
source/lib/predicates/array.ts
Outdated
| @@ -1,4 +1,4 @@ | |||
| import * as deepStrictEqual from 'deep-strict-equal'; | |||
| import * as isEqual from 'lodash.isequal'; | |||
There was a problem hiding this comment.
Isn't the point of
Line 20 in 89b49e7
module.export?
There was a problem hiding this comment.
Jup, you're right!
There was a problem hiding this comment.
I'll use the import isEqual right here and will change all the others in a different PR.
There was a problem hiding this comment.
Apparently it doesn't. I thought it was, but it's not working. It tries to access isEqual.default... Have to investigate this further.
There was a problem hiding this comment.
Apparently, allowSyntheticDefaultImports doesn't change the code that is being emitted, hence the description.
Allow default imports from modules with no default export. This does not affect code emit, just typechecking.
More info: microsoft/TypeScript#7518. So actually, we can remove that option is it has no use (for now).
There was a problem hiding this comment.
I see... Let's remove it then.
|
Removed the flag. Something else I need to cover? |
|
|
This PR is work in progress. Feedback in the meantime is always welcome.