Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Idea] navigateToFile #51

Open
ColinFay opened this issue Apr 26, 2019 · 6 comments
Open

[Idea] navigateToFile #51

ColinFay opened this issue Apr 26, 2019 · 6 comments

Comments

@ColinFay
Copy link
Contributor

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
Copy link
Contributor Author

ColinFay commented Apr 26, 2019

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
Copy link
Collaborator

cderv commented Apr 27, 2019

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
Copy link
Owner

RLesur commented Apr 27, 2019

My views are the following:

  • crrri should be close to the protocol
  • we should not modify domains. The main reason is that the protocol is retrieved by default from the remote Chrome
  • crrri is designed to be reused

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
Copy link
Collaborator

cderv commented Apr 27, 2019

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
Copy link
Collaborator

cderv commented Oct 8, 2019

@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
Copy link
Owner

RLesur commented Oct 8, 2019

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants