feat: Add typeArray and allow passing equal function to bindableDerived #175
Conversation
Codecov Report
@@ Coverage Diff @@
## main #175 +/- ##
==========================================
+ Coverage 89.11% 89.17% +0.05%
==========================================
Files 61 61
Lines 1488 1496 +8
Branches 232 234 +2
==========================================
+ Hits 1326 1334 +8
Misses 125 125
Partials 37 37
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
MarkoOleksiyenko
force-pushed
the
bindable
branch
from
8d8d246
to
6be18f9
Compare
core/lib/services/checks.ts
Outdated
| * @param value the value to check | ||
| * @returns true if the value is an array | ||
| */ | ||
| export function isArray(value: any) { |
There was a problem hiding this comment.
| export function isArray(value: any) { | |
| export function isArray(value: any): value is any[] { |
(cf https://www.typescriptlang.org/docs/handbook/2/narrowing.html#using-type-predicates)
core/lib/services/checks.ts
Outdated
| * @param value the value to check | ||
| * @returns true if the value is an array | ||
| */ | ||
| export function isArray(value: any) { |
There was a problem hiding this comment.
Or maybe we should do:
export const isArray = Array.isArray;There was a problem hiding this comment.
this seems more concise!
MarkoOleksiyenko
force-pushed
the
bindable
branch
from
6be18f9
to
b3b5f65
Compare
MarkoOleksiyenko
force-pushed
the
bindable
branch
5 times, most recently
from
c7cdfd2
to
a3b2ebb
Compare
MarkoOleksiyenko
force-pushed
the
bindable
branch
from
a3b2ebb
to
d6c2f01
Compare
core/lib/services/writables.spec.ts
Outdated
| const testArray = [15, 20]; | ||
| store$.set(testArray); | ||
|
|
||
| expect(store$()).toStrictEqual([15, 20]); |
There was a problem hiding this comment.
| expect(store$()).toStrictEqual([15, 20]); | |
| expect(store$()).toBe(testArray); |
core/lib/services/writables.spec.ts
Outdated
| expect(typeArray.equal(arr, arr2)).toBe(true); | ||
| expect(typeArray.equal([15], [15, 15])).toBe(false); | ||
| expect(typeArray.equal([15, 15], [15])).toBe(false); | ||
| expect(typeArray.equal(undefined, [15])).toBe(false); |
There was a problem hiding this comment.
undefined is not part of the accepted types for this equal function.
You should do undefined as any if you still want to test this case.
There was a problem hiding this comment.
true, will type the method params
core/lib/services/writables.spec.ts
Outdated
|
|
||
| expect(store$()).toStrictEqual([15, 20]); | ||
|
|
||
| testArray[1] = 15; |
There was a problem hiding this comment.
I am not really in favor of using stores this way. I prefer immutability, otherwise we need equal functions that consider objects as always different (even when they are the same reference). I would remove this part of the test.
MarkoOleksiyenko
force-pushed
the
bindable
branch
from
d6c2f01
to
85d54d1
Compare
core/lib/services/writables.ts
Outdated
|
|
||
| export const typeArray: WritableWithDefaultOptions<any[]> = { | ||
| normalizeValue: testToNormalizeValue(isArray), | ||
| equal: (a: any[], b: any[]) => { |
There was a problem hiding this comment.
| equal: (a: any[], b: any[]) => { | |
| equal: (a, b) => { |
(the type of equal is already implied by WritableWithDefaultOptions<any[]> line 27)
There was a problem hiding this comment.
it is, however, the IDE highlights it in red saying that it as implicit type
MarkoOleksiyenko
force-pushed
the
bindable
branch
from
85d54d1
to
b7ca1aa
Compare
No description provided.