Store directories in $LOADED_FEATURES

lib/zeitwerk/kernel.rb

@@ -29,6 +29,7 @@ def require(path)
         required
       else
         loader.on_dir_autoloaded(path)
+        $LOADED_FEATURES << path
         true
       end
     else

lib/zeitwerk/loader.rb

@@ -123,13 +123,7 @@ def setup
     # @sig () -> void
     def unload
       mutex.synchronize do
-        # We are going to keep track of the files that were required by our
-        # autoloads to later remove them from $LOADED_FEATURES, thus making them
-        # loadable by Kernel#require again.
-        #
-        # Directories are not stored in $LOADED_FEATURES, keeping track of files
-        # is enough.
-        unloaded_files = Set.new
+        abspaths_of_unloaded_crefs = Set.new
 
         autoloads.each do |abspath, (parent, cname)|
           if parent.autoload?(cname)
@@ -139,7 +133,7 @@ def unload
             # and the constant path would escape unloadable_cpath? This is just
             # defensive code to clean things up as much as we are able to.
             unload_cref(parent, cname)
-            unloaded_files.add(abspath) if ruby?(abspath)
+            abspaths_of_unloaded_crefs.add(abspath)
           end
         end
 
@@ -150,10 +144,14 @@ def unload
           end
 
           unload_cref(parent, cname)
-          unloaded_files.add(abspath) if ruby?(abspath)
+          abspaths_of_unloaded_crefs.add(abspath)
         end
 
-        unless unloaded_files.empty?
+        unless abspaths_of_unloaded_crefs.empty?
+          # We remove these abspaths from $LOADED_FEATURES because, otherwise,
+          # `require`'s idempotence would prevent newly defined autoloads from
+          # loading them again.
+          #
           # Bootsnap decorates Kernel#require to speed it up using a cache and
           # this optimization does not check if $LOADED_FEATURES has the file.
           #
@@ -165,7 +163,7 @@ def unload
           # Rails applications may depend on bootsnap, so for unloading to work
           # in that setting it is preferable that we restrict our API choice to
           # one of those methods.
-          $LOADED_FEATURES.reject! { |file| unloaded_files.member?(file) }
+          $LOADED_FEATURES.reject! { |file| abspaths_of_unloaded_crefs.member?(file) }
         end
 
         autoloads.clear

test/lib/zeitwerk/test_eager_load.rb

@@ -159,6 +159,7 @@ class MyGem
 
         remove_const :DbAdapters
         delete_loaded_feature "db_adapters/mysql_adapter.rb"
+        delete_loaded_feature "db_adapters"
       end
 
       $test_eager_load_eager_loaded_p = false
@@ -212,6 +213,7 @@ module DbAdapters::MysqlAdapter
 
         delete_loaded_feature "ns/foo.rb"
         delete_loaded_feature "ns/bar.rb"
+        delete_loaded_feature "ns"
       end
 
       $test_eager_load_eager_loaded_p = false
@@ -234,6 +236,7 @@ module DbAdapters::MysqlAdapter
 
         delete_loaded_feature "ns/foo.rb"
         delete_loaded_feature "ns/bar.rb"
+        delete_loaded_feature "ns"
       end
 
       $test_eager_load_eager_loaded_p = false
@@ -243,7 +246,7 @@ module DbAdapters::MysqlAdapter
       ]
       with_files(files) do
         loader = new_loader(dirs: %w(lazylib eagerlib), enable_reloading: enable_reloading)
-        loader.do_not_eager_load('lazylib/namespace')
+        loader.do_not_eager_load('lazylib/ns')
         loader.eager_load
 
         assert $test_eager_load_eager_loaded_p

test/lib/zeitwerk/test_exceptions.rb

@@ -41,6 +41,7 @@ def assert_error_message(message, error)
     on_teardown do
       remove_const :CLI
       delete_loaded_feature 'cli/x.rb'
+      delete_loaded_feature 'cli'
     end
 
     files = [["cli/x.rb", "module CLI; X = 1; end"]]

test/lib/zeitwerk/test_reloading.rb

@@ -88,6 +88,18 @@ module Namespace; end
     end
   end
 
+  test "reloading works even if directories for managed namespaces get somehow pushed ot $LOADED_FEATURES" do
+    files = [["x/y.rb", "X::Y = true"]]
+    with_setup(files) do
+      assert X::Y
+      $LOADED_FEATURES << File.expand_path("x")
+
+      loader.reload
+
+      assert X::Y
+    end
+  end
+
   test "reloading raises if the flag is not set" do
     e = assert_raises(Zeitwerk::ReloadingDisabledError) do
       loader = Zeitwerk::Loader.new
@@ -108,6 +120,7 @@ module Namespace; end
 
       remove_const :Z
       delete_loaded_feature "z/a.rb"
+      delete_loaded_feature "z"
     end
 
     files = [
@@ -141,6 +154,7 @@ module Namespace; end
 
       remove_const :Z
       delete_loaded_feature "z/a.rb"
+      delete_loaded_feature "z"
     end
 
     files = [

test/lib/zeitwerk/test_require_interaction.rb

@@ -93,6 +93,14 @@ def assert_not_required(str)
     end
   end
 
+  test "autovivification pushes the directory of the implicit namespace to $LOADED_FEATURES" do
+    files = [["x/y.rb", "X::Y = true"]]
+    with_setup(files) do
+      assert X
+      assert_equal File.expand_path("x"), $LOADED_FEATURES[-1]
+    end
+  end
+
   test "files deep down the current visited level are recognized as managed (implicit)" do
     files = [["foo/bar/baz/zoo/woo.rb", "Foo::Bar::Baz::Zoo::Woo = 1"]]
     with_setup(files, load_path: ".") do

test/lib/zeitwerk/test_ruby_compatibility.rb

@@ -85,19 +85,6 @@ class C; end
     end
   end
 
-  # While unloading constants we leverage this property to avoid lookups in
-  # $LOADED_FEATURES for strings that we know are not going to be there.
-  test "directories are not included in $LOADED_FEATURES" do
-    with_files([]) do
-      FileUtils.mkdir("admin")
-      loader.push_dir(".")
-      loader.setup
-
-      assert Admin
-      assert !$LOADED_FEATURES.include?(File.expand_path("admin"))
-    end
-  end
-
   # We exploit this one to simplify the detection of explicit namespaces.
   #
   # Let's suppose `Admin` is an explicit namespace and scanning finds first a

test/lib/zeitwerk/test_unloadable_cpath.rb

@@ -50,6 +50,7 @@ def self.name
       remove_const :M
       delete_loaded_feature "m/x.rb"
       delete_loaded_feature "m/y.rb"
+      delete_loaded_feature "m"
 
       remove_const :Z
       delete_loaded_feature "z.rb"