From 56a43ebb9030856717f3a9280673f3d84f574186 Mon Sep 17 00:00:00 2001
From: Michael Lawrence <lawremi@gmail.com>
Date: Wed, 21 Aug 2024 03:27:38 -0700
Subject: [PATCH 01/13] avoid treating non-S7 generics as external generics

---
 R/method-register.R | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/R/method-register.R b/R/method-register.R
index dbd22c0a..c827a4a4 100644
--- a/R/method-register.R
+++ b/R/method-register.R
@@ -85,7 +85,7 @@ register_method <- function(generic,
 
   # if we're inside a package, we also need to be able register methods
   # when the package is loaded
-  if (!is.null(package) && !is_local_generic(generic, package)) {
+  if (is_S7_generic(generic) && !is.null(package) && !is_local_generic(generic, package)) {
     generic <- as_external_generic(generic)
     external_methods_add(package, generic, signature, method)
   }

From 502197b91e4fc7863eedfd146c397d74868161db Mon Sep 17 00:00:00 2001
From: Michael Lawrence <lawremi@gmail.com>
Date: Wed, 21 Aug 2024 03:55:54 -0700
Subject: [PATCH 02/13] treat both S7 generics from another package and
 external generics as external

---
 R/method-register.R | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/R/method-register.R b/R/method-register.R
index c827a4a4..39e43570 100644
--- a/R/method-register.R
+++ b/R/method-register.R
@@ -85,7 +85,7 @@ register_method <- function(generic,
 
   # if we're inside a package, we also need to be able register methods
   # when the package is loaded
-  if (is_S7_generic(generic) && !is.null(package) && !is_local_generic(generic, package)) {
+  if ((is_S7_generic(generic) || is_external_generic(generic)) && !is.null(package) && !is_local_generic(generic, package)) {
     generic <- as_external_generic(generic)
     external_methods_add(package, generic, signature, method)
   }

From a756514df407b4f1026690997466c5040387b1bd Mon Sep 17 00:00:00 2001
From: Michael Lawrence <lawremi@gmail.com>
Date: Fri, 23 Aug 2024 08:19:12 -0700
Subject: [PATCH 03/13] use correct environment for S3 method registration

---
 R/method-register.R | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/R/method-register.R b/R/method-register.R
index 39e43570..a8f254f2 100644
--- a/R/method-register.R
+++ b/R/method-register.R
@@ -78,7 +78,7 @@ register_method <- function(generic,
       register_method(gen, signature, method, package = NULL)
     }
   } else if (is_S3_generic(generic)) {
-    register_S3_method(generic, signature, method)
+    register_S3_method(generic, signature, method, env)
   } else if (is_S4_generic(generic)) {
     register_S4_method(generic, signature, method, env)
   }
@@ -93,7 +93,7 @@ register_method <- function(generic,
   invisible(generic)
 }
 
-register_S3_method <- function(generic, signature, method) {
+register_S3_method <- function(generic, signature, method, envir = parent.frame()) {
   if (class_type(signature[[1]]) != "S7") {
     msg <- sprintf(
       "When registering methods for S3 generic %s(), signature must be an S7 class, not %s.",
@@ -103,7 +103,7 @@ register_S3_method <- function(generic, signature, method) {
     stop(msg, call. = FALSE)
   }
   class <- S7_class_name(signature[[1]])
-  registerS3method(generic$name, class, method, envir = parent.frame())
+  registerS3method(generic$name, class, method, envir)
 }
 
 register_S7_method <- function(generic, signature, method) {

From c7fd04ddb1fa3cb35b689e4bc3b8240bf5b7e2e6 Mon Sep 17 00:00:00 2001
From: Michael Lawrence <lawremi@gmail.com>
Date: Fri, 23 Aug 2024 09:35:01 -0700
Subject: [PATCH 04/13] use correct package in S3 external generic

---
 R/external-generic.R | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/R/external-generic.R b/R/external-generic.R
index ddcf663f..d66be786 100644
--- a/R/external-generic.R
+++ b/R/external-generic.R
@@ -47,7 +47,7 @@ as_external_generic <- function(x) {
   } else if (is_external_generic(x)) {
     x
   } else if (is_S3_generic(x)) {
-    pkg <- package_name(x)
+    pkg <- package_name(x$generic)
     new_external_generic(pkg, x$name, "__S3__")
   } else if (is_S4_generic(x)) {
     new_external_generic(x@package, as.vector(x@generic), x@signature)

From c731cdbb81c6bb3ad80885fa7c88771f2881d7c4 Mon Sep 17 00:00:00 2001
From: Michael Lawrence <lawremi@gmail.com>
Date: Fri, 23 Aug 2024 09:35:37 -0700
Subject: [PATCH 05/13] Revert "avoid treating non-S7 generics as external
 generics"

This reverts commit 56a43ebb9030856717f3a9280673f3d84f574186.
---
 R/method-register.R | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/R/method-register.R b/R/method-register.R
index a8f254f2..5bc4fd63 100644
--- a/R/method-register.R
+++ b/R/method-register.R
@@ -85,7 +85,7 @@ register_method <- function(generic,
 
   # if we're inside a package, we also need to be able register methods
   # when the package is loaded
-  if ((is_S7_generic(generic) || is_external_generic(generic)) && !is.null(package) && !is_local_generic(generic, package)) {
+  if (!is.null(package) && !is_local_generic(generic, package)) {
     generic <- as_external_generic(generic)
     external_methods_add(package, generic, signature, method)
   }

From 6e4e99678611e04c928a5db61ab3a678ce83c3b5 Mon Sep 17 00:00:00 2001
From: Michael Lawrence <lawremi@gmail.com>
Date: Sat, 24 Aug 2024 13:09:40 -0700
Subject: [PATCH 06/13] methods_register() passes the package namespace to
 register_method()

---
 R/external-generic.R | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/R/external-generic.R b/R/external-generic.R
index d66be786..b14cb265 100644
--- a/R/external-generic.R
+++ b/R/external-generic.R
@@ -91,7 +91,7 @@ methods_register <- function() {
   tbl <- S7_methods_table(package)
 
   for (x in tbl) {
-    register <- registrar(x$generic, x$signature, x$method)
+    register <- registrar(x$generic, x$signature, x$method, parent.frame())
 
     if (isNamespaceLoaded(x$generic$package)) {
       register()
@@ -103,7 +103,7 @@ methods_register <- function() {
   invisible()
 }
 
-registrar <- function(generic, signature, method) {
+registrar <- function(generic, signature, method, env) {
   # Force all arguments
   list(generic, signature, method)
 
@@ -115,7 +115,7 @@ registrar <- function(generic, signature, method) {
         warning(msg, call. = FALSE)
       } else {
         generic_fun <- get(generic$name, envir = ns, inherits = FALSE)
-        register_method(generic_fun, signature, method, package = NULL)
+        register_method(generic_fun, signature, method, env, package = NULL)
       }
     }
   }

From 34d0c286f752eb8eeb18b9c1ed9a797df90f1f26 Mon Sep 17 00:00:00 2001
From: Tomasz Kalinowski <tomasz@posit.co>
Date: Wed, 2 Oct 2024 17:02:42 -0400
Subject: [PATCH 07/13] Update tests

---
 tests/testthat/t0/DESCRIPTION          |  2 +-
 tests/testthat/t0/NAMESPACE            |  3 ++-
 tests/testthat/t0/R/pkg.R              |  2 --
 tests/testthat/t0/R/t0.R               |  5 +++++
 tests/testthat/t1/DESCRIPTION          |  2 +-
 tests/testthat/t1/NAMESPACE            |  3 ++-
 tests/testthat/t1/R/foo.R              |  2 --
 tests/testthat/t1/R/t1.R               |  5 +++++
 tests/testthat/t2/DESCRIPTION          |  2 +-
 tests/testthat/t2/NAMESPACE            |  3 ++-
 tests/testthat/t2/R/pkg.R              |  8 --------
 tests/testthat/t2/R/t2.R               | 16 ++++++++++++++++
 tests/testthat/test-external-generic.R | 14 +++++++++-----
 13 files changed, 44 insertions(+), 23 deletions(-)
 delete mode 100644 tests/testthat/t0/R/pkg.R
 create mode 100644 tests/testthat/t0/R/t0.R
 delete mode 100644 tests/testthat/t1/R/foo.R
 create mode 100644 tests/testthat/t1/R/t1.R
 delete mode 100644 tests/testthat/t2/R/pkg.R
 create mode 100644 tests/testthat/t2/R/t2.R

diff --git a/tests/testthat/t0/DESCRIPTION b/tests/testthat/t0/DESCRIPTION
index 9ba949ac..6cf5954a 100644
--- a/tests/testthat/t0/DESCRIPTION
+++ b/tests/testthat/t0/DESCRIPTION
@@ -15,4 +15,4 @@ License: MIT + file LICENSE
 Encoding: UTF-8
 LazyData: true
 Roxygen: list(markdown = TRUE)
-RoxygenNote: 7.1.1
+RoxygenNote: 7.3.2
diff --git a/tests/testthat/t0/NAMESPACE b/tests/testthat/t0/NAMESPACE
index 449a7e53..5647257c 100644
--- a/tests/testthat/t0/NAMESPACE
+++ b/tests/testthat/t0/NAMESPACE
@@ -1,3 +1,4 @@
 # Generated by roxygen2: do not edit by hand
 
-export(bar)
+export(an_s3_generic)
+export(an_s7_generic)
diff --git a/tests/testthat/t0/R/pkg.R b/tests/testthat/t0/R/pkg.R
deleted file mode 100644
index 0314702b..00000000
--- a/tests/testthat/t0/R/pkg.R
+++ /dev/null
@@ -1,2 +0,0 @@
-#' @export
-bar <- S7::new_generic("bar", "x")
diff --git a/tests/testthat/t0/R/t0.R b/tests/testthat/t0/R/t0.R
new file mode 100644
index 00000000..c6c61181
--- /dev/null
+++ b/tests/testthat/t0/R/t0.R
@@ -0,0 +1,5 @@
+#' @export
+an_s7_generic <- S7::new_generic("an_s7_generic", "x")
+
+#' @export
+an_s3_generic <- function(x) UseMethod("x")
diff --git a/tests/testthat/t1/DESCRIPTION b/tests/testthat/t1/DESCRIPTION
index 90fb6a00..bd9ecdb0 100644
--- a/tests/testthat/t1/DESCRIPTION
+++ b/tests/testthat/t1/DESCRIPTION
@@ -15,4 +15,4 @@ License: MIT + file LICENSE
 Encoding: UTF-8
 LazyData: true
 Roxygen: list(markdown = TRUE)
-RoxygenNote: 7.1.1
+RoxygenNote: 7.3.2
diff --git a/tests/testthat/t1/NAMESPACE b/tests/testthat/t1/NAMESPACE
index 620cb573..d28a2ae2 100644
--- a/tests/testthat/t1/NAMESPACE
+++ b/tests/testthat/t1/NAMESPACE
@@ -1,3 +1,4 @@
 # Generated by roxygen2: do not edit by hand
 
-export(foo)
+export(another_s3_generic)
+export(another_s7_generic)
diff --git a/tests/testthat/t1/R/foo.R b/tests/testthat/t1/R/foo.R
deleted file mode 100644
index 41f8d169..00000000
--- a/tests/testthat/t1/R/foo.R
+++ /dev/null
@@ -1,2 +0,0 @@
-#' @export
-foo <- S7::new_generic("foo", "x")
diff --git a/tests/testthat/t1/R/t1.R b/tests/testthat/t1/R/t1.R
new file mode 100644
index 00000000..a40712c1
--- /dev/null
+++ b/tests/testthat/t1/R/t1.R
@@ -0,0 +1,5 @@
+#' @export
+another_s7_generic <- S7::new_generic("another_s7_generic", "x")
+
+#' @export
+another_s3_generic <- function(x) UseMethod("another_s3_generic")
diff --git a/tests/testthat/t2/DESCRIPTION b/tests/testthat/t2/DESCRIPTION
index 07267376..07c4290d 100644
--- a/tests/testthat/t2/DESCRIPTION
+++ b/tests/testthat/t2/DESCRIPTION
@@ -16,4 +16,4 @@ License: MIT + file LICENSE
 Encoding: UTF-8
 LazyData: true
 Roxygen: list(markdown = TRUE)
-RoxygenNote: 7.1.1
+RoxygenNote: 7.3.2
diff --git a/tests/testthat/t2/NAMESPACE b/tests/testthat/t2/NAMESPACE
index 31d91238..f2946151 100644
--- a/tests/testthat/t2/NAMESPACE
+++ b/tests/testthat/t2/NAMESPACE
@@ -1,3 +1,4 @@
 # Generated by roxygen2: do not edit by hand
 
-importFrom(t0,bar)
+importFrom(t0,an_s3_generic)
+importFrom(t0,an_s7_generic)
diff --git a/tests/testthat/t2/R/pkg.R b/tests/testthat/t2/R/pkg.R
deleted file mode 100644
index ee10a4df..00000000
--- a/tests/testthat/t2/R/pkg.R
+++ /dev/null
@@ -1,8 +0,0 @@
-foo <- S7::new_external_generic("t1", "foo", "x")
-S7::method(foo, S7::class_character) <- function(x) "foo"
-
-S7::method(bar, S7::class_character) <- function(x) "bar"
-
-.onLoad <- function(libname, pkgname) {
-  S7::methods_register()
-}
diff --git a/tests/testthat/t2/R/t2.R b/tests/testthat/t2/R/t2.R
new file mode 100644
index 00000000..cc08575c
--- /dev/null
+++ b/tests/testthat/t2/R/t2.R
@@ -0,0 +1,16 @@
+
+#' @importFrom t0 an_s7_generic
+S7::method(an_s7_generic, S7::class_character) <- function(x) "foo"
+
+#' @importFrom t0 an_s3_generic
+S7::method(an_s3_generic, S7::class_character) <- function(x) "foo"
+
+another_s7_generic <- S7::new_external_generic("t1", "another_s7_generic", "x")
+S7::method(another_s7_generic, S7::class_character) <- function(x) "foo"
+
+another_s3_generic <- S7::new_external_generic("t1", "another_s3_generic", "x")
+S7::method(another_s3_generic, S7::class_character) <- function(x) "foo"
+
+.onLoad <- function(libname, pkgname) {
+  S7::methods_register()
+}
diff --git a/tests/testthat/test-external-generic.R b/tests/testthat/test-external-generic.R
index 22bcd626..7d7312f3 100644
--- a/tests/testthat/test-external-generic.R
+++ b/tests/testthat/test-external-generic.R
@@ -74,13 +74,17 @@ test_that("new_method works with both hard and soft dependencies", {
 
   quick_install(test_path(c("t0", "t1")), tmp_lib)
   quick_install(test_path("t2"), tmp_lib)
+
   library("t2")
 
-  # t2 has a soft dependency on t1
+  library("t0")
+  # t2 has a hard dependency on t0
+  expect_equal(an_s3_generic("x"), "foo")
+  expect_equal(an_s7_generic("x"), "foo")
+
   library("t1")
-  expect_equal(foo("x"), "foo")
+  # t2 has a soft dependency on t1
+  expect_equal(another_s3_generic("x"), "foo")
+  expect_equal(another_s7_generic("x"), "foo")
 
-  # t2 has a hard dependency on t0
-  library("t0")
-  expect_equal(bar("x"), "bar")
 })

From 3b13b1e6416f7279477687e9100326b12944a464 Mon Sep 17 00:00:00 2001
From: Tomasz Kalinowski <tomasz@posit.co>
Date: Thu, 3 Oct 2024 08:43:55 -0400
Subject: [PATCH 08/13] thinko

---
 tests/testthat/t0/R/t0.R | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/testthat/t0/R/t0.R b/tests/testthat/t0/R/t0.R
index c6c61181..cf5b6843 100644
--- a/tests/testthat/t0/R/t0.R
+++ b/tests/testthat/t0/R/t0.R
@@ -2,4 +2,4 @@
 an_s7_generic <- S7::new_generic("an_s7_generic", "x")
 
 #' @export
-an_s3_generic <- function(x) UseMethod("x")
+an_s3_generic <- function(x) UseMethod("an_s3_generic")

From cf8066782f15359c1fc2c6d6b7c983af880f4e4e Mon Sep 17 00:00:00 2001
From: Tomasz Kalinowski <tomasz@posit.co>
Date: Thu, 3 Oct 2024 13:59:29 -0400
Subject: [PATCH 09/13] expand tests to register external S3 method for an S7
 object

---
 tests/testthat/t2/NAMESPACE |  1 +
 tests/testthat/t2/R/t2.R    | 10 ++++++++--
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/tests/testthat/t2/NAMESPACE b/tests/testthat/t2/NAMESPACE
index f2946151..b5d51e48 100644
--- a/tests/testthat/t2/NAMESPACE
+++ b/tests/testthat/t2/NAMESPACE
@@ -1,4 +1,5 @@
 # Generated by roxygen2: do not edit by hand
 
+export(an_s7_class)
 importFrom(t0,an_s3_generic)
 importFrom(t0,an_s7_generic)
diff --git a/tests/testthat/t2/R/t2.R b/tests/testthat/t2/R/t2.R
index cc08575c..540be248 100644
--- a/tests/testthat/t2/R/t2.R
+++ b/tests/testthat/t2/R/t2.R
@@ -1,15 +1,21 @@
 
+#' @export
+an_s7_class <- S7::new_class("an_s7_class")
+
 #' @importFrom t0 an_s7_generic
 S7::method(an_s7_generic, S7::class_character) <- function(x) "foo"
+S7::method(an_s7_generic, an_s7_class) <- function(x) "foo"
 
 #' @importFrom t0 an_s3_generic
-S7::method(an_s3_generic, S7::class_character) <- function(x) "foo"
+S7::method(an_s3_generic, an_s7_class) <- function(x) "foo"
+
 
 another_s7_generic <- S7::new_external_generic("t1", "another_s7_generic", "x")
 S7::method(another_s7_generic, S7::class_character) <- function(x) "foo"
+S7::method(another_s7_generic, an_s7_class) <- function(x) "foo"
 
 another_s3_generic <- S7::new_external_generic("t1", "another_s3_generic", "x")
-S7::method(another_s3_generic, S7::class_character) <- function(x) "foo"
+# S7::method(another_s3_generic, an_s7_class) <- function(x) "foo" ## BROKEN
 
 .onLoad <- function(libname, pkgname) {
   S7::methods_register()

From a6bb2762b9605ba2a8f8e2d6966be9814d9c02b6 Mon Sep 17 00:00:00 2001
From: Tomasz Kalinowski <tomasz@posit.co>
Date: Fri, 4 Oct 2024 14:40:18 -0400
Subject: [PATCH 10/13] fix external S3 method registration

---
 DESCRIPTION                            |  1 +
 R/aaa.R                                | 57 ++++++++++++++++++++++++++
 R/external-generic.R                   | 18 ++++----
 R/method-register.R                    | 17 +++++---
 tests/testthat/t2/R/t2.R               |  2 +-
 tests/testthat/test-external-generic.R | 47 ++++++++++++++++-----
 6 files changed, 117 insertions(+), 25 deletions(-)

diff --git a/DESCRIPTION b/DESCRIPTION
index 98d65945..d298719f 100644
--- a/DESCRIPTION
+++ b/DESCRIPTION
@@ -36,6 +36,7 @@ Suggests:
     methods,
     rmarkdown,
     testthat (>= 3.2.0),
+    callr,
     tibble
 VignetteBuilder: 
     knitr
diff --git a/R/aaa.R b/R/aaa.R
index 6b33979d..5eea03b6 100644
--- a/R/aaa.R
+++ b/R/aaa.R
@@ -7,3 +7,60 @@ new_function <- function(args = NULL,
                          env = asNamespace("S7")) {
   as.function.default(c(args, body) %||% list(NULL), env)
 }
+
+`append1<-` <- function (x, value) {
+  stopifnot(is.list(x) || identical(mode(x), mode(value)))
+  x[[length(x) + 1L]] <- value
+  x
+}
+
+
+dbg <- function(..., .display = utils::str) {
+  out <- NULL
+  exprs <- as.list(substitute(list(...)))[-1L]
+
+  for (i in seq_len(...length())) {
+    arg_i <- as.symbol(sprintf("..%i", i))
+    if (eval(substitute(missing(arg_i)))) next
+
+    name <- names(exprs)[[i]]
+    expr <- deparse1(exprs[[i]])
+
+    label <- if (is.null(name)) {
+      sprintf("`%s`: ", expr)
+    } else {
+      sprintf("(%s) `%s`: ", name, expr)
+    }
+    cat(label)
+    .display(out <- eval(arg_i))
+  }
+
+  cl <- sys.call()
+  filepath <- utils::getSrcFilename(cl)
+
+  if (length(filepath)) {
+    if (!file.exists(filepath) && file.exists(file.path("R", filepath))) {
+      filepath <- file.path("R", filepath)
+    }
+
+    lineno <- utils::getSrcLocation(cl)
+
+    if (isNamespaceLoaded("cli")) {
+      cli <- asNamespace("cli")
+      loc <- cli$col_grey(cli$style_hyperlink(
+        sprintf("(from %s:%i)", filepath, lineno),
+        sprintf("file://%s", normalizePath(filepath, mustWork = FALSE)),
+        params = c(line = lineno)
+      ))
+    } else {
+      loc <- sprintf("(from %s:%i)", filepath, lineno)
+    }
+
+    cat(loc, "\n")
+  } else {
+    cat(sprintf("(from call: %s (srcfile missing))\n",
+                trimws(deparse1(sys.call(-2), width.cutoff = 60))))
+  }
+
+  invisible(out)
+}
diff --git a/R/external-generic.R b/R/external-generic.R
index b14cb265..95ef67ee 100644
--- a/R/external-generic.R
+++ b/R/external-generic.R
@@ -88,10 +88,13 @@ is_external_generic <- function(x) {
 #' }
 methods_register <- function() {
   package <- packageName(parent.frame())
+  ns <- topenv(parent.frame())
+  # TODO?: check/enforce that methods_register() is being called from .onLoad()
+
   tbl <- S7_methods_table(package)
 
   for (x in tbl) {
-    register <- registrar(x$generic, x$signature, x$method, parent.frame())
+    register <- registrar(x$generic, x$signature, x$method, ns)
 
     if (isNamespaceLoaded(x$generic$package)) {
       register()
@@ -105,7 +108,7 @@ methods_register <- function() {
 
 registrar <- function(generic, signature, method, env) {
   # Force all arguments
-  list(generic, signature, method)
+  generic; signature; method; env;
 
   function(...) {
     ns <- asNamespace(generic$package)
@@ -129,12 +132,11 @@ external_methods_reset <- function(package) {
 external_methods_add <- function(package, generic, signature, method) {
   tbl <- S7_methods_table(package)
 
-  methods <- append(
-    tbl,
-    list(list(generic = generic, signature = signature, method = method))
-  )
+  append1(tbl) <- list(generic = generic,
+                       signature = signature,
+                       method = method)
 
-  S7_methods_table(package) <- methods
+  S7_methods_table(package) <- tbl
   invisible()
 }
 
@@ -144,7 +146,7 @@ external_methods_add <- function(package, generic, signature, method) {
 S7_methods_table <- function(package) {
   ns <- asNamespace(package)
   tbl <- ns[[".__S3MethodsTable__."]]
-  attr(tbl, "S7methods")
+  attr(tbl, "S7methods") %||% list()
 }
 `S7_methods_table<-` <- function(package, value) {
   ns <- asNamespace(package)
diff --git a/R/method-register.R b/R/method-register.R
index 5bc4fd63..58e34ef8 100644
--- a/R/method-register.R
+++ b/R/method-register.R
@@ -67,16 +67,15 @@ register_method <- function(generic,
   generic <- as_generic(generic)
   signature <- as_signature(signature, generic)
 
+
+  if (is_external_generic(generic) && isNamespaceLoaded(generic$package)) {
+    generic <- as_generic(getFromNamespace(generic$name, generic$package))
+  }
+
   # Register in current session
   if (is_S7_generic(generic)) {
     check_method(method, generic, name = method_name(generic, signature))
     register_S7_method(generic, signature, method)
-  } else if (is_external_generic(generic)) {
-    # Only register immediately if soft dependency is available
-    if (requireNamespace(generic$package, quietly = TRUE)) {
-      gen <- getFromNamespace(generic$name, asNamespace(generic$package))
-      register_method(gen, signature, method, package = NULL)
-    }
   } else if (is_S3_generic(generic)) {
     register_S3_method(generic, signature, method, env)
   } else if (is_S4_generic(generic)) {
@@ -102,7 +101,13 @@ register_S3_method <- function(generic, signature, method, envir = parent.frame(
     )
     stop(msg, call. = FALSE)
   }
+
+  if (is_external_generic(external_generic <- get0(generic$name, envir = envir))) {
+    envir <- asNamespace(external_generic$package)
+  }
+
   class <- S7_class_name(signature[[1]])
+  # dbg(generic$name, class, method, envir)
   registerS3method(generic$name, class, method, envir)
 }
 
diff --git a/tests/testthat/t2/R/t2.R b/tests/testthat/t2/R/t2.R
index 540be248..a98bd7b7 100644
--- a/tests/testthat/t2/R/t2.R
+++ b/tests/testthat/t2/R/t2.R
@@ -15,7 +15,7 @@ S7::method(another_s7_generic, S7::class_character) <- function(x) "foo"
 S7::method(another_s7_generic, an_s7_class) <- function(x) "foo"
 
 another_s3_generic <- S7::new_external_generic("t1", "another_s3_generic", "x")
-# S7::method(another_s3_generic, an_s7_class) <- function(x) "foo" ## BROKEN
+S7::method(another_s3_generic, an_s7_class) <- function(x) "foo"
 
 .onLoad <- function(libname, pkgname) {
   S7::methods_register()
diff --git a/tests/testthat/test-external-generic.R b/tests/testthat/test-external-generic.R
index 7d7312f3..fb9ce61f 100644
--- a/tests/testthat/test-external-generic.R
+++ b/tests/testthat/test-external-generic.R
@@ -59,10 +59,6 @@ test_that("new_method works with both hard and soft dependencies", {
   skip_if(getRversion() < "4.1" && Sys.info()[["sysname"]] == "Windows")
   skip_if(quick_test())
 
-  tmp_lib <- tempfile()
-  dir.create(tmp_lib)
-  old_libpaths <- .libPaths()
-  .libPaths(c(tmp_lib, old_libpaths))
 
   on.exit({
     .libPaths(old_libpaths)
@@ -70,21 +66,52 @@ test_that("new_method works with both hard and soft dependencies", {
     try(detach("package:t1", unload = TRUE), silent = TRUE)
     try(detach("package:t0", unload = TRUE), silent = TRUE)
     unlink(tmp_lib, recursive = TRUE)
+    # remove.packages(c("t1", "t0", "t2"))
   })
 
-  quick_install(test_path(c("t0", "t1")), tmp_lib)
+  tmp_lib <- tempfile()
+  dir.create(tmp_lib)
+  old_libpaths <- .libPaths()
+  .libPaths(c(tmp_lib, old_libpaths))
+
+  # t2 has a hard dependency on t0
+  # t2 has a soft dependency on t1
+
+  # First, ensure that t2 can install and run successfully without t1 installed
+  quick_install(test_path("t0"), tmp_lib)
   quick_install(test_path("t2"), tmp_lib)
 
   library("t2")
-
   library("t0")
-  # t2 has a hard dependency on t0
-  expect_equal(an_s3_generic("x"), "foo")
+  expect_equal(an_s3_generic(t2::an_s7_class()), "foo")
   expect_equal(an_s7_generic("x"), "foo")
 
+  # Now install the soft dependency
+  quick_install(test_path("t1"), tmp_lib)
+
   library("t1")
-  # t2 has a soft dependency on t1
-  expect_equal(another_s3_generic("x"), "foo")
+  expect_equal(another_s3_generic(t2::an_s7_class()), "foo")
   expect_equal(another_s7_generic("x"), "foo")
 
+
+  ## Check again in a fresh session, with everything installed
+  expect_no_error(callr::r(function() {
+    library(t2)
+
+    stopifnot(exprs = {
+      t0::an_s3_generic(an_s7_class()) == "foo"
+      t0::an_s7_generic("x") == "foo"
+    })
+
+    if(isNamespaceLoaded("t1"))
+      stop("Prematurely loaded {t1}")
+
+    stopifnot(exprs = {
+      t1::another_s3_generic(an_s7_class()) == "foo"
+      t1::another_s7_generic("x") == "foo"
+    })
+
+    NULL
+  }))
+
 })

From e35049f637d1e6ee45e664d216aef78b6ab7d628 Mon Sep 17 00:00:00 2001
From: Tomasz Kalinowski <tomasz@posit.co>
Date: Fri, 4 Oct 2024 14:53:46 -0400
Subject: [PATCH 11/13] tweak

---
 R/aaa.R | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/R/aaa.R b/R/aaa.R
index 5eea03b6..88c4dd72 100644
--- a/R/aaa.R
+++ b/R/aaa.R
@@ -20,19 +20,19 @@ dbg <- function(..., .display = utils::str) {
   exprs <- as.list(substitute(list(...)))[-1L]
 
   for (i in seq_len(...length())) {
-    arg_i <- as.symbol(sprintf("..%i", i))
-    if (eval(substitute(missing(arg_i)))) next
+    ..i <- as.symbol(sprintf("..%i", i))
+    if (eval(substitute(missing(..i)))) next
 
     name <- names(exprs)[[i]]
     expr <- deparse1(exprs[[i]])
 
     label <- if (is.null(name)) {
-      sprintf("`%s`: ", expr)
+      sprintf("`%s`", expr)
     } else {
-      sprintf("(%s) `%s`: ", name, expr)
+      sprintf("(%s) `%s`", name, expr)
     }
-    cat(label)
-    .display(out <- eval(arg_i))
+    cat(label, ": ", sep = "")
+    .display(out <- eval(..i))
   }
 
   cl <- sys.call()

From c95af4da3750123ef94d43f027f1899364ad0870 Mon Sep 17 00:00:00 2001
From: Tomasz Kalinowski <tomasz@posit.co>
Date: Fri, 4 Oct 2024 14:56:09 -0400
Subject: [PATCH 12/13] move `dbg()` to helpers.

---
 R/aaa.R                 | 51 --------------------------------------
 tests/testthat/helper.R | 55 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 55 insertions(+), 51 deletions(-)

diff --git a/R/aaa.R b/R/aaa.R
index 88c4dd72..452c6f19 100644
--- a/R/aaa.R
+++ b/R/aaa.R
@@ -13,54 +13,3 @@ new_function <- function(args = NULL,
   x[[length(x) + 1L]] <- value
   x
 }
-
-
-dbg <- function(..., .display = utils::str) {
-  out <- NULL
-  exprs <- as.list(substitute(list(...)))[-1L]
-
-  for (i in seq_len(...length())) {
-    ..i <- as.symbol(sprintf("..%i", i))
-    if (eval(substitute(missing(..i)))) next
-
-    name <- names(exprs)[[i]]
-    expr <- deparse1(exprs[[i]])
-
-    label <- if (is.null(name)) {
-      sprintf("`%s`", expr)
-    } else {
-      sprintf("(%s) `%s`", name, expr)
-    }
-    cat(label, ": ", sep = "")
-    .display(out <- eval(..i))
-  }
-
-  cl <- sys.call()
-  filepath <- utils::getSrcFilename(cl)
-
-  if (length(filepath)) {
-    if (!file.exists(filepath) && file.exists(file.path("R", filepath))) {
-      filepath <- file.path("R", filepath)
-    }
-
-    lineno <- utils::getSrcLocation(cl)
-
-    if (isNamespaceLoaded("cli")) {
-      cli <- asNamespace("cli")
-      loc <- cli$col_grey(cli$style_hyperlink(
-        sprintf("(from %s:%i)", filepath, lineno),
-        sprintf("file://%s", normalizePath(filepath, mustWork = FALSE)),
-        params = c(line = lineno)
-      ))
-    } else {
-      loc <- sprintf("(from %s:%i)", filepath, lineno)
-    }
-
-    cat(loc, "\n")
-  } else {
-    cat(sprintf("(from call: %s (srcfile missing))\n",
-                trimws(deparse1(sys.call(-2), width.cutoff = 60))))
-  }
-
-  invisible(out)
-}
diff --git a/tests/testthat/helper.R b/tests/testthat/helper.R
index 45061f8a..0ca68375 100644
--- a/tests/testthat/helper.R
+++ b/tests/testthat/helper.R
@@ -69,3 +69,58 @@ named_list <- function(...) {
   names(x) <- names2(x)
   x
 }
+
+
+dbg <- function(..., .display = utils::str) {
+  out <- NULL
+  exprs <- as.list(substitute(list(...)))[-1L]
+
+  for (i in seq_len(...length())) {
+    ..i <- as.symbol(sprintf("..%i", i))
+    if (eval(substitute(missing(..i)))) {
+      next
+    }
+
+    name <- names(exprs)[[i]]
+    expr <- deparse1(exprs[[i]])
+
+    label <- if (is.null(name)) {
+      sprintf("`%s`", expr)
+    } else {
+      sprintf("(%s) `%s`", name, expr)
+    }
+    cat(label, ": ", sep = "")
+    .display(out <- eval(..i))
+  }
+
+  cl <- sys.call()
+  filepath <- utils::getSrcFilename(cl)
+
+  if (length(filepath)) {
+    if (!file.exists(filepath) &&
+        file.exists(file.path("R", filepath))) {
+      filepath <- file.path("R", filepath)
+    }
+
+    lineno <- utils::getSrcLocation(cl)
+
+    if (isNamespaceLoaded("cli")) {
+      cli <- asNamespace("cli")
+      loc <- cli$col_grey(cli$style_hyperlink(
+        sprintf("(from %s:%i)", filepath, lineno),
+        sprintf("file://%s", normalizePath(filepath, mustWork = FALSE)),
+        params = c(line = lineno)
+      ))
+    } else {
+      loc <- sprintf("(from %s:%i)", filepath, lineno)
+    }
+
+    cat(loc, "\n")
+  } else {
+    cat(sprintf("(from call: %s (srcfile missing))\n", trimws(
+      deparse1(sys.call(-2), width.cutoff = 60)
+    )))
+  }
+
+  invisible(out)
+}

From 37e9169eba79086cabbe58ae105686e6ac359bcb Mon Sep 17 00:00:00 2001
From: Tomasz Kalinowski <kalinowskit@gmail.com>
Date: Mon, 7 Oct 2024 08:35:21 -0400
Subject: [PATCH 13/13] Add NEWS

---
 NEWS.md | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/NEWS.md b/NEWS.md
index f8c6c47b..31ecac69 100644
--- a/NEWS.md
+++ b/NEWS.md
@@ -1,16 +1,18 @@
 # S7 (development version)
 
-* `convert()` now provides a default method to transform a parent class instance 
+* Fixed S3 methods registration across packages (#422).
+
+* `convert()` now provides a default method to transform a parent class instance
   into a subclass, enabling class construction from a prototype (#444).
 
-* The default object constructor returned by `new_class()` has been updated. 
+* The default object constructor returned by `new_class()` has been updated.
   It now accepts lazy (promise) property defaults and includes dynamic properties
-  with a `setter` in the constructor. Additionally, all custom property setters 
-  are now consistently invoked by the default constructor. If you're using S7 in 
-  an R package, you'll need to re-document to ensure that your documentation 
+  with a `setter` in the constructor. Additionally, all custom property setters
+  are now consistently invoked by the default constructor. If you're using S7 in
+  an R package, you'll need to re-document to ensure that your documentation
   matches the updated usage (#438, #445).
 
-* Fixed an issue where a custom property `getter()` would infinitely recurse 
+* Fixed an issue where a custom property `getter()` would infinitely recurse
   when accessing itself (reported in #403, fixed in #406).
 
 * Property setting (via `prop<-` and `@<-`) rewritten in C for performance (#396).