From 2528aad8c3053850114744e3a86f622b52f8faaa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Ver=C3=ADssimo?= <211358+averissimo@users.noreply.github.com> Date: Wed, 4 Dec 2024 14:14:39 +0000 Subject: [PATCH] Fixes problem destroying objects outside a interactive console (#613) # Pull Request Fixes #612 notes: - When the rsession is closed the default `session` object is changed to `NULL`, while the `module`'s `session` is not finalized is not. This ensures that it always looks at the current reactive domain ### Changes description - `session_bindings` are only cleared if there is a non-null active domain (session) and it hasn't ended --------- Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com> --- R/FilterState.R | 4 +--- R/FilterStateExpr.R | 4 +--- R/FilterStates.R | 18 ++++++++++++------ R/FilterStatesSE.R | 4 +--- R/FilteredData.R | 4 +--- R/FilteredDataset.R | 4 +--- R/utils.R | 8 +++++++- 7 files changed, 24 insertions(+), 22 deletions(-) diff --git a/R/FilterState.R b/R/FilterState.R index c73147e3d..8bec8b0c9 100644 --- a/R/FilterState.R +++ b/R/FilterState.R @@ -278,9 +278,7 @@ FilterState <- R6::R6Class( # nolint private$session_bindings[[session$ns("inputs")]] <- list( destroy = function() { logger::log_debug("Destroying FilterState inputs and observers; id: { private$get_id() }") - if (!session$isEnded()) { - lapply(session$ns(names(input)), .subset2(input, "impl")$.values$remove) - } + lapply(session$ns(names(input)), .subset2(input, "impl")$.values$remove) } ) diff --git a/R/FilterStateExpr.R b/R/FilterStateExpr.R index aec04968b..747c25b9c 100644 --- a/R/FilterStateExpr.R +++ b/R/FilterStateExpr.R @@ -179,9 +179,7 @@ FilterStateExpr <- R6::R6Class( # nolint private$session_bindings[[session$ns("inputs")]] <- list( destroy = function() { logger::log_debug("Destroying FilterState inputs and observers; id: { private$get_id() }") - if (!session$isEnded()) { - lapply(session$ns(names(input)), .subset2(input, "impl")$.values$remove) - } + lapply(session$ns(names(input)), .subset2(input, "impl")$.values$remove) } ) diff --git a/R/FilterStates.R b/R/FilterStates.R index 5ac79756e..04a92313b 100644 --- a/R/FilterStates.R +++ b/R/FilterStates.R @@ -65,6 +65,15 @@ FilterStates <- R6::R6Class( # nolint private$data <- data private$data_reactive <- data_reactive private$state_list <- reactiveVal() + + # Clears state list when finalizing the object + private$session_bindings[["clear_state_list"]] <- list( + destroy = function() { + private$state_list_empty(force = TRUE) + isolate(private$state_list(NULL)) + } + ) + invisible(self) }, @@ -492,9 +501,7 @@ FilterStates <- R6::R6Class( # nolint # Extra observer that clears all input values in session private$session_bindings[[session$ns("inputs")]] <- list( destroy = function() { - if (!session$isEnded()) { - lapply(session$ns(names(input)), .subset2(input, "impl")$.values$remove) - } + lapply(session$ns(names(input)), .subset2(input, "impl")$.values$remove) } ) @@ -512,9 +519,7 @@ FilterStates <- R6::R6Class( # nolint #' @return `NULL`, invisibly. #' finalize = function() { - .finalize_session_bindings(self, private) # Remove all inputs and observers - private$state_list_empty(force = TRUE) - isolate(private$state_list(NULL)) + .finalize_session_bindings(self, private) invisible(NULL) } ), @@ -651,6 +656,7 @@ FilterStates <- R6::R6Class( # nolint state_list_remove = function(state_id, force = FALSE) { checkmate::assert_character(state_id) logger::log_debug("{ class(self)[1] } removing a filter, state_id: { toString(state_id) }") + isolate({ current_state_ids <- vapply(private$state_list(), function(x) x$get_state()$id, character(1)) to_remove <- state_id %in% current_state_ids diff --git a/R/FilterStatesSE.R b/R/FilterStatesSE.R index 06e14eac1..fca4fb188 100644 --- a/R/FilterStatesSE.R +++ b/R/FilterStatesSE.R @@ -300,9 +300,7 @@ SEFilterStates <- R6::R6Class( # nolint # Extra observer that clears all input values in session private$session_bindings[[session$ns("inputs")]] <- list( destroy = function() { - if (!session$isEnded()) { - lapply(session$ns(names(input)), .subset2(input, "impl")$.values$remove) - } + lapply(session$ns(names(input)), .subset2(input, "impl")$.values$remove) } ) diff --git a/R/FilteredData.R b/R/FilteredData.R index c36227c80..5f7f00255 100644 --- a/R/FilteredData.R +++ b/R/FilteredData.R @@ -690,9 +690,7 @@ FilteredData <- R6::R6Class( # nolint private$session_bindings[[session$ns("inputs")]] <- list( destroy = function() { - if (!session$isEnded()) { - lapply(session$ns(names(input)), .subset2(input, "impl")$.values$remove) - } + lapply(session$ns(names(input)), .subset2(input, "impl")$.values$remove) } ) diff --git a/R/FilteredDataset.R b/R/FilteredDataset.R index 2ffefe420..691a16226 100644 --- a/R/FilteredDataset.R +++ b/R/FilteredDataset.R @@ -375,9 +375,7 @@ FilteredDataset <- R6::R6Class( # nolint private$session_bindings[[session$ns("inputs")]] <- list( destroy = function() { - if (!session$isEnded()) { - lapply(session$ns(names(input)), .subset2(input, "impl")$.values$remove) - } + lapply(session$ns(names(input)), .subset2(input, "impl")$.values$remove) } ) diff --git a/R/utils.R b/R/utils.R index caceb09c5..d29cb9b7b 100644 --- a/R/utils.R +++ b/R/utils.R @@ -53,7 +53,13 @@ make_c_call <- function(choices) { #' @return `NULL` invisibly #' @keywords internal .finalize_session_bindings <- function(self, private) { - if (length(private$session_bindings) > 0) lapply(private$session_bindings, function(x) x$destroy()) + # Only finalize shiny session binding when there is an active session + if ( + !is.null(getDefaultReactiveDomain()) && + !getDefaultReactiveDomain()$isEnded() + ) { + lapply(private$session_bindings, function(x) x$destroy()) + } invisible(NULL) }