♻ PoC navigators with set #294
Conversation
Codecov Report
@@ Coverage Diff @@
## dev #294 +/- ##
========================================
- Coverage 100% 99.15% -0.85%
========================================
Files 97 105 +8
Lines 285 356 +71
========================================
+ Hits 285 353 +68
- Misses 0 3 +3
Continue to review full report at Codecov.
|
|
🎉 |
|
@frinyvonnick |
|
|
||
| export class ArrayNav extends BaseNav { | ||
| get length() { | ||
| if (this._length === undefined) this._length = length(this.value) |
There was a problem hiding this comment.
Is this optimization really efficient ?
There was a problem hiding this comment.
I think you're right, I'll remove this
| copy() { | ||
| const { value } = this | ||
| if (isNil(value)) return [] | ||
| return Array.isArray(value) ? [...value] : { ...value } |
There was a problem hiding this comment.
Why are you testing if this is an Array whereas we are in ArrayNav ?
There was a problem hiding this comment.
Because users might use slice or array index notation on an object, this is to avoid replacing the object by an array.
This conforms to normal JavaScript behavior:
const obj = {}
obj[0] = 'val'
console.log(obj) // Outputs: {0: "val"}| class IndexNav extends ArrayNav { | ||
| constructor(value, index, next) { | ||
| super(value, next) | ||
| this._index = index |
There was a problem hiding this comment.
Maybe we could use private syntax https://github.com/tc39/proposal-private-methods#private-methods-and-fields ?
There was a problem hiding this comment.
Yes this is appropriate, let's close our eyes on the ugly # and try this
There was a problem hiding this comment.
I can't find the babel plugin corresponding to the proposal.
I think it's time to rewrite in TS...
There was a problem hiding this comment.
OK it seems to be in babel 7 beta 48, it might be risky to upgrade to babel 7 but I'll give it a try...
There was a problem hiding this comment.
|
|
||
| if (isNil(value)) return [] | ||
|
|
||
| return Array.from(range, index => _next(value[index]).get()) |
|
I'll try upgrading to babel 7 and using private fields in another PR. |
* 🚧 PoC nav * 🚧 Add index navigator * 🚧 Add slice navigator * ♻️ Reorganize code * ♻️ Move apart get and update logic and use ES6 classes * 🔥 Remove unused currification level * ⚡ Use nav in set and validate performance improvement with benchmark * ✨ Put back negative array index support * ✅ Report apply tests to nav * 🚚 Move each navigator in its own file as suggested by @frinyvonnick * :refactor: Optional currying on set * ✨ Add all props navigator * ✨ Prop list navigator * ♻️ Put TypeError for empty path in nav * 👌 @frinyvonnick's review
No description provided.