Preserve methods of generics defined in namespace being unloaded

Closes r-lib/vctrs#1341

NEWS.md

@@ -1,5 +1,6 @@
 # pkgload (development version)
 
+* `unload()` no longer unregisters methods for generics of the package being unloaded. This way dangling references to generics defined in the stale namespace still work as expected (r-lib/vctrs#1341).
 * `load_all()` will now work for packages that have testthat tests but do not have testthat installed (#151)
 * The `pkgbuild` dependency has been moved to `Suggests`, as it is only needed for packages with compiled code.
 * `load_all(warn_conflicts = TRUE)` becomes more narrow and only warns when a *function* in the global environment masks a *function* in the package, consistent with the docs (#125, #143 @jennybc).

R/unload.r

@@ -128,6 +128,13 @@ s3_unregister <- function(package) {
       next
     }
 
+    # Don't remove methods for generics defined in the namespace being
+    # unloaded. The stale namespace should still work as much as
+    # possible.
+    if (is_string(ns_env_name(generic_ns), package)) {
+      next
+    }
+
     table <- generic_ns$.__S3MethodsTable__.
     if (!is_environment(table)) {
       next

tests/testthat/test-load.r

@@ -74,8 +74,15 @@ test_that("reloading a package unloads deleted S3 methods", {
   load_all("testS3removed")
   expect_equal(as.character(x), "registered")
 
+  # Hold a reference to the generic in the currently loaded namespace
+  stale_generic <- testS3removed::my_generic
+
   load_all("testS3removed2")
   expect_equal(as.character(x), character())
+
+  # Still works because we don't unregister methods for the package
+  # being unloaded (r-lib/vctrs#1341)
+  expect_equal(stale_generic(x), "registered")
 })
 
 test_that("load_all() errors when no DESCRIPTION found", {

tests/testthat/testS3removed/NAMESPACE

@@ -1,3 +1,5 @@
 # Generated by roxygen2: do not edit by hand
 
 S3method(as.character,pkgload_foobar)
+S3method(my_generic,pkgload_foobar)
+export(my_generic)

tests/testthat/testS3removed/R/S3.r

@@ -1,5 +1,13 @@
-
 #' @export
 as.character.pkgload_foobar <- function(x, ...) {
   "registered"
 }
+
+#' @export
+my_generic <- function(x) {
+  UseMethod("my_generic")
+}
+#' @export
+my_generic.pkgload_foobar <- function(x) {
+  "registered"
+}