feat: Add session$destroy() to clean up dangling reactivity

[schloerke]

Summary

Port of posit-dev/py-shiny#2209 — adds session$destroy() and session$onDestroy() to R Shiny.

• Adds session$destroy() on module session proxies to destroy all reactive state (values, expressions, observers, inputs, outputs) for a module scope and its descendants
• Adds session$onDestroy(callback) to register cleanup callbacks that fire on destroy
• Reactive primitives auto-register via weak references during init, so destruction is automatic and comprehensive
• Destroy callbacks fire deepest-namespace-first (children before parents)
• Accessing a destroyed reactive raises a loud shiny.destroyed.error
• No OTel/rLog emissions during destruction

Key design decisions

• Weak references via rlang::new_weakref() allow reactive objects to be GC'd before explicit destroy() — matching py-shiny semantics
• ShinySession$destroy() throws an error — only module session proxies can be destroyed; root session uses close()
• Destroy callbacks stored centrally on root session in a Map keyed by namespace string
• make_weak_destroy_wrapper() helper avoids closure environment leaks (discovered via GC tests)

Files changed

File	Changes
R/reactives.R	destroyedReactiveError, ReactiveVal$destroy(), Observable$destroy(), Observer weak registration, ReactiveValues$_destroy(), make_weak_destroy_wrapper()
R/shiny.R	ShinySession destroy infrastructure, makeScope() proxy overrides, wsClosed() integration
R/mock-session.R	MockShinySession destroy support for testing
R/insert-ui.R	@seealso on removeUI
tests/testthat/test-destroy.R	56 tests covering all destroy scenarios including weakref GC behavior

Test plan

• Unit tests for each reactive primitive's destroy/guards
• Weakref GC behavior tests (verify no reference leaks)
• Session proxy destroy with namespace ordering
• Integration tests: full module destroy flow, namespace reuse, nested modules
• Full test suite passes (2912 tests, 0 failures)

🤖 Generated with Claude Code

[schloerke]

MockShinySession only invokes destroy callbacks here; it does not remove namespaced inputs/outputs the way ShinySession does. That divergence makes destroy/recreate tests less faithful and can leave stale mock state behind across module lifecycles.

[schloerke]

The proxy gets destroy() / onDestroy(), but it still inherits onSessionEnded() from the root session. So session$onSessionEnded() or onStop(..., session = scope) inside a module will not run on scope$destroy(); they only run when the whole session closes. There is already a note near onStop() calling out that stopped modules need their own onSessionEnded() implementation.

[schloerke]

names(private$.outputs) is NULL until a module has defined an output, so startsWith(outputNames, nsPrefixWithSep) can error with non-character object(s). That makes session$destroy() fail for modules that never created outputs. Coercing outputNames to character(0) before filtering would cover that edge case.

[schloerke]

This only removes clientData keys that start with the module namespace prefix (for example mod1-). Output-related clientData entries are named like output_mod1-plot_hidden / _width / _height, so they survive destroy and can leak stale metadata into a re-created module with the same id.

[Copilot]

Pull request overview

Adds an explicit module-scope teardown API to Shiny sessions to eliminate “dangling reactivity” when dynamic module UI is removed, with reactive primitives participating automatically via weakly-registered destroy hooks.

Changes:

• Introduces session$destroy() / session$onDestroy() for module session proxies, backed by namespace-keyed destroy callback infrastructure on the root session.
• Adds destroy semantics/guards to core reactive primitives (ReactiveVal, Observable, Observer) plus ReactiveValues$_destroy() for namespace cleanup.
• Adds comprehensive destroy-focused test coverage and documentation/NEWS updates.

Reviewed changes

Copilot reviewed 10 out of 11 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
R/shiny.R	Implements destroy callback registry on ShinySession, adds proxy onDestroy()/destroy(), and invokes destroy on websocket close.
R/reactives.R	Adds destroyed error condition + destroy methods and weak onDestroy registration for reactive primitives; adds ReactiveValues$_destroy().
R/mock-session.R	Adds destroy callback infra and proxy destroy support for MockShinySession to enable testing.
tests/testthat/test-destroy.R	New unit tests covering destroyed guards, idempotency, weakref behavior, and module-scope teardown ordering.
man/session.Rd	Documents session$onDestroy() and session$destroy().
man/insertUI.Rd	Documents how session$destroy() relates to removeUI() for module cleanup.
R/insert-ui.R	Adds roxygen section describing module cleanup with session$destroy().
man/MockShinySession.Rd	Documents new MockShinySession$onDestroy() / $destroy().
NEWS.md	Announces new session destroy APIs and behavior.
.gitignore	Ignores docs/ and .context.
.Rbuildignore	Excludes docs and .context from package builds.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

When destroying the root session (nsPrefix = ""), the sentinel key "root" is treated as depth 0 and then tie-broken lexicographically, which can cause root-level onDestroy callbacks to fire before top-level module namespaces. If root callbacks are meant to be the shallowest scope, adjust the sort to always run the root sentinel last (or assign it a depth of -Inf / explicitly move it to the end).

Suggested change

	# Sort deepest-first (most hyphens first)
	depths <- nchar(gsub("[^-]", "", matching))
	matching <- matching[order(-depths, matching)]
	# Sort deepest-first (most hyphens first), but always run the root
	# sentinel last so root-level callbacks are invoked after top-level
	# module namespaces during full-session teardown.
	depths <- nchar(gsub("[^-]", "", matching))
	isRootSentinel <- matching == "__root__"
	matching <- matching[order(-depths, isRootSentinel, matching)]

[Copilot]

Namespace matching and depth calculation are hard-coded to use "-" (e.g., paste0(nsPrefix, "-") and counting hyphens). Since Shiny already defines ns.sep, consider using that constant instead so the destroy logic stays correct/consistent if the separator ever changes.

[Copilot]

MockShinySession$close() invokes destroy callbacks by iterating over Map keys without applying the same deepest-namespace-first ordering used by invokeDestroyCallbacks(). This makes ordering nondeterministic and can diverge from real ShinySession behavior; consider delegating to private$invokeDestroyCallbacks("") (or replicating its sorting) here.

Suggested change

	# Invoke destroy callbacks for all namespaces
	allNs <- private$destroyCallbacksByNs$keys()
	for (ns in allNs) {
	cbs <- private$destroyCallbacksByNs$get(ns)
	if (!is.null(cbs)) {
	cbs$invoke(onError = printError)
	}
	private$destroyCallbacksByNs$remove(ns)
	}
	# Invoke destroy callbacks for all namespaces using the same
	# deepest-namespace-first ordering as the main destroy path.
	private$invokeDestroyCallbacks("")

[Copilot]

This test name mentions a weakref "value (self$destroy)", but the implementation uses rlang::new_weakref(rv_impl) without setting a value. Consider renaming the test description to reflect what’s actually being validated (that the weakref key can be GC’d).