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

DiagrammeR - Multiple calls in RShiny fails #51

Closed
vbakella opened this issue Jan 25, 2015 · 40 comments
Closed

DiagrammeR - Multiple calls in RShiny fails #51

vbakella opened this issue Jan 25, 2015 · 40 comments

Comments

@vbakella
Copy link

Hi,

I am finding this package very useful and have been testing its using R Shiny v0.11 quite a bit and found a strange issue. This is using Mermaid with R Shiny v0.11.

I have taken the sample R Shiny program (at the end of the README file in https://github.com/rich-iannone/DiagrammeR) and modified it to make two calls to DiagrammeR.

The output contains the raw text showing the spec for the first diagram, and draws the visual for the second spec. In other words, only one call gets executed, while for the second call it simply dumps the markdown.

The code is given below.

library(shiny)

ui = shinyUI(fluidPage(
textInput('spec', 'Diagram Spec', value = ""),
DiagrammeROutput('diagram'),
DiagrammeROutput('diagram2')
))

server = function(input, output){
output$diagram <- renderDiagrammeR(DiagrammeR(
input$spec
))
output$diagram2 <- renderDiagrammeR(DiagrammeR(
input$spec
))

}

shinyApp(ui = ui, server = server)

My session Info

sessionInfo()
R version 3.1.2 (2014-10-31)
Platform: x86_64-pc-linux-gnu (64-bit)

locale:
[1] LC_CTYPE=en_US.UTF-8 LC_NUMERIC=C LC_TIME=C LC_COLLATE=C LC_MONETARY=C LC_MESSAGES=C LC_PAPER=C LC_NAME=C
[9] LC_ADDRESS=C LC_TELEPHONE=C LC_MEASUREMENT=C LC_IDENTIFICATION=C

attached base packages:
[1] stats graphics grDevices utils datasets methods base

other attached packages:
[1] shinyAce_0.2.0 whisker_0.3-2 shinythemes_1.0 DiagrammeR_0.3 dplyr_0.4.1 shiny_0.11 RMySQL_0.10.1 DBI_0.3.1

loaded via a namespace (and not attached):
[1] R6_2.0.1 RJSONIO_1.3-0 Rcpp_0.11.3 assertthat_0.1 digest_0.6.8 htmltools_0.2.6 htmlwidgets_0.3.2 httpuv_1.3.2 lazyeval_0.1.10
[10] magrittr_1.5 mime_0.2 parallel_3.1.2 rstudioapi_0.2 tools_3.1.2 xtable_1.7-4 yaml_2.1.13

@rich-iannone
Copy link
Owner

@vbakella thanks for using and testing the package! I'll try to reproduce here on my end and try to fix. If you should find some other clues as to why this is happening, let me know here.

@timelyportfolio
Copy link
Contributor

shiny is difficult with mermaid, but I think I have a working version in an experimental branch fix/mermaid-shiny. If possible, @rich-iannone and @vbakella, could you test?

devtools::install_github("timelyportfolio/DiagrammeR@fix/mermaid-shiny")

I have noticed it doesn't work well with RStudio Viewer, but worked well in Chrome.

If ok, I'll submit pull. Thanks.

@rich-iannone
Copy link
Owner

Going to test it now. Thanks!

@rich-iannone
Copy link
Owner

I seem to get 'parse error' each time. I believe I put in a valid mermaid diagram spec each time. It should work just with the graph definition ("graph LR").

shiny-test

@timelyportfolio
Copy link
Contributor

That is the issue I mentioned with RStudio Viewer. Can you try in the browser?

@rich-iannone
Copy link
Owner

I did try it in Safari (in screenshot). I'll also try in Firefox.

@timelyportfolio
Copy link
Contributor

Ok, I see it now. Maybe it only works with Chrome then. hmmm...

@rich-iannone
Copy link
Owner

Maybe. Same problem with latest Firefox on Mac.
diagrammer_shiny_firefox mac _test

@timelyportfolio
Copy link
Contributor

Well, it does need a ; or line break, so does graph LR; give you no error and a blank space?

@rich-iannone
Copy link
Owner

Pretty sure I exhausted all possibilities. I'll try again shortly with a ; and a space.

@timelyportfolio
Copy link
Contributor

ok, I think I see the problem. Let me play a bit.

@timelyportfolio
Copy link
Contributor

Does a non-shiny static mermaid work? If it is what I think it is, then it should not.

@rich-iannone
Copy link
Owner

I'll check that out once I get into the office. About 20 min.

Sent from my iPhone

On Jan 26, 2015, at 7:22 AM, timelyportfolio notifications@github.com wrote:

Does a non-shiny static mermaid work? If it is what I think it is, then it should not.


Reply to this email directly or view it on GitHub.

@rich-iannone
Copy link
Owner

Okay. Tested it out! It's fine with the space following the ;. Sorry about that.

@timelyportfolio
Copy link
Contributor

Will you try again with my newest push? I think it will be more robust. Still does not work in RStudio viewer though :(

devtools::install_github("timelyportfolio/DiagrammeR@fix/mermaid-shiny")

@rich-iannone
Copy link
Owner

I'm sorry but I just went to the field so I'm away from that test computer. Might have to do this by the end of the day.

Sent from my iPhone

On Jan 26, 2015, at 8:48 AM, timelyportfolio notifications@github.com wrote:

Will you try again with my newest push? I think it will be more robust. Still does not work in RStudio viewer though :(

devtools::install_github("timelyportfolio/DiagrammeR@fix/mermaid-shiny")

Reply to this email directly or view it on GitHub.

@vbakella
Copy link
Author

Hi,

I installed the fix mentioned above. I am able to generate Mermaid and GraphViz plots as per the examples. However, a Shiny program with a single DiagrammeR call is not working - gives a parser error.

Here is the code I tried:
library(shiny)
library(DiagrammeR)

ui = shinyUI(fluidPage(
textInput('spec', 'Diagram Spec', value = ""),
DiagrammeROutput('diagram')
))

server = function(input, output){
output$diagram <- renderDiagrammeR(DiagrammeR(
input$spec
))
}

shinyApp(ui = ui, server = server)

dd

@timelyportfolio
Copy link
Contributor

Try separating by ; so

graph LR; a;

@rich-iannone
Copy link
Owner

Okay, back to the test computer. Tried this:

graph LR; F-->R

Got this:

parse error with graph LR; F-->R
s.rules is undefined

@timelyportfolio
Copy link
Contributor

Well unexpected but not surprising. So the old behavior from this morning did work on all your browsers? I'll revert back to that if so.

@rich-iannone
Copy link
Owner

The earlier build was okay. Perhaps revert to that and continue a bit later with testing of the Shiny app. Also, @jjallaire, is the new Preview release of RStudio the one that supports syntax highlighting of Graphviz and mermaid graphs?

@jjallaire
Copy link
Contributor

No the preview doesn't yet support syntax highlighting, for that you need the daily build which you can get from here: http://www.rstudio.org/download/daily/desktop/

@rich-iannone
Copy link
Owner

Still a problem for me. Get a DOM exception 12. Tried in both RStudio and in browser.

dom_exception_12

@jjallaire
Copy link
Contributor

I can't repro this but I'll bet if I get more info on your configuration I
will be able to:

(1) What OS and exact version of RStudio are you running?

(2) Could you share the source code of your example from above?

On Wed, Jan 28, 2015 at 12:05 AM, Richard Iannone notifications@github.com
wrote:

Still a problem for me. Get a DOM exception 12. Tried in both RStudio and
in browser.

[image: dom_exception_12]
https://cloud.githubusercontent.com/assets/5612024/5932705/13168aa2-a668-11e4-9063-ac129d6bb442.png


Reply to this email directly or view it on GitHub
#51 (comment)
.

@timelyportfolio
Copy link
Contributor

What about without shiny? Wonder why it worked before. Hmmmm...

@jjallaire
Copy link
Contributor

I tried it without Shiny (in RStudio and in R Markdown). I could have
constructed a Shiny example but wanted to make sure I was using the exact
code that Rich was before proceeding.

On Wed, Jan 28, 2015 at 7:07 AM, timelyportfolio notifications@github.com
wrote:

What about without shiny? Wonder why it worked before. Hmmmm...


Reply to this email directly or view it on GitHub
#51 (comment)
.

@timelyportfolio
Copy link
Contributor

Let's wait to hear from @rich-iannone. Works for me static with RStudio Viewer, Chrome, and Firefox. Works for me shiny with Chrome and Firefox and not on RStudio (0.99.179). I'll try the nightly build to see if it fixes.

@rich-iannone
Copy link
Owner

I'll be able to test this shortly...

@timelyportfolio
Copy link
Contributor

Does not work for me in shiny context with newest RStudio viewer. @jjallaire, if you want to try on yours with shiny, the code above allows a quick test:

library(shiny)

ui = shinyUI(fluidPage(
textInput('spec', 'Diagram Spec', value = ""),
DiagrammeROutput('diagram'),
DiagrammeROutput('diagram2')
))

server = function(input, output){
output$diagram <- renderDiagrammeR(DiagrammeR(
input$spec
))
output$diagram2 <- renderDiagrammeR(DiagrammeR(
input$spec
))

}

shinyApp(ui = ui, server = server)

@rich-iannone
Copy link
Owner

Yesterday's testing that didn't work for me used the following:

Jan 26 build of RStudio for Mac
Latest version of Safari/Mac

@timelyportfolio
Copy link
Contributor

What about static?

@rich-iannone
Copy link
Owner

Just to be sure I'm testing this correctly, how do you run a static test?

@timelyportfolio
Copy link
Contributor

non-shiny, so mermaid(...)

@rich-iannone
Copy link
Owner

Ah, of course :) (morning time). I'll test again shortly once I get onto my train.

@rich-iannone
Copy link
Owner

Okay was able to test.
— static: works
— Shiny in RStudio: doesn't work
— Shiny in Safari 8.0.2 (10600.3.5) Mac: doesn't work
— Shiny in Firefox 35.0 Mac: works

@rich-iannone
Copy link
Owner

Here is the screen grab of it working in Firefox:

firefox 35 0_mac

@timelyportfolio
Copy link
Contributor

Ok, that is helpful, so we have contained it. I thought it was caused by the css changes, but actually it is happening somewhere else (in mermaid.init it seems). I'll report back.

@timelyportfolio
Copy link
Contributor

It seems the error comes due to a security constraint with the websocket. Don't have the skillset to fix this, so @rich-iannone up to you how we proceed unless @jjallaire has other ideas.

image

@rich-iannone
Copy link
Owner

Ah, good investigative work. I think we should simply proceed and state the compatibility amongst browsers in a table and update that as we go on.
@jjallaire, what do you think?

@jjallaire
Copy link
Contributor

Yes, that sounds fine to me!

On Wed, Jan 28, 2015 at 11:07 AM, Richard Iannone notifications@github.com
wrote:

Ah, good investigative work. I think we should simply proceed and state
the compatibility amongst browsers in a table and update that as we go on.
@jjallaire https://github.com/jjallaire, what do you think?


Reply to this email directly or view it on GitHub
#51 (comment)
.

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

4 participants