RLesur / crrri

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

[Idea] navigateToFile #51

Open ColinFay opened 5 years ago

ColinFay commented 5 years ago

Right now the Page object allows to navigate to a url, and if you want to navigate to a file you have to Page$navigate(url = sprintf("file://%s", normalizePath(file_path)) ).

It could be nice to have a $navigateToFile method that takes a relative path, normalize it, and dooes the sprintf("file://%s").

ColinFay commented 5 years ago

Example :

library(promises)
library(crrri)

chrome <- Chrome$new("/Applications/Google Chrome.app/Contents/MacOS/Google Chrome", headless = FALSE) 

plop <- "<h1> pouet </h1>"
tmp <- tempfile(fileext = ".html")
write(plop, tmp)

chrome$connect() %...>% 
  (function(client) {
    Page <- client$Page
    #browser()
    Page$enable() %...>% { # await enablement of the Page domain
      Page$navigate(url = sprintf("file://%s", tmp) )
      Page$loadEventFired() # await the load event
    } %...>% {
      client
    }
  }) %...!% {
    # handle errors
    cat("An error has occured:", .$message, "\n")
  }
cderv commented 5 years ago

Nice idea for a helper function !

@RLesur This keeps on bringing the question of this kind of function being inside this 📦 or in the other one (still in conception in-our-mind though...). What still bother me is that I think that crrri should only have methods related to chrome remote interface or only internal helpers and that any other methods seen as user's helpers should live in another object completing the one in crrri.

This mean I don't think adding a navigateToFile inside Page Domain R6 object makes sense here as it is not a method of the Page Domain from the protocol. All the more because we generate the Domain method directly from the protocol. Do you share this thought ?

However, we may be able to find how to add those in a way that makes it clear in the code (for us and other) what is from the Page domain and what is not (i.e completing it).

No matter what we decide here, we should consider, now the API is more stable, how and where we start adding such helper.

What are your thoughts on that ?

RLesur commented 5 years ago

My views are the following:

In order to provide an example, here is a sketch of the puppeteer's page.goto() method

First, the user's view (I think this is very close to @ColinFay's proposal):

chrome <- Chrome$new()

plop <- "<h1> pouet </h1>"
tmp <- tempfile(fileext = ".html")
write(plop, tmp)

client <- marionette(chrome)

client %...>% (function(client) {
  page <- client$page
  page$goto(tmp)
})

# confirm that we got a beautiful Pouet in Chrome
chrome$view()

Now, the whole script. I've adopted here the same structure as in crrri:

library(crrri)

# Domains for Marionette
MarionetteDomain <- R6::R6Class(
  "MarionetteDomain",
  public = list(
    initialize = function(marionette) {
      private$.marionette <- marionette
    }
  ),
  private = list(
    .marionette = NULL
  )
)

# page domain (as in puppeteer)
page <- R6::R6Class(
  "page",
  inherit = MarionetteDomain,
  public = list(
    # see puppeteer page.goto method
    goto = function(url, waitUntil = c("load", "domcontentloaded"), timeout = 30) {
      url <- httr::parse_url(url)
      if(is.null(url$scheme)) {
        url$scheme <- "file"
      }
      url <- httr::build_url(url)
      Page <- private$.marionette$.__client__$Page
      waitUntil <- match.arg(waitUntil)
      ready <- switch(waitUntil,
                      load = Page$loadEventFired(),
                      domcontentloaded = Page$domContentEventFired())

      Page$enable() %...>% {
        Page$navigate(url = url)
        ready
      } %>%
        timeout(timeout)
    }
  )
)

# Marionette R6 class
Marionette <- R6::R6Class( 
  "Marionette",
  public = list(
    initialize = function(client) {
      self$.__client__ <- client
      self$page <- page$new(self)
    },
    page = NULL,
    disconnect = function() {
      self$.__client__$disconnect()
    },
    .__client__ = NULL
  )
)

# use a constructor
marionette <- function(chrome) {
  chrome$connect() %...>% (function(client) {
    Marionette$new(client)
  })
}

# let's play with marionette
chrome <- Chrome$new()

plop <- "<h1> pouet </h1>"
tmp <- tempfile(fileext = ".html")
write(plop, tmp)

client <- marionette(chrome)

client %...>% (function(client) {
  page <- client$page
  page$goto(tmp)
})

# confirm that we got a beautiful Pouet in Chrome
chrome$view()
cderv commented 5 years ago

Ok so we are in sync. 🎉

I digged into puppeteer back in december when I worked on the first version of eventemitter and CDPSession, so I exactly see your point and I share this 100%. This is also close to the idea (I can't find where I put it...) of registering known event from the protocol, as in puppeteer.

We should then bring to live our other 📦 idea to begin filling this in there. This issue belongs there.

cderv commented 5 years ago

@RLesur how to do we proceed on this one ? Should we create another recipes package ? @ColinFay is this idea of yours part of your project 📦 for shiny helpers based on crrri ?

RLesur commented 5 years ago

I still think that recipes should be in another package. I have no time yet to initiate it, do not hesitate to start!