RLesur / crrri

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

Perform with Chrome #62

Closed RLesur closed 5 years ago

RLesur commented 5 years ago

Here is a first draft of a chrome_execute() function that takes as argument(s) one (or many) async function(s) of the form:

function(client) {
  # ...
}

It turns these async functions to a sync one and performs the following actions:

Here is a demo:

async_save_as_pdf <- function(url) {
  function(client) {
    Page <- client$Page

    Page$enable() %...>% {
      Page$navigate(url = url)
      Page$loadEventFired()
    } %...>% {
      Page$printToPDF()
    } %...>% {
      .$data %>%
        jsonlite::base64_dec() %>%
        writeBin(paste0(httr::parse_url(url)$hostname, ".pdf"))
    }
  }
}

save_as_pdf <- function(...) {
  list(...) %>%
    purrr::map(async_save_as_pdf) %>%
    chrome_execute(.list = .)
}

save_as_pdf("https://www.r-project.org/", "https://rstudio.com/")
RLesur commented 5 years ago

What do you think of that? I hesitate about the name of the function chrome_execute() Maybe perform_with_chrome() would be a better name?

RLesur commented 5 years ago

FMPOV, there is one remaining point. Here, all the funs are executed in serial order in the same browser and the same client/tab/target. It means that when an async function is executed, its execution context can be different if a previous async function has modified some parameters in the browser or runtime...

If the user wants to execute several async functions and needs to ensure that each of them runs in a new fresh browser, he has to apply chrome_execute() to each function. This is easy.

But here, maybe there is one uncovered need: running several functions in the same browser but each of them in a new tab/target.

I wonder whether we should provide an option to run each function in a new fresh target (in the same browser).

cderv commented 5 years ago

This is a good point. I don't think this is something difficult. I need to be finished with #64 and I'll see that.

RLesur commented 5 years ago

I think that this branch could be merged into master. As the chrome_execute() function would be one of the most important, I think we have to chose its name carefully. I am not good for naming things. What name do you prefer?

cderv commented 5 years ago

Among the possible name we have

this week end I have also thought about with_chrome() because this kind of function make me think about withr package, when you want to run some code in a specific environment. I find it to be like

It may be a long shot because we are not there yet but having a common prefix instead of a suffix maybe be easier.

execute_with_* or simply run_with_* could be at the same time clear on what it does and easy to remember. with_* could be to close to the withr 📦

But I also think we don't need to put too much pression on this for a merge request. Even if we present with a name and change afterward, it is ok. The deadline would be the first release. What is important is to handle change smoothly (with deprecation or warning about the change).

Those are my thoughts for now. 😉

RLesur commented 5 years ago

This is a very convincing approach. I like the execute_with_*, run_with_*, perform_with_* and with_* . As you indicate, with_* could be confusing.

I am indifferent about the verb. What do you prefer?

cderv commented 5 years ago

You're right : perform is a great verb. Not so used by other :package:, not so common, and I find it clear on what we are trying to achieve.

so perform_with_* (or perform_in_* 😉 ?) it is !