watch is a stream #121
watch is a stream #121
Conversation
| this.range = range | ||
| this.current = null | ||
| this.previous = null | ||
| this.map = opts.map || null |
There was a problem hiding this comment.
map and unmap are already set in the superclass. Would either handle completely there, or completely in the subclasses.
KeyWatchStream only seems to have map and no unmap, and no concept of _mapped either, so probably makessense to keep in subclass
There was a problem hiding this comment.
The opened logic could live fully in the superclass btw, and the subclasses just need to overwrite _setup--now it lives in both (sueprclass still has this.opened)
| this._differ = opts.differ || defaultDiffer | ||
| this._checkout = opts.checkout || (opts.eager ? 1 : 0) | ||
|
|
||
| this.opened = this._setup() |
There was a problem hiding this comment.
The opening flow is a bit confusing compared with how we do it elsewhere, normally this.opened is a bool (in ready-resource). Not sure what the standard for streams is though
How about calling it this.opening and exposing an async ready() { return this.opening } method? That way the api is compat with ready resource
There was a problem hiding this comment.
we can it opened on other streams in the stack so named it this for consistency. also you shouldn’t really use this
There was a problem hiding this comment.
In that case it makes sense to keep the stream naming. Do note that await watcher.opened is documented in the readme
There was a problem hiding this comment.
ah ok, we should kill that. the stream already emits open when its open, so its mostly a stream concern anywho
| }) | ||
|
|
||
| if (!(await this._didRangeChange())) return null | ||
|
|
There was a problem hiding this comment.
Sanity check: no need for the if (this.destroying) return null check here? (not sure if _didRangeChange can return true with the stream being destroyed before this is code is reached)
There was a problem hiding this comment.
Good idea! Will add
Some changes to the experimental watch api