onRender() doesn't support expressions with a trailing comma

[cpsievert]

From the documentation for onRender():

The jsCode parameter must be a valid JavaScript expression that returns a function.

This works as expected

library(plotly)
library(htmlwidgets)

plot_ly() %>%
  onRender("(function (gd) { return console.log(gd); })")

But, if you have a (valid) expression with a trailing semicolon, it doesn't work:

library(plotly)
library(htmlwidgets)

plot_ly() %>%
  onRender("(function (gd) { return console.log(gd); });")

The main use case for supporting the trailing comma is to make it easier to take the output of something like reactR::babel_transform() and feed it into onRender()

[jcheng5]

First, s/comma/semicolon/g. 😄

Second, there's a difference in JavaScript between expressions and statements; and statements, not expressions, are terminated by semicolons.

http://2ality.com/2012/09/expressions-vs-statements.html

So I think it's confusing to require an expression, but if you give us a statement, we strip off the ; and treat it like an expression.

Third, I don't think you need the parens surrounding the function in the first place, and if you don't include them, doesn't this problem go away? (i.e. onRender("function (gd) { return console.log(gd); }"))

[gordonwoodhull]

I think the rule is that if function is the first thing on the line, it is a function declaration instead of a function expression. :frown:

[alandipert]

I think onRender is already confusing, because function (gd) { return console.log(gd); } is a valid expression only if it appears in expression context, but that context is missing until the supplied text is surrounded by parentheses within onRender before being passed to eval.

So, while the function claims to accept a JavaScript expression, really it accepts an invalid statement (a function declaration missing its name) that it transforms into a valid expression (by wrapping with parentheses) before eval-ing.

The contract that would make the most sense to me would be that of eval itself, which is extensively documented elsewhere, but adopting it would not be backward-compatible because instances of function (gd) { return console.log(gd); } in user code would need to be converted to (function (gd) { return console.log(gd); }) to work.

IIRC, one backward-compatible approach suggested by @cpsievert when we last discussed it is something like this:

onRender(evalJS("(function (gd) { return console.log(gd); });"))

where evalJS is a new function that returns a classed character vector understood by onRender to be passed straight to eval without the parentheses treatment.

[cpsievert]

Thanks for shedding light on the issue everyone. Regardless of what happens here I think the documentation for onRender() (and JS()) could mention these subtle points.

To address your question @jcheng5:

Third, I don't think you need the parens surrounding the function in the first place, and if you don't include them, doesn't this problem go away?

Yes, but the point is to support arrow functions and other ES6 features via reactR::babel_transform() (or similar)

plotly::plot_ly() %>%
    htmlwidgets::onRender(reactR::babel_transform("gd => console.log(gd)"))

Problem is, babel transforms the arrow function into an ES5 statement.

(function (gd) {  return console.log(gd); });

It's worth noting that JS() seems to suffer from the same issue, as it also wraps it's input in parentheses before evaluating

https://github.com/ramnathv/htmlwidgets/blob/5db27ada3895b8cf2ba6ee1dd9ebfa5201729e5b/inst/www/htmlwidgets.js#L735

It seems the best way to proceed is to provide a user-facing option to eval() the JS verbatim in both onRender() and JS(), maybe as an argument, for example:

plotly::plot_ly() %>%
    htmlwidgets::onRender(reactR::babel_transform("gd => console.log(gd)"), evalVerbatim = TRUE)