Merge pull request #435 from JuroOravec/431-fix-autodiscovery-name-clash

refactor: fix clash with autodiscovery

README.md

@@ -221,6 +221,19 @@ class Calendar(component.Component):
 
 And voilá!! We've created our first component.
 
+## Autodiscovery
+
+By default, the Python files in the `components` app are auto-imported in order to auto-register the components (e.g. `components/button/button.py`).
+
+Autodiscovery occurs when Django is loaded, during the `ready` hook of the `apps.py` file.
+
+If you are using autodiscovery, keep a few points in mind:
+- Avoid defining any logic on the module-level inside the `components` dir, that you would not want to run anyway.
+- Components inside the auto-imported files still need to be registered with `@component.register()`
+- Auto-imported component files must be valid Python modules, they must use suffix `.py`, and module name should follow [PEP-8](https://peps.python.org/pep-0008/#package-and-module-names).
+
+Autodiscovery can be disabled via in the [settings](#disable-autodiscovery).
+
 ## Use the component in a template
 
 First load the `component_tags` tag library, then use the `component_[js/css]_dependencies` and `component` tags to render the component to the page.

src/django_components/__init__.py

@@ -1,12 +1,13 @@
 import importlib
 import importlib.util
-import sys
+import os
 from pathlib import Path
-from typing import Union
 
 import django
+from django.conf import settings
 from django.utils.module_loading import autodiscover_modules
 
+from django_components.logger import logger
 from django_components.utils import search
 
 if django.VERSION < (3, 2):
@@ -21,20 +22,42 @@ def autodiscover() -> None:
         autodiscover_modules("components")
 
         # Autodetect a <component>.py file in a components dir
-        component_filepaths = search(search_glob="**/*.py")
+        component_filepaths = search(search_glob="**/*.py").matched_files
+        logger.debug(f"Autodiscover found {len(component_filepaths)} files in component directories.")
+
         for path in component_filepaths:
-            import_file(path)
+            # This imports the file and runs it's code. So if the file defines any
+            # django components, they will be registered.
+            module_name = _filepath_to_python_module(path)
+            logger.debug(f'Importing module "{module_name}" (derived from path "{path}")')
+            importlib.import_module(module_name)
 
     for path_lib in app_settings.LIBRARIES:
         importlib.import_module(path_lib)
 
 
-def import_file(path: Union[str, Path]) -> None:
-    MODULE_PATH = path
-    MODULE_NAME = Path(path).stem
-    spec = importlib.util.spec_from_file_location(MODULE_NAME, MODULE_PATH)
-    if spec is None:
-        raise ValueError(f"Cannot import file '{path}' - invalid path")
-    module = importlib.util.module_from_spec(spec)
-    sys.modules[spec.name] = module
-    spec.loader.exec_module(module)  # type: ignore
+def _filepath_to_python_module(file_path: Path) -> str:
+    """
+    Derive python import path from the filesystem path.
+
+    Example:
+    - If project root is `/path/to/project`
+    - And file_path is `/path/to/project/app/components/mycomp.py`
+    - Then the path relative to project root is `app/components/mycomp.py`
+    - Which we then turn into python import path `app.components.mycomp`
+    """
+    if hasattr(settings, "BASE_DIR"):
+        project_root = str(settings.BASE_DIR)
+    else:
+        # Fallback for getting the root dir, see https://stackoverflow.com/a/16413955/9788634
+        project_root = os.path.abspath(os.path.dirname(__name__))
+
+    rel_path = os.path.relpath(file_path, start=project_root)
+    rel_path_without_suffix = str(Path(rel_path).with_suffix(""))
+
+    # NOTE: Path normalizes paths to use `/` as separator, while os.path
+    # uses `os.path.sep`.
+    sep = os.path.sep if os.path.sep in rel_path_without_suffix else "/"
+    module_name = rel_path_without_suffix.replace(sep, ".")
+
+    return module_name

src/django_components/component.py

@@ -89,7 +89,7 @@ def _resolve_component_relative_files(attrs: MutableMapping) -> None:
 
     # Prepare all possible directories we need to check when searching for
     # component's template and media files
-    components_dirs = search()
+    components_dirs = search().searched_dirs
 
     # Get the directory where the component class is defined
     try:

src/django_components/utils.py

@@ -1,13 +1,18 @@
 import glob
 from pathlib import Path
-from typing import List, Optional, Union
+from typing import List, NamedTuple, Optional
 
 from django.template.engine import Engine
 
 from django_components.template_loader import Loader
 
 
-def search(search_glob: Optional[str] = None, engine: Optional[Engine] = None) -> Union[List[str], List[Path]]:
+class SearchResult(NamedTuple):
+    searched_dirs: List[Path]
+    matched_files: List[Path]
+
+
+def search(search_glob: Optional[str] = None, engine: Optional[Engine] = None) -> SearchResult:
     """
     Search for directories that may contain components.
 
@@ -22,11 +27,11 @@ def search(search_glob: Optional[str] = None, engine: Optional[Engine] = None) -
     dirs = loader.get_dirs()
 
     if search_glob is None:
-        return dirs
+        return SearchResult(searched_dirs=dirs, matched_files=[])
 
-    component_filenames: List[str] = []
+    component_filenames: List[Path] = []
     for directory in dirs:
         for path in glob.iglob(str(Path(directory) / search_glob), recursive=True):
-            component_filenames.append(path)
+            component_filenames.append(Path(path))
 
-    return component_filenames
+    return SearchResult(searched_dirs=dirs, matched_files=component_filenames)

tests/test_autodiscover.py

@@ -1,4 +1,5 @@
 from pathlib import Path
+from unittest import mock
 
 from django.template.engine import Engine
 from django.urls import include, path
@@ -9,7 +10,7 @@ from .testutils import Django30CompatibleSimpleTestCase as SimpleTestCase
 
 # isort: on
 
-from django_components import autodiscover, component
+from django_components import _filepath_to_python_module, autodiscover, component, component_registry
 from django_components.template_loader import Loader
 
 urlpatterns = [
@@ -25,11 +26,17 @@ class TestAutodiscover(SimpleTestCase):
         del settings.SETTINGS_MODULE  # noqa
 
     def test_autodiscover_with_components_as_views(self):
+        all_components_before = component_registry.registry.all().copy()
+
         try:
             autodiscover()
         except component.AlreadyRegistered:
             self.fail("Autodiscover should not raise AlreadyRegistered exception")
 
+        all_components_after = component_registry.registry.all().copy()
+        imported_components_count = len(all_components_after) - len(all_components_before)
+        self.assertEqual(imported_components_count, 1)
+
 
 class TestLoaderSettingsModule(SimpleTestCase):
     def tearDown(self) -> None:
@@ -116,3 +123,38 @@ class TestBaseDir(SimpleTestCase):
             Path(__file__).parent.resolve() / "test_structures" / "test_structure_1" / "components",
         ]
         self.assertEqual(sorted(dirs), sorted(expected))
+
+
+class TestFilepathToPythonModule(SimpleTestCase):
+    def test_prepares_path(self):
+        self.assertEqual(
+            _filepath_to_python_module(Path("tests.py")),
+            "tests",
+        )
+        self.assertEqual(
+            _filepath_to_python_module(Path("tests/components/relative_file/relative_file.py")),
+            "tests.components.relative_file.relative_file",
+        )
+
+    def test_handles_nonlinux_paths(self):
+        with mock.patch("os.path.sep", new="//"):
+            self.assertEqual(
+                _filepath_to_python_module(Path("tests.py")),
+                "tests",
+            )
+
+            self.assertEqual(
+                _filepath_to_python_module(Path("tests//components//relative_file//relative_file.py")),
+                "tests.components.relative_file.relative_file",
+            )
+
+        with mock.patch("os.path.sep", new="\\"):
+            self.assertEqual(
+                _filepath_to_python_module(Path("tests.py")),
+                "tests",
+            )
+
+            self.assertEqual(
+                _filepath_to_python_module(Path("tests\\components\\relative_file\\relative_file.py")),
+                "tests.components.relative_file.relative_file",
+            )