Skip to content

Conversation

@alandipert
Copy link
Collaborator

@alandipert alandipert commented Dec 18, 2018

Rationale

reactR is a great effort to close the gap between the React and R ecosystems and proposes a way to use JSX without installing any JavaScript build tools, which greatly empowers a certain category of users.

However, another category of "power" users — those who wish to consume JavaScript libraries from npm, or write their own components with the assistance of JavaScript tools and libraries — are not as well supported.

In lieu of tooling and documentation in reactR for this user category, each developer must create their own patterns for interfacing Shiny and React. They must also select and configure their JavaScript build tools manually.

The lower we make the bar for writing new widgets and inputs based on React, the more widget and input authors we can create. The more authors, the more widgets and inputs. The more widgets, the more empowered the larger R community of widget and input consumers becomes.

Objectives

  1. Prescribe a convention for building the JavaScript parts of HTMLWidgets and Shiny outputs with yarn and webpack
    • Codify the convention by baking it into a scaffold function in the reactR package
  2. Define an R interface for constructing React component trees in R such that they can be rendered in the browser. Currently that interface consists of:
    • reactR::component: Analogous to htmltools::tag
    • reactR::React: Analogous to htmltools::tags
  3. Define a JavaScript interface for making React components available to R/Shiny from the JavaScript side. Currently that interface consists of:
    • reactR.hydrate: Recursively converts a JSON-encoded React component tree as generated on the R side into a Reactive element. Used directly by Shiny inputs and outputs.
    • reactR.reactWidget: Accepts a set of React components to make accessible within the context of an HTMLWidget, using hydrate internally to render components.
  4. Deliver documentation and examples demonstrating use of these tools.

Tasks

  • Finalize the design and implementation of the R and JavaScript interfaces
  • Write unit tests for new R and JavaScript
  • Complete and fully document the react-sparklines htmlwidget example
    • Pass the dependencies to the createWidget(deps = ...) argument instead of adding them to sparklineswidget_html.
  • Complete and fully document the react-color Shiny input example Deferred
  • Write a React-based htmlwidget scaffold function
  • Write a React-based Shiny input scaffold function Deferred
  • Write one or more articles on https://timelyportfolio.github.io/reactR/ introducing the new APIs and demonstrating their use in a tutorial style.

Open Questions

  1. What's better, React$SomeComponent(x = 1) or component("SomeComponent", x = 1)?
    • Joe proposed React in the style of htmltools::tags, but Winston suggested a function like component, with the argument that HTML tags are closed but React component names are open. So, it's not like they can be auto-completed. I personally lean toward a function, but maybe we should just have both? (similar to htmltools::tag + htmltools::tags)
  2. Should reactR::component in reacttools.R take a varArgs list just like htmltools::tag?
    • Thinking: yes
  3. Should we use webpack instead of browserify? It might have a more elegant way to depend indirectly on react and react-dom (currently we use a hack in the Makefile to get browserify to do this)
    • Yes, webpack reduces the amount of configuration, takes make out of the equation, and gives us --watch

@timelyportfolio
Copy link
Collaborator

timelyportfolio commented Dec 19, 2018

@alandipert playing with the pull, and I picked the magical react-spring as a splashy example. The example widget repo is here. I don't think this is a typical use case, but it will likely poke holes in some of our thinking.

  1. The pattern of parent/root component in react-sparklines is not always the case. I thought we might be able to resolve with Fragment shown in lines js and R. This seems to work and could provide a solution, but should be documented as a pattern.
  2. The first Spring example demonstrates something with props as a function that I am not sure we currently accommodate.
<Spring from={{ opacity: 0 }} to={{ opacity: 1 }}>
  {props => <div style={props}>hello</div>}
</Spring>
  1. Also in the same react-spring example we run across a div that is not handled since it is not in components. I remember us quickly talking about this case with svg.
    image

I'd love to hear your thoughts and please feel free to tell me if this is expanding beyond expected scope. Thanks!

@alandipert
Copy link
Collaborator Author

@timelyportfolio very cool experiment!

  1. Your Fragment technique is awesome. I think we should definitely document that.
  2. We don't have an answer for this right now. I'm not sure what the best way to handle it is, but I'll think about it and see what Joe thinks and report back.
  3. I decided at one point to raise that error if a non-component appeared in markup, but I think it's better to allow it. I propose we bake in HTML5 and svg whitelists and check for the tag in those too, to at least help people find typos.

None of this feels out of scope to me, I'm very grateful for your experience report! You've hit on some really important/interesting things.

@timelyportfolio
Copy link
Collaborator

timelyportfolio commented Dec 19, 2018

@alandipert regarding html elements such as div we have a clue in that React.createElement expects a string ‘div’ rather than an unquoted component. Sorry that I won’t be on computer until tonight but could we simply double quote html elements or if not upper case and not found in components then infer that it is an html element. I don’t think we need to whitelist because I believe React.dom handles this.

@alandipert
Copy link
Collaborator Author

@timelyportfolio Ah yes, good point about React checking validity for us 👍

@timelyportfolio
Copy link
Collaborator

@alandipert I just remembered this ancient idea rstudio/htmltools#72 of using noquote for functions and objects. I don't think we need for objects, but this might help us solve the problem of functions.

@alandipert
Copy link
Collaborator Author

alandipert commented Dec 20, 2018

