clean-up comments; add more tests

Author: cpsievert

R/partial_bundles.R

@@ -32,8 +32,6 @@
 #'
 
 
-# TODO: implement type = 'auto' which would attempts to find the smallest partial bundle that can render `p`.
-# this would require, however, knowing the bundle -> trace mapping *for `p`'s plotly.js version*
 partial_bundle <- function(p, type = "auto", local = TRUE, minified = TRUE) {
 
   if (!is.plotly(p)) stop("The first argument to `partial_bundle()` must be a plotly object", call. = FALSE)
@@ -45,6 +43,8 @@ partial_bundle <- function(p, type = "auto", local = TRUE, minified = TRUE) {
   p$dependencies[[idx]]$minified <- minified
   p$dependencies[[idx]]$partial_bundle <- match.arg(type, c("auto", "main", names(bundleTraceMap)))
 
+  # unfortunately, htmlwidgets doesn't appear to support manipulation of 
+  # dependencies field during preRenderHook(), so we need to explicitly build
   plotly_build(p)
 }
 
@@ -69,8 +69,8 @@ verify_partial_bundle <- function(p) {
     }
 
     if (identical(bundleType, "auto")) {
-      message(
-        "Couldn't find a single partial bundle that would support this plotly",
+      warning(
+        "Couldn't find a single partial bundle that would support this plotly ",
         "visualization. Using the main (full) bundle instead."
       )
       p$dependencies[[plotlyjsBundleIDX(p)]] <- plotlyMainBundle()

tests/testthat/test-plotly-partial-bundles.R

@@ -1,13 +1,14 @@
 context("partial-bundles")
 
-
-test_that("Can reduce saved file size with an auto partial bundle", {
+test_that("Can reduce saved file size with an basic (auto) partial bundle by half", {
+  skip_on_cran()
 
   p1 <- plot_ly(x = 1:10, y = 1:10) %>% add_markers()
   p2 <- partial_bundle(p1)
+  expect_match(plotlyjsBundle(p2)$name, "basic")
+  
   f1 <- tempfile(fileext = ".html")
   f2 <- tempfile(fileext = ".html")
-  
   file_size <- function(p, f) {
     owd <- setwd(dirname(f))
     on.exit(setwd(owd))
@@ -16,3 +17,34 @@ test_that("Can reduce saved file size with an auto partial bundle", {
   }
   expect_true(file_size(p1, f1) / 2 > file_size(p2, f2))
 })
+
+test_that("Can find the right bundle", {
+  skip_on_cran()
+  
+  p1 <- plot_ly(z = ~volcano) %>% add_heatmap()
+  p2 <- partial_bundle(p1)
+  expect_match(plotlyjsBundle(p2)$name, "cartesian")
+  
+  p3 <- plot_ly(z = ~volcano) %>% add_surface()
+  p4 <- partial_bundle(p3)
+  expect_match(plotlyjsBundle(p3)$name, "gl3d")
+  
+  # At least right now, we don't support multiple partial bundles
+  expect_warning(
+    subplot(p1, p3) %>% partial_bundle(),
+    "Using the main (full) bundle", 
+    fixed = TRUE
+  )
+})
+
+test_that("Can specify the partial bundle", {
+  skip_on_cran()
+  
+  p1 <- plot_ly(x = 1:10, y = 1:10) %>% add_markers()
+  p2 <- partial_bundle(p1, type = "basic")
+  p3 <- partial_bundle(p1, type = "cartesian")
+  
+  expect_match(plotlyjsBundle(p2)$name, "basic")
+  expect_match(plotlyjsBundle(p3)$name, "cartesian")
+  
+})