EmilStenstrom / django-components

Create simple reusable template components in Django.
MIT License
973 stars 62 forks source link

Issue migrating to 0.5+ #413

Closed TreyWW closed 2 months ago

TreyWW commented 2 months ago

Hey,

I've been on v0.27 for a while and started having issues with new installations of the project (only ran the components when using terminal commands like migrate, but not on run server). But I can't get it to work on v0.61 i'm guessing due to the changes in 0.50

I've set all {% component %} blocks to have a {% endcomponent %} and removed all {%component_block%} tags. But I'm getting this error:

File "/project/components/components.py", line 42, in register_components
    component_class = type(
  File "/project/venv/lib/python3.10/site-packages/django_components/component.py", line 66, in __new__
    _resolve_component_relative_files(attrs)
  File "/project/venv/lib/python3.10/site-packages/django_components/component.py", line 77, in _resolve_component_relative_files
    module_name = attrs["__module__"]
KeyError: '__module__'

My code in my components.py (this file registers all of my custom "+abc.html" files into components on startup) has this

print(class_name, component_base_class, template_name)
component_class = type(
    class_name,
    (component_base_class,),
    {"template_name": template_name},
)
component_class = class_decorator(component_class)

An example print Modal <class 'components.SimpleComponent'> +modal.html

Has something been removed which means my script will no longer work? (full script)

Thanks

JuroOravec commented 2 months ago

Hey @TreyWW, the addition of _resolve_component_relative_files was my doing, it was to enable us to refer to template and media files relatively to the component class.

My guess is that, based on your snippet, it seems like the error occurs because the component class is being created dynamically, and therefore __module__ is not populated.

Could you try manually adding __module__ to the third argument like below?

print(class_name, component_base_class, template_name)
component_class = type(
    class_name,
    (component_base_class,),
    {"template_name": template_name, "__module__": "<python.path.to.file.py>"},
)
component_class = class_decorator(component_class)

Moreover, looking at the full script, it looks unconventional. Could you describe in more details what's the purpose of the script?

The reason is, if the only purpose of the script is to avoid the boilerplate code for the components, then in I think 0.32, the support for single-file components was added.

With single-file components, you don't need separate files for Python and HTML files, so that could be a good replacement for the your script. And that would also resolve your issue, as the classes would not be defined dynamically, so the __module__ value would be populated automatically with the python path to the file where the component is defined.

TreyWW commented 2 months ago

Hey @JuroOravec, thanks for the reply back.

Could you describe in more details what's the purpose of the script?

Basically I want to have components that are in files, so lets say modal.html but prefixed, like +modal.html to symbolise that it's a component. Because I never needed the feature of backend code per component it seemed an annoyance to manually make a class every time I add a component. So this script was aimed to just fill them in for me. Scan all my files, find ones with the prefix of + and add to components.

With single-file components, you don't need separate files for Python and HTML files, so that could be a good replacement for the your script.

But it still needs the python part right? I just want the HTML part.

I'll give the module approach an attempt now though, thanks! I'll let you know if it works.

TreyWW commented 2 months ago

🤦🏽 I can't reproduce this now. I've upgraded to 0.61 but now my components.py file isn't getting read at all which is odd because earlier it was (hence why I got the errors). It goes straight to

Performing system checks...

[DEBUG] Template loader will search for valid template dirs from following options:
 - /media/trey/projects/MyFinances/frontend/static
2024-03-29 21:25:00,967 django_components [DEBUG] Template loader will search for valid template dirs from following options:
 - /media/trey/projects/MyFinances/frontend/static
[DEBUG] Template loader matched following template dirs:
 - /media/trey/projects/MyFinances/frontend/static
2024-03-29 21:25:00,968 django_components [DEBUG] Template loader matched following template dirs:
 - /media/trey/projects/MyFinances/frontend/static
System check identified no issues (0 silenced).

And then when I try and visit the site iget a "Component "x" is not registered". Not too sure why it's not reading my components.py file though

TreyWW commented 2 months ago

Aha, downgraded to 0.50 and now it's getting read again.. I'm going to take a look at the changelog to see if something was removed when reading components.py

Edit: Yeah i'm not too sure.. Updating to 0.50 seemed to at least work and all I had to do was change the template blocks from _block to just component and add endcomponent tags. So that's a start at least.

Though I agree, this script shouldn't be necessary, it kind of feels like it is unnecessary and I have no idea how it even works now so it's not really something i'd like to say is production ready lol.

This originally came from my discussion: https://github.com/EmilStenstrom/django-components/discussions/333

But i'd be happy to hear alternatives because I've kind of turned this into a huge mess

JuroOravec commented 2 months ago

Good to know that 0.50 is working. The changes surround the component imports were made in v0.60.

What's does your settings.py look like? Have a look at how the demo project is made https://github.com/EmilStenstrom/django-components/blob/master/sampleproject/sampleproject/settings.py#L130. There, in the settings, there's

STATICFILES_DIRS = [BASE_DIR / "components"]

This is what tells django-components where to look for components.

Try adding the directory where components.py is to the STATICFILES_DIRS.

TreyWW commented 2 months ago

Ah yeah that's probably the issue, I removed components from staticfiles dirs for some reason. I'll give that another shot tomorrow then on the latest version, hopefully it works!

Thanks, your help has been much appreciated

JuroOravec commented 2 months ago

Good luck!

I just had a read through the discussion in https://github.com/EmilStenstrom/django-components/discussions/333.

From that discussion, it feels like you wish to achieve 3 things simultaneously:

  1. Define components in multiple locations
  2. Import components in {% component %} block by specifying the component path.
  3. Have clear distinction between global and "local" components

As for the first point, I plan to do that in my project, but it's low in priority. But I think it should be possible with the STATICFILES_DIRS as discussed above.

As for the second point, that's an interesting idea. Though I'd argue that using unique component names would be more maintainable approach. Because consider it in the context of e.g. VSCode. With their Python language support, when I move a python file, VSCode updates the import paths for me. But if you imported components via import path, and you moved the component file, you'd need to remember to fix it yourself.

As for third point, might be relevant to see https://github.com/EmilStenstrom/django-components/issues/265? I haven't read that one though, so just guessing.

As for naming components, what I do in my project is that:

  1. UI components (my "globals") are unprefixed and named after the material design component names, e.g. dialog, button, etc.
  2. "Business logic" components (my "locals") are prefixed with group/module that describes it. E.g. if I had a shopping cart page, the page-specific components would be called cart_list cart_list_item, etc.
EmilStenstrom commented 2 months ago

I'm closing this thread as it seems there's no error here one django-components parts, just internal changes that broke a script.