@timelyportfolio awesome, yes I thought of a similar scheme in the shower today! I was thinking something like a function, reactR::evalJS, for conveying snippets of JS code to the client. The return value of reactR::evalJS could appear as an attribute value or as a child in a reactR::component tree. It would get sent to the client as some specially-shaped object that hydrate would know to window.eval.

I think we can possibly do a couple extra cool things:

  1. Give the string of JavaScript the ES6/JSX treatment using standalone Babel
  2. Handle value splicing so that users don't need to worry about escaping: evalJS('{}.forEach(i => console.log(i))', c(1,2,3))

I'm aware of another related thing, htmlwidgets::JS. I don't know if it's for the kind of thing we imagine, or that it would be a good idea to lean on a function from htmlwidgets.

@alandipert
Copy link
Collaborator Author

@timelyportfolio I took a stab at evalJS and I think the Spring example kinda works:

  output$spring <- renderSpringwidget(
    springwidget(
      data = NULL,
      spring(
        from = list(opacity = 0),
        to = list(opacity = 1),
        reactR::evalJS("props => <h1 style={props}>hello</h1>")
      )
    )
  )

The h1 fades in but doesn't fade back out, which I thought it was supposed to do. I don't know if I misunderstand what it should do or if there's a problem with evalJS.

@timelyportfolio
Copy link
Collaborator

@alandipert the loop on the react-spring site is the result of an additional rewind component that is not part of react-spring. The effect should only happen once as you describe. I played around yesterday on npm download charts and was surprised to see recharts as the top download. Traveling today and tomorrow so won’t be able to do much but will get back on it once settled. Guessing there might be security concerns with eval but that is an area I know little about. Might want to check with some RStudio folks on the security aspect.

@alandipert
Copy link
Collaborator Author

@timelyportfolio after consulting with @jcheng5 we decided to hold off on evalJS and focus instead on refining/documenting what we have, at least until after RStudio conf.

Thanks for the pointer to recharts -- it looks very amenable to our approach, I'll play with it too.

Safe travels, looking forward to collaborating with you further when you're back.

@timelyportfolio
Copy link
Collaborator

@alandilpert ok, understood. I think we should discuss this in the Readme or documentation, so users also understand the decision and also the potential for addressing later. I'll pivot my example use cases to those that don't require this functionality.

@timelyportfolio
Copy link
Collaborator

@alandipert @jcheng5 I started another widget roffice as an alternate example. As part of the testing, I built a proof of concept for adding Shiny event integration in enhancements-shiny. Have you had any discussions about this?

Here is how this integration would look. A user would add a shiny prop providing the name of the event prop.

#devtools::install_github("timelyportfolio/reactR@enhancements")
#devtools::install_github("timelyportfolio/roffice")

library(reactR)
library(roffice)
library(shiny)

#options(shiny.trace = TRUE)
ui <- officeuiwidgetOutput("me")

server <- function(input, output, session) {
  # slider with just one prop and some Shiny
  output$me <- renderOfficeuiwidget({
    officeuiwidget(
      reactR::React$Slider(label = "Basic Slider", shiny = "onChange")
    )
  })
}

shinyApp(ui, server)

@alandipert
Copy link
Collaborator Author

@timelyportfolio that's an interesting approach. I like it!

Compared to evalJS, the primary advantage seems to be that custom JS is unnecessary for most cases when a React callback should be converted into a Shiny input value. Is that your understanding also? Seems great.

We haven't discussed beyond the evalJS idea, but in general we have concluded it's best to lean on Shiny.onInputChange as directly as possible. I think your approach is nicely aligned with that philosophy, and doesn't drag any other ideas in the way evalJS did.

@alandipert
Copy link
Collaborator Author

@timelyportfolio I had one thought, that perhaps shinyEvent would be a better name for the proposed special attribute.

@timelyportfolio
Copy link
Collaborator

timelyportfolio commented Dec 31, 2018

@alandipert shinyEvent probably is a bit more descriptive. Just thought it would be a safe means of accomplishing communication especially considering very short timeline. If acceptable, how would you like to incorporate? Pull request to your enhancements branch?

One other question is how to best id the event. In addition to shinyEvent we could also consider shinyId as another prop that when undefined could stick with current behavior. Or a user could specify id attribute that we could use.

@alandipert
Copy link
Collaborator Author

alandipert commented Dec 31, 2018

@timelyportfolio Feel free to make a commit to the enhancements branch. I'll remove evalJS today or tomorrow.

I'm in favor of incorporating this ASAP barring an objection by @jcheng5. I think it gives us a very nice "get values back to R" story and is a simple thing to document and maintain.

Thanks for coming up with it!

R/reacttools.R Outdated
reactMarkup <- function(tag) {
stopifnot(class(tag) == "shiny.tag"
|| (is.character(tag) && length(tag) == 1))
list(tag = tag, class = "reactR.markup")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

inherits(tag, "shiny.tag") seems more appropriate here. Also, what about "shiny.tag.list"? Perhaps you could allow a lit then wrap it in a div?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great point about tag lists. There's actually an analogous React thing we could support that they could turn into, React.Fragment. But I think we should maybe add that in a future PR.

stop("Component name must be specified and start with an upper case character")
}
htmltools::tag(name, varArgs)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might want to add a class or a subclass here? As a package author, I might want to know if a user is supplying a component versus a tag

@timelyportfolio timelyportfolio merged commit f03c135 into master Jan 12, 2019
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

Successfully merging this pull request may close these issues.

5 participants