EmilStenstrom / django-components

Create simple reusable template components in Django.
MIT License
1.15k stars 75 forks source link

If settings.BASE_DIR is a string, django-components errors when loading the templates #381

Closed alexgmin closed 8 months ago

alexgmin commented 8 months ago

In my settings file I had this as BASE_DIR BASE_DIR = os.path.dirname(os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) Which was later used for this in TEMPLATES: 'DIRS': [os.path.join(BASE_DIR, 'templates')],

When I added django-components to the project this error happened

  File "/foo/lib/python3.11/site-packages/django_components/template_loader.py", line 18, in get_dirs
    path = (settings.BASE_DIR / component_dir).resolve()
            ~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~
TypeError: unsupported operand type(s) for /: 'str' and 'str'

The solution is to change the BASE_DIRto BASE_DIR = Path(__file__).resolve().parent.parent.parent, which is what https://github.com/EmilStenstrom/django-components/issues/290 does in the example settings.

Since BASE_DIR is not an official Django setting, just something mentioned in some places of the documentation, the library documentation should explain what the setting is and what value is expected to have (a pathlib.Path instance)

EmilStenstrom commented 8 months ago

Hmm... maybe we should handle the case that it's a string too. I think wrapping BASE_DIR in Path is safe no matter if it's a Path or not from the start? Mind adding that as a PR?

alexgmin commented 8 months ago

I can confirm that wrapping a Path again is safe.

>>> foo = pathlib.Path('foo')
>>> bar = pathlib.Path(foo)
>>> foo == bar
True

I'll try to create a PR this week for the code. I'll also write a test. If someone else wants to do it instead I don't mind. However I think the readme should have a section about the BASE_DIR setting but I don't really know how to explain it.

spapas commented 8 months ago

I am also having the same problem. Please notice that for django_components==0.34.1 it works, it breaks only for django_components==0.35.

This is because of this line https://github.com/EmilStenstrom/django-components/compare/0.34.1...0.35#diff-c6c9a460aa194de49542b7c8e241152e7727fd8d1ea49f77c3437b0373427d24R17 .

My BASE_DIR is this BASE_DIR = os.path.dirname(os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) and I can't change it to be a Path because it's used in too many places!

Please provide a patch, thank you

EmilStenstrom commented 8 months ago

@spapas this is a hobby project for everyone here! If you have the bandwidth to wrap our Base_dir usage with Path() we will happily merge it.

spapas commented 8 months ago

@EmilStenstrom unfortunately not right now, I'm to pressured by other stuff... I've resolved the issue by staying on 0.34.1 for now.