feat: Add session$destroy() to remove all module 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 (ReactiveVal, Observable, Observer, ReactiveValues) auto-register via weak references during init, so destruction is automatic and comprehensive
• ReactiveValues gains a full destroy() method with domain auto-registration, destroyed guards on get()/set(), and a destroyByPrefix() method for namespace-scoped cleanup
• invalidateLater() timers are cancelled on both session end and module destroy, with cross-deregistration to prevent leaks
• Observer onDestroy registration respects the autoDestroy flag
• Destroy callbacks fire deepest-namespace-first (children before parents)
• Accessing a destroyed reactive raises a loud shiny.destroyed.error
• createMockDomain() supports onDestroy/destroy lifecycle (destroy fires on end())
• Reserved namespace ..root is rejected in makeScope()
• registerBookmarkExclude() returns an unsubscribe handle, cleaned up on module destroy
• No OTel/rLog emissions during destruction

Closes #2281 — The canonical issue: insertUI()/callModule() to add, removeUI()/??? to remove. No mechanism to deactivate a module server instance.

Closes #825 — "Reactive subDomains" proposal (2016). Proposes createSubDomain() where ending the subdomain destroys all reactive objects created within it.

Related to #2374 — Request to delete server-side input values on removeUI(). Fix is to wrap the UI component into a module. Then removal of all reactivity (not just the input) becomes: mod_session$destroy().

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)
• ReactiveValues domain auto-registration — user-created reactiveValues() inside a reactive domain auto-register for destroy (like reactiveVal); session-level stores (input/clientData) use destroyByPrefix() for namespace-scoped cleanup instead
• Cross-deregistration pattern for invalidateLater() — onEnded and onDestroy callbacks each deregister the other when they fire, preventing double-cleanup and leaked handles

Files changed

File	Changes
R/reactives.R	destroyedReactiveError, ReactiveVal$destroy(), Observable$destroy(), Observer weak registration with autoDestroy gating, ReactiveValues$destroy() + destroyByPrefix() + domain auto-registration, make_weak_destroy_wrapper()
R/shiny.R	ShinySession destroy infrastructure, makeScope() proxy overrides, wsClosed() integration, registerBookmarkExclude() unsubscribe handle, ..root namespace guard
R/reactive-domains.R	createMockDomain() with onDestroy/destroy support
R/mock-session.R	MockShinySession destroy support, bookmark-exclude real implementations, ..root namespace guard
R/insert-ui.R	@section on removeUI for destroy usage
R/modules.R	@section on moduleServer for destroy usage, realistic docs example
tests/testthat/test-destroy.R	99 tests covering all destroy scenarios including weakref GC, invalidateLater timers, bookmark-exclude cleanup, ReactiveValues destroy
tests/testthat/test-mock-session.R	Updated bookmark-exclude tests for real implementations
tests/testthat/test-test-server.R	Fixed test using reactiveValues as state tracker after session close

🤖 Generated with Claude Code

[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]

Pull request overview

Copilot reviewed 13 out of 14 changed files in this pull request and generated 3 comments.

---

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

[Copilot]

Pull request overview

Copilot reviewed 15 out of 16 changed files in this pull request and generated 3 comments.

---

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

[schloerke]

@rpodcast & @khusmann , I believe it's ready to go. Please let me know how this PR works out for you!

[cpsievert]

Really like how this turned out — centralizing destroy state on the root session and keying it by fully-qualified namespace makes the whole thing compose cleanly, and the nested-scope handling just falls out of that design. 👍

One thing I wanted to talk through before the docs calcify: the public entry point we're steering people toward.

The session/moduleServer/removeUI docs all teach the "return a handle" pattern:

list(result = ..., destroy = session$destroy)
# parent:
mod <- myModuleServer("editor")
mod$destroy()

But because destroy is keyed by namespace (not by proxy object), the parent can also just destroy by id without any handle threading:

myModuleServer("editor")                 # no handle captured
session$makeScope("editor")$destroy()    # tears down the same scope

I confirmed (B) actually destroys the original scope's reactives — it works today because of how you built it.

What gives me pause about leading with (A):

• The parent already knows the id (it generated the ns() for insertUI/removeUI), so threading a closure back out is redundant — it's handing the parent something it could derive from what it already owns.
• It's opt-in, so "destroyable" becomes a property of modules that remembered to export destroy, rather than a property of module scopes generally (which is the nice thing this PR actually gives us).
• It fits awkwardly with "modules return reactives" — we'd be asking authors to return a list where using one element (destroy) poisons the others (the reactives), which the docs themselves warn about.

The mental model that feels most natural to me is the parent owns the child's lifecycle: it inserted the UI under an id, it removes the UI, it destroys the server under the same id. That'd argue for a first-class, symmetric entry point — something like session$destroyScope(id) / destroyModule(session, id) — that takes the id the parent already has, needs no opt-in, and reads as what it does. (makeScope(id)$destroy() works as a backdoor, but the name doesn't read as "tear down," and each call registers throwaway bookmark callbacks as a side effect.)

I don't think this blocks the core mechanism — it's really about which path we make canonical in the docs. Curious whether you considered a parent-driven destroyScope(id)-style API and decided against it, or if it's just not been the focus yet? Happy to prototype what it'd look like if useful.

[schloerke]

The parent already knows the id
Yes, but I don't want to use the id twice if I can help it

"modules return reactives"
Really, that's modules return data and handlers... this is a handler

feels most natural to me is the parent owns the child's lifecycle

session$destroyScope(id) would be session$makeScope(id)$destroy(). Similar to other reactive objects, each object owns its own destroy method. Moving the method to the parent session would break this pattern.

Also, most people don't know about makeScope(id). But will do have access to the current module's session. If they need to perform the destroy elsewhere, then they can use makeScope as a workaround.

[cpsievert]

Do you view it as an anti-pattern to destroy a module from the parent session? My initial reaction is that this would be the more common pattern, which is why I think a more ergonomic API would make sense.

Similar to other reactive objects, each object owns its own destroy method. Moving the method to the parent session would break this pattern.

I see what you mean, but it also feels a little bit odd to me to motivate a harder-to-discover API based on this. The fact that we have a method for making scope also argues to me that it's sensible to one for the reverse operation. That said, maybe it is a little bit weird considering that $makeScope() isn't really public?

[cpsievert]

After chatting, we think session$destroy() should be able to take an id and have it become session$makeScope(id)$destroy()

[mlechon]

Hi! I'm really looking forward to this feature. Thanks for taking the time to implement it!

Have you considered triggering destroy automatically on a module's session when a call to moduleServer(id = "previously_used_id") happens? I can't think of a situation in which one would want the two servers to coexist, so collecting the first one implicitly could be a nice touch.

This may not make sense if you're trying to keep the R and Python APIs aligned. And I can always write a wrapper that does this, so I'll be happy anyways :)