Fix #2946: Ensure invalidation on freezeReactiveValue(input) freeze and new value

NEWS.md

@@ -4,11 +4,14 @@ shiny 1.5.0.9000
 
 ## Full changelog
 
-
 ### Breaking changes
 
 * Closed #3074: Shiny no longer supports file uploads for Internet Explorer 8 or 9. (#3075)
 
+* Subtle changes, and some soft-deprecations, have come to `freezeReactiveValue` and `freezeReactiveVal` (#3055). These functions have been fragile at best in previous releases (issues #1791, #2463, #2946). In this release, we've solved all the problems we know about with `freezeReactiveValue(input, "x")`, by 1) invalidating `input$x` and set it to `NULL` whenever we freeze, and 2) ensuring that, after a freeze, even if the effect of `renderUI` or `updateXXXInput` is to set `input$x` to the same value it already has, this will result in an invalidation (whereas by default, Shiny filters out such spurious assignments).
+
+  Similar problems may exist when using `freezeReactiveVal`, and when using `freezeReactiveValue` with non-`input` reactive values objects. But support for those was added mostly for symmetry with `freezeReactiveValue(input)`, and given the above issues, it's not clear to us how you could have used these successfully in the past, or why you would even want to. For this release, we're soft-deprecating both of those uses, but we're more than willing to un-deprecate if it turns out people are using these; if that includes you, please join the conversation at https://github.com/rstudio/shiny/issues/3063. In the meantime, you can squelch the deprecation messages for these functions specifically, by setting `options(shiny.deprecation.messages.freeze = FALSE)`.
+
 ### Accessibility
 
 * Added [bootstrap accessibility plugin](https://github.com/paypal/bootstrap-accessibility-plugin) under the hood to improve accessibility of shiny apps for screen-reader and keyboard users: the enhancements include better navigations for alert, tooltip, popover, modal dialog, dropdown, tab Panel, collapse, and carousel elements. (#2911)

R/reactives.R

@@ -231,6 +231,12 @@ reactiveVal <- function(value = NULL, label = NULL) {
 #' @rdname freezeReactiveValue
 #' @export
 freezeReactiveVal <- function(x) {
+  if (getOption("shiny.deprecation.messages", TRUE) && getOption("shiny.deprecation.messages.freeze", TRUE)) {
+    rlang::warn(
+      "freezeReactiveVal() is soft-deprecated, and may be removed in a future version of Shiny. (See https://github.com/rstudio/shiny/issues/3063)",
+      .frequency = "once", .frequency_id = "freezeReactiveVal")
+  }
+
   domain <- getDefaultReactiveDomain()
   if (is.null(domain)) {
     stop("freezeReactiveVal() must be called when a default reactive domain is active.")
@@ -357,7 +363,7 @@ ReactiveValues <- R6Class(
       keyValue
     },
 
-    set = function(key, value) {
+    set = function(key, value, force = FALSE) {
       # if key exists
       #   if it is the same value, return
       #
@@ -389,10 +395,8 @@ ReactiveValues <- R6Class(
 
       key_exists <- .values$containsKey(key)
 
-      if (key_exists) {
-        if (.dedupe && identical(.values$get(key), value)) {
-          return(invisible())
-        }
+      if (key_exists && !isTRUE(force) && .dedupe && identical(.values$get(key), value)) {
+        return(invisible())
       }
 
       # set the value for better logging
@@ -469,10 +473,15 @@ ReactiveValues <- R6Class(
 
     # Mark a value as frozen If accessed while frozen, a shiny.silent.error will
     # be thrown.
-    freeze = function(key) {
+    freeze = function(key, invalidate = FALSE) {
       domain <- getDefaultReactiveDomain()
       rLog$freezeReactiveKey(.reactId, key, domain)
       setMeta(key, "frozen", TRUE)
+
+      if (invalidate) {
+        # Force an invalidation
+        self$set(key, NULL, force = TRUE)
+      }
     },
 
     thaw = function(key) {
@@ -727,7 +736,10 @@ str.reactivevalues <- function(object, indent.str = " ", ...) {
 #' thing that happens if `req(FALSE)` is called. The value is thawed
 #' (un-frozen; accessing it will no longer raise an exception) when the current
 #' reactive domain is flushed. In a Shiny application, this occurs after all of
-#' the observers are executed.
+#' the observers are executed. **NOTE:** We are considering deprecating
+#' `freezeReactiveVal`, and `freezeReactiveValue` except when `x` is `input`.
+#' If this affects your app, please let us know by leaving a comment on
+#' [this GitHub issue](https://github.com/rstudio/shiny/issues/3063).
 #'
 #' @param x For `freezeReactiveValue`, a [reactiveValues()]
 #'   object (like `input`); for `freezeReactiveVal`, a

R/shiny.R

@@ -943,7 +943,33 @@ ShinySession <- R6Class(
 
       impl <- .subset2(x, 'impl')
       key <- .subset2(x, 'ns')(name)
-      impl$freeze(key)
+
+      is_input <- identical(impl, private$.input)
+
+      # There's no good reason for us not to just do force=TRUE, except that we
+      # know this fixes problems for freezeReactiveValue(input) but we don't
+      # currently even know what you would use freezeReactiveValue(rv) for. In
+      # the spirit of not breaking things we don't understand, we're making as
+      # targeted a fix as possible, while emitting a deprecation warning (below)
+      # that should help us gather more data about the other case.
+      impl$freeze(key, invalidate = is_input)
+
+      if (is_input) {
+        # Notify the client that this input was frozen. The client will ensure
+        # that the next time it sees a value for that input, even if the value
+        # has not changed from the last known value of that input, it will be
+        # sent to the server anyway.
+        private$sendMessage(frozen = list(
+          ids = list(key)
+        ))
+      } else {
+        if (getOption("shiny.deprecation.messages", TRUE) && getOption("shiny.deprecation.messages.freeze", TRUE)) {
+          rlang::warn(
+            "Support for calling freezeReactiveValue() with non-`input` reactiveValues objects is soft-deprecated, and may be removed in a future version of Shiny. (See https://github.com/rstudio/shiny/issues/3063)",
+            .frequency = "once", .frequency_id = "freezeReactiveValue")
+        }
+      }
+
       self$onFlushed(function() impl$thaw(key))
     },
 

srcjs/init_shiny.js

@@ -106,6 +106,16 @@ function initShiny() {
     inputs.setInput(name, value, opts);
   };
 
+  // By default, Shiny deduplicates input value changes; that is, if
+  // `setInputValue` is called with the same value as the input already
+  // has, the call is ignored (unless opts.priority = "event"). Calling
+  // `forgetLastInputValue` tells Shiny that the very next call to
+  // `setInputValue` for this input id shouldn't be ignored, even if it
+  // is a dupe of the existing value.
+  exports.forgetLastInputValue = function(name) {
+    inputsNoResend.forget(name);
+  };
+
   var boundInputs = {};
 
   function valueChangeCallback(binding, el, allowDeferred) {

srcjs/input_binding_tabinput.js

@@ -36,6 +36,7 @@ $.extend(bootstrapTabInputBinding, {
   receiveMessage: function(el, data) {
     if (data.hasOwnProperty('value'))
       this.setValue(el, data.value);
+    $(el).trigger("change");
   },
   subscribe: function(el, callback) {
     $(el).on('change shown.bootstrapTabInputBinding shown.bs.tab.bootstrapTabInputBinding', function(event) {

srcjs/input_rate.js

@@ -260,6 +260,9 @@ var InputNoResendDecorator = function(target, initialValues) {
 
     this.lastSentValues = cacheValues;
   };
+  this.forget = function(name) {
+    delete this.lastSentValues[name];
+  };
 }).call(InputNoResendDecorator.prototype);
 
 

srcjs/shinyapp.js

@@ -696,6 +696,12 @@ var ShinyApp = function() {
     });
   });
 
+  addMessageHandler('frozen', function(message) {
+    for (let i = 0; i < message.ids.length; i++) {
+      exports.forgetLastInputValue(message.ids[i]);
+    }
+  });
+
   function getTabset(id) {
     var $tabset = $("#" + $escape(id));
     if ($tabset.length === 0)