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

runtime: shinyrmd always renders at startup #2217

Closed
5 tasks done
aronatkins opened this issue Sep 17, 2021 · 7 comments
Closed
5 tasks done

runtime: shinyrmd always renders at startup #2217

aronatkins opened this issue Sep 17, 2021 · 7 comments
Assignees
Labels
bug an unexpected problem or unintended behavior next to consider for next release

Comments

@aronatkins
Copy link
Contributor

aronatkins commented Sep 17, 2021

The 2.11 rmarkdown release (as well as the latest code) always re-renders the HTML before starting the Shiny HTTP server. This is a regression from 2.10 and earlier, which would re-render the HTML only if it was seen as out-dated (per rmarkdown:::shiny_prerendered_prerender).

Given the following server.Rmd and server.R:

---
title: "server.Rmd: rendered at most once"
runtime: shinyrmd
---

This document can be slow for the first client. The R code block is evaluated
at render-time, which occurs at most once, and only when the HTML does not
already exist. When the HTML already exists, there is no render cost.

```{r}
Sys.sleep(1)
```
# server.R that pairs with server.Rmd.
function(input, output) {}

Run this document, stop the server, then check if we see a pre-render as still necessary.

# This call to run() will render "server.html" and then start a Shiny server.
# Stop the Shiny server once it starts listening.
rmarkdown::run(file = "./server.Rmd")

# Check to see if a pre-render is still needed:
rmarkdown:::shiny_prerendered_prerender("./server.Rmd", "./server.html", ".", "1")
# => TRUE

# Perform another call to run(); this will run another render of "server.html"
# before starting the Shiny server.
rmarkdown::run(file = "./server.Rmd")

In rmarkdown-2.10, the call to rmarkdown:::shiny_prerendered_prerender returns FALSE and the second call to rmarkdown::run does not perform a render. This is correct; rmarkdown::run should not render when its inputs are unchanged.

In rmarkdown-2.11, the call to rmarkdown:::shiny_prerendered_prerender returns TRUE and the second call to rmarkdown::run performs an unnecessary render of the HTML. This is a difference from the previous release; the document is unchanged.

