micro-analytics / micro-analytics-cli

Public analytics as a Node.js microservice. No sysadmin experience required! 📈
MIT License
734 stars 39 forks source link

Add realtime events with SSE #41

Closed relekang closed 7 years ago

relekang commented 7 years ago

This is very much work in progress, but wanted to open a pr so that we can get this right.

Relates to #16


How to use this

const sse = new EventSource('/sse')
sse.onopen = function () { console.log('sse open') }
sse.onerror = function (error) { console.error('[sse error]', error) }
sse.addEventlistener('new-event', function (e) { console.log(e) })

Rendered version of updated readme

mxstbr commented 7 years ago

Awesome, can't wait to see this! Reminds me I still need to figure out how to get #31 to work…

relekang commented 7 years ago

Unfortunately I haven't had the time to put into this lately, will hopefully have more time next week.

mxstbr commented 7 years ago

How are we looking with this PR? Would love to land this!

relekang commented 7 years ago

@mxstbr I am working through the last of the issues now. Have any inputs on the event name at the moment it is new-event.

codecov-io commented 7 years ago

Codecov Report

Merging #41 into master will decrease coverage by 12.5%. The diff coverage is 66.66%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master      #41       +/-   ##
===========================================
- Coverage   83.63%   71.13%   -12.51%     
===========================================
  Files           3        6        +3     
  Lines          55       97       +42     
  Branches       13       20        +7     
===========================================
+ Hits           46       69       +23     
- Misses          9       28       +19
Impacted Files Coverage Δ
src/parseArgs.js 100% <100%> (ø)
src/index.js 7.69% <7.69%> (-78.03%) :arrow_down:
src/handler.js 72.97% <72.97%> (ø)
src/db.js 83.33% <81.25%> (+13.33%) :arrow_up:
src/sse.js 83.33% <83.33%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update c6b1c44...5e2ab85. Read the comment docs.

relekang commented 7 years ago

Just need to look into some tests for the sse parts now. Will try to do it tomorrow.

mxstbr commented 7 years ago

Hmm, in terms of names, do they need to be unique? How about micro-analytics-ping?

relekang commented 7 years ago

I think this is ready for review, one of the tests will fail until we release and upgrade adapter-flat-file-db. It needs subscribe from micro-analytics/adapter-flat-file-db#2

relekang commented 7 years ago

Hmm, in terms of names, do they need to be unique? How about micro-analytics-ping?

No, the is just what is passed to the event listener:

sse.addEventlistener('micro-analytics-ping', function (event) { console.log(event) })

The only thing I think we need to consider about the name is that it should describe what you get in the data field of the event passed to the callback of the listener. I think micro-analytics-ping works, will change to use that.

relekang commented 7 years ago

@mxstbr I think this is ready, it only needs a release of flat-file-adapter to make the tests pass. Could you do a release?

mxstbr commented 7 years ago

I did a npm owner add relekang so you should be able to publish it!

relekang commented 7 years ago

@mxstbr It is green now, the decrease in coverage is mostly because of the manuall creation of the server and custom cli. I guess we could ship this?

mxstbr commented 7 years ago

Is this ready to go?

relekang commented 7 years ago

Yes ✌️

mxstbr commented 7 years ago

Merging this tomorrow with a big PR push around the release 👌 Great work man!