Skip to content

Commit

Permalink
Fixes problem destroying objects outside a interactive console (#613)
Browse files Browse the repository at this point in the history
# Pull Request

<!--- Replace `#nnn` with your issue link for reference. -->

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>
  • Loading branch information
averissimo and github-actions[bot] authored Dec 4, 2024
1 parent 1ba57ba commit 2528aad
Show file tree
Hide file tree
Showing 7 changed files with 24 additions and 22 deletions.
4 changes: 1 addition & 3 deletions R/FilterState.R
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
)

Expand Down
4 changes: 1 addition & 3 deletions R/FilterStateExpr.R
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
)

Expand Down
18 changes: 12 additions & 6 deletions R/FilterStates.R
Original file line number Diff line number Diff line change
Expand Up @@ -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)
},

Expand Down Expand Up @@ -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)
}
)

Expand All @@ -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)
}
),
Expand Down Expand Up @@ -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
Expand Down
4 changes: 1 addition & 3 deletions R/FilterStatesSE.R
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
)

Expand Down
4 changes: 1 addition & 3 deletions R/FilteredData.R
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
)

Expand Down
4 changes: 1 addition & 3 deletions R/FilteredDataset.R
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
)

Expand Down
8 changes: 7 additions & 1 deletion R/utils.R
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down

0 comments on commit 2528aad

Please sign in to comment.