It appears as if the following block of code is returning TRUE because we are detecting inconsistent package dependencies (when I run, depPkg="jquerylib" is seen as having a depVer=NULL, which is enough to trigger the render.

# check that all html dependencies exist
dependencies_json <- shiny_prerendered_extract_context(html_lines, "dependencies")
dependencies <- jsonlite::unserializeJSON(dependencies_json)
pkgsSeen <- list()
for (dep in dependencies) {
if (is.null(dep$package)) {
src_file <- dep$src$file
if (!is.null(src_file)) {
if (!file.exists(src_file)) {
# might create a missing file compile-time error,
# but that's better than a missing file prerendered error
return(TRUE)
}
}
# if there is a dep$src$href but no dep$package,
# then we can't determine where the file came from.
# Ignore checking for the href files for now, as pkg versions are checked below
} else {
depPkg <- dep$package
depVer <- dep$pkgVersion
if (is.null(pkgsSeen[[depPkg]])) {
# has not seen pkg
# depVer could be NULL, producing a logical(0)
# means old prerender version, render again
if (!isTRUE(get_package_version_string(depPkg) == depVer)) {
# was not rendered with the same R package. must render again
return(TRUE)
}
pkgsSeen[[depPkg]] <- depVer
}
}
}
# all html dependencies are accounted for

# Session info during some of this experimentation; the rmarkdown version has varied.

> xfun::session_info('rmarkdown')
R version 3.6.3 (2020-02-29)
Platform: x86_64-apple-darwin15.6.0 (64-bit)
Running under: OS X Snow Leopard 11.6, RStudio 2021.8.0.324.0

Locale: en_US.UTF-8 / en_US.UTF-8 / en_US.UTF-8 / C / en_US.UTF-8 / en_US.UTF-8

Package version:
  base64enc_0.1.3  bslib_0.2.5.1    digest_0.6.27    evaluate_0.14   
  fastmap_1.1.0    fs_1.5.0         glue_1.4.2       graphics_3.6.3  
  grDevices_3.6.3  highr_0.9        htmltools_0.5.2  jquerylib_0.1.4 
  jsonlite_1.7.2   knitr_1.34       magrittr_2.0.1   markdown_1.1    
  methods_3.6.3    mime_0.11        R6_2.5.0         rappdirs_0.3.3  
  rlang_0.4.11     rmarkdown_2.11.1 sass_0.4.0       stats_3.6.3     
  stringi_1.7.4    stringr_1.4.0    tinytex_0.33     tools_3.6.3     
  utils_3.6.3      xfun_0.26        yaml_2.2.1      

Pandoc version: 2.14.0.3

Checklist

When filing a bug report, please check the boxes below to confirm that you have provided us with the information we need. Have you:

  • formatted your issue so it is easier for us to read?

  • included a minimal, self-contained, and reproducible example?

  • pasted the output from xfun::session_info('rmarkdown') in your issue?

  • upgraded all your packages to their latest versions (including your versions of R, the RStudio IDE, and relevant R packages)?

    Not really, at least not completely. Using R-3.6.3, as compatibility across R versions is important. Using a recent (but not absolutely the latest) RStudio IDE. Updated rmarkdown package dependencies to latest CRAN releases during my experimentation.

  • installed and tested your bug with the development version of the rmarkdown package using remotes::install_github("rstudio/rmarkdown")?

    Not exactly. I installed a local copy of the latest rmarkdown and was using the RStudio debugger to step through the relevant code.

@aronatkins
Copy link
Contributor Author

Likely related to the following change: #2203

@cderv
Copy link
Collaborator

cderv commented Sep 17, 2021

I'll have a look. cc @gadenbuie as #2203 was opened and merged for the need of learnr

@aronatkins
Copy link
Contributor Author

Workaround:

The render can be avoided by setting the environment variable RMARKDOWN_RUN_PRERENDER=0 ahead of rmarkdown::run. This requires that the caller take responsibility for rendering the original HTML, as rmarkdown::run will err if pre-rendering is disabled and the HTML does not exist.

Sys.setenv(RMARKDOWN_RUN_PRERENDER = "0")
rmarkdown::render("./server.Rmd")
rmarkdown::run("./server.Rmd")

@gadenbuie
Copy link
Member

I think maybe the problem lies in shiny_prerendered_append_dependencies(). If deps$pkgVersion is NULL when walking the deps in the pre-rendered HTML, then it's because dependency$package isn't NULL when walking the shiny prerendered dependencies:

if (is.null(dependency$package) && is.character(dependency$src$file)) {
# check for a package directory parent
package_dir <- proj_root(dependency$src$file)
# if we have one then populate the package field and make the
# src$file relative to the package
if (!is.null(package_dir)) {
package_desc <- read.dcf(file.path(package_dir, "DESCRIPTION"),
all = TRUE)
dependency$package <- package_desc$Package
# named to something that doesn't start with 'package' to deter lazy name matching
dependency$pkgVersion <- package_desc$Version
dependency$src$file <- normalized_relative_to(package_dir,
dependency$src$file)
}
}
# if we couldn't resolve the src to a package then copy the files
if (is.null(dependency$package) && !is.null(dependency$src$file)) {
dependency <- copyDependencyToDir(dependency, files_dir)
dependency <- makeDependencyRelative(dependency, output_dir)
dependency$src = list(href = unname(dependency$src))
}

But the jquery dependency comes with a $package item

List of 10
 $ name      : chr "jquery"
 $ version   : chr "3.6.0"
 $ src       :List of 1
  ..$ file: chr "lib/3.6.0"
 $ meta      : NULL
 $ script    : chr "jquery-3.6.0.min.js"
 $ stylesheet: NULL
 $ head      : NULL
 $ attachment: NULL
 $ package   : chr "jquerylib"
 $ all_files : logi TRUE
 - attr(*, "class")= chr "html_dependency"

we just don't add the $pkgVersion for jquerylib to what we write into the prerendered doc. If we just add that item into the transformed dependency (e.g. after line 348 above), we can "fix" this problem.

    if (!is.null(dependency$package)) {
      dependency$pkgVersion <- utils::packageVersion(dependency$package)
    }

But I don't think this is the right approach. We really shouldn't be using the package version but rather the dependency version (or a combo of both maybe?). For example, a dependency with version a.b.c provided by package X at version x.y.z might not change when package X updates to version x.y.z+ due to unrelated changes.

I think the above fix is a reasonable short-term bandaid, but my understanding of the shiny prerendered dependency resolution is that much of it was developed alongside early versions of htmltools. Now that htmltools is more mature, we should revisit this section of R Markdown to update the handling of dependencies here.

@cderv
Copy link
Collaborator

cderv commented Sep 17, 2021

Thanks for the quick look and investigation !

I think the above fix is a reasonable short-term bandaid, but my understanding of the shiny prerendered dependency resolution is that much of it was developed alongside early versions of htmltools. Now that htmltools is more mature, we should revisit this section of R Markdown to update the handling of dependencies here.

Last time I look into it, I had the same impression: Would it work the same with new htmltools ?
But this is a part of the codebase hard to change from experience. Touching rmarkdown intenrals is often not without consequence 😅

I'll have a look following your hints and path @gadenbuie ! Thanks a lot!

@cderv cderv added bug an unexpected problem or unintended behavior next to consider for next release labels Dec 15, 2021
@cderv cderv moved this to Backlog in R Markdown Team Projects Dec 22, 2021
@cderv
Copy link
Collaborator

cderv commented Feb 7, 2022

@aronatkins I have merged @gadenbuie PR. It fixes the regression from rmarkdown 2.11 introduced while migrating to using jquerylib. I know you have your workarounds, just a heads up that this will be in next 2.12 version.

I'll close this here then.

@cderv cderv closed this as completed Feb 7, 2022
Repository owner moved this from Backlog to Done in R Markdown Team Projects Feb 7, 2022
@github-actions
Copy link

github-actions bot commented Aug 9, 2022

This old thread has been automatically locked. If you think you have found something related to this, please open a new issue by following the issue guide (https://yihui.org/issue/), and link to this old issue if necessary.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug an unexpected problem or unintended behavior next to consider for next release
Projects
Archived in project
Development

No branches or pull requests

3 participants