From 3c7e2adb6c0cc10d1d2e888951650423f3371642 Mon Sep 17 00:00:00 2001
From: Konrad Rudolph <konrad.rudolph@gmail.com>
Date: Fri, 21 May 2021 23:00:45 +0100
Subject: [PATCH] Reload dependent modules (#195)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Naïve implementation of dependency reloading (fixes #39, #165)
---
 NEWS.md                      |  1 +
 R/unload.r                   |  5 ++++
 tests/testthat/test-reload.r | 44 ++++++++++++++++++++++++++++++++++++
 3 files changed, 50 insertions(+)

diff --git a/NEWS.md b/NEWS.md
index 1c86a07a..b64c4b33 100644
--- a/NEWS.md
+++ b/NEWS.md
@@ -1,5 +1,6 @@
 # box (development version)
 
+* Fix: Reload dependent modules (#39, #165, #195)
 * Fix: Don’t crash in the presence of nested, expanded functions inside modules
     (#203)
 
diff --git a/R/unload.r b/R/unload.r
index 981ddfde..85a4ea3f 100644
--- a/R/unload.r
+++ b/R/unload.r
@@ -67,6 +67,11 @@ reload = function (mod) {
     call_hook(mod_ns, '.on_unload', mod_ns)
     deregister_mod(info)
 
+    for (import_ns in namespace_info(mod_ns, 'imports')) {
+        call_hook(import_ns, '.on_unload', import_ns)
+        deregister_mod(attr(import_ns, 'info'))
+    }
+
     on.exit({
         warning(sprintf(
             'Reloading module %s failed, attempting to restore the old instance.',
diff --git a/tests/testthat/test-reload.r b/tests/testthat/test-reload.r
index 1086d77b..b0ed52a7 100644
--- a/tests/testthat/test-reload.r
+++ b/tests/testthat/test-reload.r
@@ -9,6 +9,24 @@ unload_all = function () {
     rm(list = ls(modenv), envir = modenv)
 }
 
+tempfile_dir = function (...) {
+    file = tempfile()
+    dir.create(file)
+    file
+}
+
+create_nested_test_module = function (dir) {
+    mod = file.path(dir, 'mod', 'a')
+    dir.create(mod, recursive = TRUE)
+    writeLines("#' @export\nbox::use(./sub)", file.path(mod, '__init__.r'))
+    writeLines("#' @export\nvalue = 1L", file.path(mod, 'sub.r'))
+}
+
+edit_nested_test_module = function (dir) {
+    mod = file.path(dir, 'mod', 'a')
+    writeLines("#' @export\nvalue = 2L", file.path(mod, 'sub.r'))
+}
+
 test_that('module can be reloaded', {
     # Required since other tests have side-effects.
     # Tear-down would be helpful here, but not supported by testthat.
@@ -32,3 +50,29 @@ test_that('reload checks its arguments', {
     box::use(mod/a)
     expect_error(box::reload((a)))
 })
+
+test_that('reload includes submodules', {
+    dir = tempfile_dir()
+    on.exit(unlink(dir, recursive = TRUE))
+
+    old_path = options(box.path = dir)
+    on.exit(options(old_path), add = TRUE)
+
+    create_nested_test_module(dir)
+
+    box::use(mod/a)
+
+    expect_equal(a$sub$value, 1L)
+
+    edit_nested_test_module(dir)
+
+    box::reload(a)
+
+    expect_equal(a$sub$value, 2L)
+
+    # To do:
+    # * modules with compiled source,
+    # * tricky packages loaded as modules, e.g. packages that call
+    #   system.file(), and alike, and
+    # * modules with S4 classes/object,
+})