search

RLesur / crrri

A Chrome Remote Interface written in R
https://rlesur.github.io/crrri/
Other
157 stars 12 forks source link

Feature/cdp session with eventemitter #38

Closed RLesur closed 5 years ago

RLesur commented 5 years ago

I open this PR now in order to have a room for discussion for this branch.

RLesur commented 5 years ago

I am really embarrassed by https://github.com/RLesur/crrri/blob/224df2f68b84ea8aae231078eba658f88f26af63/R/DevToolsConnexion.R#L27-L32

There are two kinds of messages sent by Chrome:

With this implementation, the EventEmitter object throws the same kind of events for Chrome events and Chrome responses: by listening the EventEmitter object, we cannot distinguish Chrome events and Chrome responses . I understand your goal but I fear that could lead to some errors.

I would prefer to keep the distinction between Chrome events and Chrome responses: I will propose some changes in this direction.

cderv commented 5 years ago

It was my understanding that only commands have id. These lines are just emitting the event corresponding to the command sent. Lines after are responding the same way but to method. What difference would like to have between those two?

RLesur commented 5 years ago

What do you think of that? I used many events from the chrome-remote-interface library: connect, ready... I also created new events that would be useful to implement the promises API on top of CDPSession.

cderv commented 5 years ago

Definitely looks good ! It is fine by me. Should we merge and see how it goes from there with example and test run ?