Ensure that reactiveValues keys and values are sorted (#3774)

NEWS.md

@@ -8,6 +8,8 @@
 
 * Closed #789: `<script>` loaded from dynamic UI are no longer loaded using synchronous `XMLHttpRequest` (via jQuery). (#3666)
 
+* For `reactiveValues()` objects, whenever the `$names()` or `$values()` methods are called, the keys are now returned in the order that they were inserted. (#3774)
+
 * `Map` objects are now initialized at load time instead of build time. This avoids potential problems that could arise from storing `fastmap` objects into the built Shiny package. (#3775)
 
 ### Bug fixes

R/reactives.R

@@ -326,6 +326,9 @@ ReactiveValues <- R6Class(
     .dedupe = logical(0),
     # Key, asList(), or names() have been retrieved
     .hasRetrieved = list(),
+    # All names, in insertion order. The names are also stored in the .values
+    # object, but it does not preserve order.
+    .nameOrder = character(0),
 
 
     initialize = function(
@@ -403,6 +406,11 @@ ReactiveValues <- R6Class(
         return(invisible())
       }
 
+      # If it's new, append key to the name order
+      if (!key_exists) {
+        .nameOrder[length(.nameOrder) + 1] <<- key
+      }
+
       # set the value for better logging
       .values$set(key, value)
 
@@ -444,14 +452,13 @@ ReactiveValues <- R6Class(
     },
 
     names = function() {
-      nameValues <- .values$keys()
       if (!isTRUE(.hasRetrieved$names)) {
         domain <- getDefaultReactiveDomain()
-        rLog$defineNames(.reactId, nameValues, .label, domain)
+        rLog$defineNames(.reactId, .nameOrder, .label, domain)
         .hasRetrieved$names <<- TRUE
       }
       .namesDeps$register()
-      return(nameValues)
+      return(.nameOrder)
     },
 
     # Get a metadata value. Does not trigger reactivity.
@@ -499,7 +506,7 @@ ReactiveValues <- R6Class(
     },
 
     toList = function(all.names=FALSE) {
-      listValue <- .values$values()
+      listValue <- .values$mget(.nameOrder)
       if (!all.names) {
         listValue <- listValue[!grepl("^\\.", base::names(listValue))]
       }

tests/testthat/test-reactivity.r

@@ -126,6 +126,23 @@ test_that("ReactiveValues", {
   expect_error(values$a <- 1)
 })
 
+test_that("reactiveValues keys are sorted", {
+  values <- reactiveValues(b=2, a=0)
+  values$C <- 13
+  values$A <- 0
+  values$c <- 3
+  values$B <- 12
+  # Setting an existing value shouldn't change order
+  values$a <- 1
+  values$A <- 11
+
+  expect_identical(isolate(names(values)), c("b", "a", "C", "A", "c", "B"))
+  expect_identical(
+    isolate(reactiveValuesToList(values)),
+    list(b=2, a=1, C=13, A=11, c=3, B=12)
+  )
+})
+
 test_that("reactiveValues() has useful print method", {
   verify_output(test_path("print-reactiveValues.txt"), {
     x <- reactiveValues(x = 1, y = 2, z = 3)