Refactor Event #36
Refactor Event #36
Conversation
| let subscribe = (evt: t('a), f: cb('a)) => { | ||
| evt := List.append(evt^, [f]); | ||
| let unsubscribe = () => { | ||
| evt := List.filter(f => f !== f, evt^); |
There was a problem hiding this comment.
This was a bug. It didn't remove one callback, but cleared the list entirely f !== f is always false. That's one argument against shadowing.
| unsubscribe; | ||
| }; | ||
|
|
||
| let dispatch = (evt: t('a), v: 'a) => List.iter(c => c(v), evt^); | ||
| let dispatch = (event, message) => List.iter(cb => cb(message), event^); |
There was a problem hiding this comment.
This module doesn't actually implement an "event" but rather an "event delegator". And the sloppy naming causes an issue here if you try to name the arguments properly, because what's passed in to be dispatched is what's usually considered the actual event. But since that name is taken I had to go with message here. Which isn't terrible, but also not all that accurate.
| @@ -31,7 +31,7 @@ describe("ChildProcess", ({describe, _}) => { | |||
| let proc = ChildProcess.spawn("node", [|"-e", script|]); | |||
|
|
|||
| let data = ref(""); | |||
| let _event = | |||
There was a problem hiding this comment.
This was just plain wrong, and is another good reason why you should add proper type annotations to ignored values, as a sanity check for yourself. That is what a static type system is for, after all.
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Mac CI seems broken :/ |
Cleans it up and ensures it works on 4.09