fix: add typings for event stream and fix bugs that surfaced #5290
Conversation
domoritz
force-pushed
the
dom/events-stream-typings
branch
from
00c4626 to
3cf59c5
Compare
domoritz
changed the title
domoritz
changed the title
domoritz
force-pushed
the
dom/events-stream-typings
branch
from
3cf59c5 to
acce897
Compare
| if (filters.indexOf(filterExpr) < 0) { | ||
| filters.push(filterExpr); | ||
| } | ||
| return on; |
There was a problem hiding this comment.
I had to add this to make the typings correct. Does JavaScript magically return the first argument here or does reduce have some other magic behavior?
There was a problem hiding this comment.
No, nothing magic. Just a (somewhat lazy) re-appropriation of a reduce when a forEach would suffice (reusing the method to be reduce duplication of error checks for evt.between).
| @@ -13,7 +13,7 @@ const clear: TransformCompiler = { | |||
|
|
|||
| parse: (model, selDef, selCmpt) => { | |||
| if (selDef.clear) { | |||
| selCmpt.clear = parseSelector(selDef.clear, 'scope'); | |||
| selCmpt.clear = isString(selDef.clear) ? parseSelector(selDef.clear, 'scope') : selDef.clear; | |||
There was a problem hiding this comment.
The check for string was missing here.
| events(selCmpt, (_: OnEvent[], evt: EventStream) => { | ||
| const filters = evt.between[0].filter || (evt.between[0].filter = []); | ||
| events(selCmpt, (on: OnEvent[], evt: Stream) => { | ||
| const filters = array(evt.between[0].filter || (evt.between[0].filter = [])); |
There was a problem hiding this comment.
We previously assumed that filter is an array. Was this assumption correct?
There was a problem hiding this comment.
Yes, once filters have been parsed by vega-event-selector, this property will always be an array.
domoritz
force-pushed
the
dom/events-stream-typings
branch
from
acce897 to
63f3f1e
Compare
domoritz
changed the title
domoritz
force-pushed
the
dom/events-stream-typings
branch
from
63f3f1e to
423811c
Compare
|
Blocked by vega/ts-json-schema-generator#192 |
domoritz
force-pushed
the
dom/events-stream-typings
branch
from
423811c to
27a59f0
Compare
|
Instead of blocking on vega/ts-json-schema-generator#192, I sent vega/vega#1926. |
domoritz
force-pushed
the
dom/events-stream-typings
branch
from
d2e8c5b to
ccd8209
Compare
| events(selCmpt, (_: OnEvent[], evt: EventStream) => { | ||
| const filters = evt.between[0].filter || (evt.between[0].filter = []); | ||
| events(selCmpt, (on: OnEvent[], evt: Stream) => { | ||
| const filters = array(evt.between[0].filter || (evt.between[0].filter = [])); |
There was a problem hiding this comment.
Yes, once filters have been parsed by vega-event-selector, this property will always be an array.
| if (filters.indexOf(filterExpr) < 0) { | ||
| filters.push(filterExpr); | ||
| } | ||
| return on; |
There was a problem hiding this comment.
No, nothing magic. Just a (somewhat lazy) re-appropriation of a reduce when a forEach would suffice (reusing the method to be reduce duplication of error checks for evt.between).
| @@ -13,7 +13,7 @@ const clear: TransformCompiler = { | |||
|
|
|||
| parse: (model, selDef, selCmpt) => { | |||
| if (selDef.clear) { | |||
| selCmpt.clear = parseSelector(selDef.clear, 'scope'); | |||
| selCmpt.clear = isString(selDef.clear) ? parseSelector(selDef.clear, 'scope') : selDef.clear; | |||
Fixes #5289