From d2deda238aa7674b6bc4c786fd00400ec46c741f Mon Sep 17 00:00:00 2001 From: trestletech Date: Wed, 14 Aug 2019 14:25:05 -0500 Subject: [PATCH] Move global.R sourcing into an exported load function --- NAMESPACE | 1 + R/app.R | 39 ++++++++++++---- inst/staticdocs/index.r | 3 +- man/loadSupport.Rd | 28 +++++++++++ tests/test-helpers/app1-standard/global.R | 1 + tests/test-helpers/app3-badglobal/R/helper.R | 2 + tests/test-helpers/app3-badglobal/global.R | 2 + tests/testthat/test-app.R | 49 +++++++++++++------- 8 files changed, 98 insertions(+), 27 deletions(-) create mode 100644 man/loadSupport.Rd create mode 100644 tests/test-helpers/app3-badglobal/R/helper.R create mode 100644 tests/test-helpers/app3-badglobal/global.R diff --git a/NAMESPACE b/NAMESPACE index 0c2b3bdbb..408912ba6 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -138,6 +138,7 @@ export(knit_print.shiny.appobj) export(knit_print.shiny.render.function) export(knit_print.shiny.tag) export(knit_print.shiny.tag.list) +export(loadSupport) export(mainPanel) export(makeReactiveBinding) export(markRenderFunction) diff --git a/R/app.R b/R/app.R index 6791c0216..5a5bee20a 100644 --- a/R/app.R +++ b/R/app.R @@ -219,10 +219,8 @@ shinyAppDir_serverR <- function(appDir, options=list()) { oldwd <<- getwd() setwd(appDir) monitorHandle <<- initAutoReloadMonitor(appDir) - if (file.exists(file.path.ci(appDir, "global.R"))){ - sourceUTF8(file.path.ci(appDir, "global.R")) - } - loadHelpers(appDir, envir = sharedEnv) + # TODO: we should support hot reloading on global.R and R/*.R changes. + loadSupport(appDir, renv=sharedEnv, globalrenv=globalenv()) } onStop <- function() { setwd(oldwd) @@ -295,14 +293,34 @@ initAutoReloadMonitor <- function(dir) { obs$destroy } -# Loads in all helpers in the R/ directory of the app -# From `list.files`: -# > The files are sorted in alphabetical order, on the full path -loadHelpers <- function(appDir, envir=globalenv()){ +#' Load an app's supporting R files +#' +#' Loads all of the supporting R files of a Shiny application. Specifically, +#' this function loads any top-level supporting `.R` files in the `R/` directory +#' adjacent to the app and a `global.R` file. +#' +#' @details The files are sourced in alphabetical order (as determined by +#' [list.files]). `global.R` is evaluated before the supporting R files in the +#' `R/` directory. +#' @param appDir The application directory +#' @param renv The environmeny in which the files in the `R/` directory should +#' be evaluated. +#' @param globalrenv The environment in which `global.R` should be evaluated. If +#' `NULL`, `global.R` will not be evaluated at all. +#' @export +loadSupport <- function(appDir, renv=new.env(parent=globalenv()), globalrenv=globalenv()){ + if (!is.null(globalrenv)){ + # Evaluate global.R, if it exists. + if (file.exists(file.path.ci(appDir, "global.R"))){ + sourceUTF8(file.path.ci(appDir, "global.R"), envir=globalrenv) + } + } helpersDir <- file.path(appDir, "R") helpers <- list.files(helpersDir, pattern="\\.[rR]$", recursive=FALSE, full.names=TRUE) - lapply(helpers, sourceUTF8, envir=envir) + lapply(helpers, sourceUTF8, envir=renv) + + invisible(renv) } # This reads in an app dir for a single-file application (e.g. app.R), and @@ -364,7 +382,8 @@ shinyAppDir_appR <- function(fileName, appDir, options=list()) onStart <- function() { oldwd <<- getwd() setwd(appDir) - loadHelpers(appDir, sharedEnv) + # TODO: we should support hot reloading on R/*.R changes. + loadSupport(appDir, renv=sharedEnv, globalrenv=NULL) monitorHandle <<- initAutoReloadMonitor(appDir) if (!is.null(appObj()$onStart)) appObj()$onStart() } diff --git a/inst/staticdocs/index.r b/inst/staticdocs/index.r index 1244d6e47..d1089b4cb 100644 --- a/inst/staticdocs/index.r +++ b/inst/staticdocs/index.r @@ -155,7 +155,8 @@ sd_section("Running", "runUrl", "stopApp", "viewer", - "isRunning" + "isRunning", + "loadSupport" ) ) sd_section("Bookmarking state", diff --git a/man/loadSupport.Rd b/man/loadSupport.Rd new file mode 100644 index 000000000..f2876cd36 --- /dev/null +++ b/man/loadSupport.Rd @@ -0,0 +1,28 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/app.R +\name{loadSupport} +\alias{loadSupport} +\title{Load an app's supporting R files} +\usage{ +loadSupport(appDir, renv = new.env(parent = globalenv()), + globalrenv = globalenv()) +} +\arguments{ +\item{appDir}{The application directory} + +\item{renv}{The environmeny in which the files in the \code{R/} directory should +be evaluated.} + +\item{globalrenv}{The environment in which \code{global.R} should be evaluated. If +\code{NULL}, \code{global.R} will not be evaluated at all.} +} +\description{ +Loads all of the supporting R files of a Shiny application. Specifically, +this function loads any top-level supporting \code{.R} files in the \code{R/} directory +adjacent to the app and a \code{global.R} file. +} +\details{ +The files are sourced in alphabetical order (as determined by +\link{list.files}). \code{global.R} is evaluated before the supporting R files in the +\code{R/} directory. +} diff --git a/tests/test-helpers/app1-standard/global.R b/tests/test-helpers/app1-standard/global.R index e69de29bb..7d32435f1 100644 --- a/tests/test-helpers/app1-standard/global.R +++ b/tests/test-helpers/app1-standard/global.R @@ -0,0 +1 @@ +global <- "ABC" diff --git a/tests/test-helpers/app3-badglobal/R/helper.R b/tests/test-helpers/app3-badglobal/R/helper.R new file mode 100644 index 000000000..d1ce60eef --- /dev/null +++ b/tests/test-helpers/app3-badglobal/R/helper.R @@ -0,0 +1,2 @@ + +helper1 <- 456 diff --git a/tests/test-helpers/app3-badglobal/global.R b/tests/test-helpers/app3-badglobal/global.R new file mode 100644 index 000000000..2c6e78a68 --- /dev/null +++ b/tests/test-helpers/app3-badglobal/global.R @@ -0,0 +1,2 @@ + +stop("I wasn't supposed to be sourced") diff --git a/tests/testthat/test-app.R b/tests/testthat/test-app.R index c6c18fffe..0287d5987 100644 --- a/tests/testthat/test-app.R +++ b/tests/testthat/test-app.R @@ -1,15 +1,30 @@ context("app") -test_that("helpers are loaded into the right env", { - env <- new.env(parent=environment()) - loadHelpers("../test-helpers/app1-standard", envir=env) - expect_equal(get("helper1", env), 123) - expect_equal(get("helper2", env), "abc") +test_that("files are loaded into the right env", { + renv <- new.env(parent=environment()) + genv <- new.env(parent=environment()) + + loadSupport("../test-helpers/app1-standard", renv=renv, globalrenv=genv) + expect_equal(get("helper1", renv, inherits=FALSE), 123) + expect_equal(get("helper2", renv, inherits=FALSE), "abc") + + expect_equal(get("global", genv, inherits=FALSE), "ABC") }) -test_that("nested helpers are loaded", { - loadHelpers("../test-helpers/app2-nested") +test_that("Can suppress sourcing global.R", { + # Confirm that things blow up if we source global.R + expect_error(loadSupport("../test-helpers/app3-badglobal")) + + # Shouldn't see an error now that we're suppressing global sourcing. + renv <- loadSupport("../test-helpers/app3-badglobal", globalrenv=NULL) + + # But other helpers are still sourced + expect_true(exists("helper1", envir=renv)) +}) + +test_that("nested helpers are not loaded", { + loadSupport("../test-helpers/app2-nested", renv=environment()) expect_equal(helper1, 456) expect_false(exists("helper2")) }) @@ -20,7 +35,7 @@ test_that("app with both r/ and R/ prefers R/", { warning=function(w){testthat::skip("File system is not case-sensitive")}) writeLines("upperHelper <- 'abc'", file.path("../test-helpers/app4-both/R", "upper.R")) - loadHelpers("../test-helpers/app4-both") + loadSupport("../test-helpers/app4-both") expect_false(exists("lowerHelper")) expect_equal(upperHelper, "abc") @@ -35,10 +50,10 @@ test_that("With ui/server.R, global.R is loaded before R/ helpers and into the r # + shinyAppDir_serverR # +--- sourceUTF8 - # +--+ loadHelpers + # +--+ loadSupport # | +--- sourceUTF8 - loadSpy <- rewire(loadHelpers, sourceUTF8 = sourceStub) - sad <- rewire(shinyAppDir_serverR, sourceUTF8 = sourceStub, loadHelpers = loadSpy) + loadSpy <- rewire(loadSupport, sourceUTF8 = sourceStub) + sad <- rewire(shinyAppDir_serverR, sourceUTF8 = sourceStub, loadSupport = loadSpy) sa <- sad(normalizePath("../test-helpers/app1-standard")) sa$onStart() @@ -51,8 +66,10 @@ test_that("With ui/server.R, global.R is loaded before R/ helpers and into the r expect_match(calls[[3]][[1]], "/helperLower\\.r$", perl=TRUE) # Check environments - # global.R has no env specified -- loaded into the global env. - expect_length(calls[[1]], 1) + # global.R loaded into the global env + gEnv <- calls[[1]]$envir + expect_identical(gEnv, globalenv()) + # helpers are loaded into a child of the global env helperEnv1 <- calls[[2]]$envir helperEnv2 <- calls[[3]]$envir @@ -86,10 +103,10 @@ test_that("app.R is loaded after R/ helpers and into the right envs", { # + shinyAppDir_serverR # +--- sourceUTF8 - # +--+ loadHelpers + # +--+ loadSupport # | +--- sourceUTF8 - loadSpy <- rewire(loadHelpers, sourceUTF8 = sourceSpy) - sad <- rewire(shinyAppDir_appR, sourceUTF8 = sourceSpy, loadHelpers = loadSpy) + loadSpy <- rewire(loadSupport, sourceUTF8 = sourceSpy) + sad <- rewire(shinyAppDir_appR, sourceUTF8 = sourceSpy, loadSupport = loadSpy) sa <- sad("app.R", normalizePath("../test-helpers/app2-nested")) sa$onStart()