EmilStenstrom / django-components

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

[Opinions Needed] Namespacing components / increasing reusability #265

Closed lemontheme closed 1 month ago

lemontheme commented 1 year ago

Background

Today (anno v0.27), registered Components are kept in a single global registry. The value passed to the name parameter of component.register() serves as a unique identifier for the Component. Accidental naming collisions are prevented by raising an exception if the same name value is passed in again, meaning a Component is already registered under that name.

This works fine if your direct code base is the only place where components are potentially being registered. In the event of a name conflict, the solution is as easy as assigning a different name to one of your components.

But now consider one of Django's big selling points: reusable applications. Applications are 'installable': they can be swapped in and out, and – if all goes well – they get along with each other. Imagine you've built an 'app' containing reusable generic components, e.g. with tailwindcss and alpine.js, and now you'd like to install that app into your other projects – perhaps even making it open-source and sharing it with others, too. What guarantees do you actually have that the names you've used for your generic components won't clash with the names of components in other projects?

I've been thinking about possible strategies to provide better guarantees about the reusability of components. All of them, implictly or explicitly, revolve around the idea of grouping components into separate collections, or 'namespaces', that provide a higher-level address space and let us disambiguate between possibly identically named components.

But there are quite a few possibilities here. I'd like to gather some opinions before proceeding with an implementation. Any and all feedback is very welcome.

Strategies

1. Discipline and allowing /s in the component name

Make it best practice to add some kind of app-specific prefix to your component names. Seeing as file paths are a natural analog for namespaces, an (unenforced) convention could be to use / separators.

Here's an example. Instead of this –

@component.register("form")  # TOO GENERAL
class BasicForm(component.Component):
    ...

– do this:

@component.register("calendarapp/form")
class BasicForm(component.Component):
    ...

Then, in your templates, call the component with its fully qualified name:

{% component "calendarapp/form" %}

Pros:

Cons:

Note: Component authors would need to ensure that their component directories follow the same structure that Django advises for template and static directories, i.e. components/<app>/<component>/<component>.{html, py, ...}. Else, registered components might fetch their template or media from the wrong place.

Note: #262 asks about about allowing / in component names – perhaps for similar reasons?

2. Add (optional) namespace param to register function.

This is similar to the first strategy, except now the namespace is an actual parameter of the register function, making the goal more explicit.

Example:

@component.register("form", namespace="calendarapp")   # or ns="calendar"
class BasicForm(component.Component):
    ...

When including the component, a forward slash (/) can be treated as a special character, separating namespaces from component name, allowing usages such as:

{% component "calendarapp/form" %}

Alternatively, we could use a colon (:) as the separator – which I've seen used in Django for URLs – giving e.g.:

{% component "calendarapp:form" %}

Pros:

Cons:

3. The register function accepts a tuple as its name argument.

Rather than using a keyword argument as in approach (2), we can allow the namespace → component path to be expressed with a tuple.

Example:

@component.register(("calendarapp", "form"))
class BasicForm(component.Component):
    ...

Invoking the component inside the template would happen similarly to in (2).

Pros:

Cons:

4. Sub-registries

In addition to the global register, accessible via component.register(), make it possible to create sub-registries and register components with those. Sub-registries serve as a namespace. New sub-registries can be created from existing sub-registries, allowing complexer namespace paths. Behind the scenes, sub-registries get linked with the global registry.

Usage example:

# in app/__init__.py
import django_components

calendarapp_components = django_components.create_registry("calendarapp")

# in components/componentA/componentA.py

from . import calendar_app_components

@calendarapp_components.register("form")
class BasicForm(component.Component):
    ...

Invoking the component inside the template would happen the same way as in approaches (2) and (3)

Pros:

Cons:

Closing thoughts

EmilStenstrom commented 1 year ago

Great points, and as always an interesting read of the different options. I think this is a problem worth solving.

Quick thought: Let's say there exists two reusable apps, both with a component with the name "component", and you want to use both. Since you have control of none of the two apps (you just install them), does that not mean that you have no control over the call to @components.register? If so, the fix should not be there, but when the user of an installable app "mounts" their components.

This is how urls.py work, you import urlpatterns from an app, but you can set your own url name when you mount that app in your own url structure. Not sure what mounting would amount to for django-components, but maybe you have an idea?

lemontheme commented 1 year ago

Glad you're on board!

So, approaches (1) through (3) work on the assumption that the onus is on the provider of the app to get it right, not its consumer. This is similar to why the official docs recommend the _templates//template.html_ and _static//js/index.js_ folder structures.

If we continue the comparison with templates and static files, both of these are 'found and loaded' when needed. At the moment, components have a similar way of springing into existence. They, too, are 'found' – their modules imported, e.g, during the autodiscovery step.

By contrast, URLs follow more the 'pulled into one place' pattern, either by importing views or using include(). I suppose we could have something like this:

# in components/<appname>/__init__.py
from .component_a import ComponentA
from .component_b import ComponentB
components = [
    ComponentA,
    ComponentB,
    include("awesome_tailwindcss_djcomps", namespace="twdcss")
]

# in project/components.py
components = [
    include("appname", namespace="appname")
]

(Frankly, I don't like this very much, although this is of course just a first design attempt.)

Each application would technically have the ability to define their own set of namespace–components associations. That would complicate namespace resolution, but I believe it's doable. Also, barring inspecting Components' __module__, __qualname__, etc., each would still need to maintain its own separate registry in order for name resolution to work.

timothyis commented 1 year ago

Love the thoughts here. As per usual, I can't comment on the implementation because I'm not a Python dev, but if it helps, I like the way that you can do this in React when you can export multiple parts of a component from a file, and then use ComponentName.Part as the component in JSX.

e.g., you export Root and Item from a component file, and then wherever you want to use those, you can do

{% component_block "DropdownMenu.Root" %}
    {% fill items %}
        {% component_block "DropdownMenu.Item" %}
            {% fill value %}
                Some menu item value
            {% endfill value %}
        {% endcomponent_block %}
    {% endfill items %}
{% endcomponent_block %}

I think using a . rather than / makes it feel as if it's part of the same component.

For what it's worth also; we currently have a few components that are within one directory, and being able to define multiple components like this in one file would make this much better to navigate in our codebase. (e.g., dialogs/dialog-header/dialog-body, dropdown-menu/dropdown-item, etc.)

Just again, throwing ideas out 😅

lemontheme commented 1 year ago

Thanks for chiming in. Keep the ideas coming! Appreciate you sharing ideas from other frameworks/ecosystems.

Your example sort of reminds me of something I've been working on. It's still very much in the early design phase. Also, I just found out I forgot to stash before git resetting, so this is from memory. :')

I call them 'component packages'. Their current design seems like a pretty radical overhaul. It's not meant to be. Bit by bit, I'm trying to figure out how/if we could introduce them without breaking stuff. The idea is to reduce boilerplate in the 80% of cases where you just want a simple component. No register decorators, no naming components, no providing paths to templates or other media: none of that's needed – although it remains available.

The layout of a component package looks a bit like this. We'll start with the class definition.

class _(BaseComponent):  # If name == "_", inherit name from immediate package

    def get_context_data(self, date):
        return {
            "date": date,
        }

    class Config:  # This is new.
        auto = (  # By default all of these are disabled. Subclass to avoid repetition.
            "name",  # Infer name from class name
            "namespace",  # Use file path below components/ to determine namespace.
            "template",  # Automatically find template.html (in same dir)
            "media"  # Automatically find js and css (in same dir)
            "register"  # If discovered at startup, add to global registry.
        )

The module containing the component is the __init__.py of a regular Python package. One module can in principle contain multiple components (to tie back to the React case in you example), although lets say for now 1 package = 1 component.

Depending on which 'auto' flags are provided, component dependencies are findable by virtue of the fact that they occur within the same package.

Components are organized according to the following file hierarchy:

.
└── components
    └── appname
        ├── dropdown_menu
        │   ├── __init__.py
        │   ├── index.js
        │   ├── style.css
        │   └── template.html
        └── menubar
            ├── __init__.py
            ├── index.js
            ├── style.css
            └── template.html

Note: I don't repeat the component name in the template filename and media filenenames, since in my experience it makes renaming components pretty tedious. In the proposed design, renaming a component simply means renaming the containing package. magic.gif (However, it's easy enough to search for media files based on filename extension as well.)

To use the component, I figured . notation would work best (again tying back to your example ;) ).

{% component "appname.dropdown" %}

I suppose if you have multiple components in the same __init__.py, one could do:

{% component "appname.dropdown_menu.Item" %}

At any rate, you get the idea. It's typical Python/JS syntax for navigating through packages to modules to symbols.

Main challenge atm is to make component discovery + registration work in such a way that can be carefully controlled under unit testing conditions.

An additional, possibly contentious point is that for the sake of consistency all names along the naming path should be valid python identifiers – i.e., comprising nothing but alphanumerics and underscores.


Anyway, just wanted to share that, as the . separator reminded me of it. But by the looks of it, the usage of that separator in React is slightly different – less about digging down through packages, more about individual component files.

What would the component path in your example look like if your component file were located further down in a sub-package?

Also, just to be clear, the problem in OP is much more incremental than the design proposed in this comment. I definitely see it as a potential step in that direction though.

EmilStenstrom commented 1 year ago

So, approaches (1) through (3) work on the assumption that the onus is on the provider of the app to get it right, not its consumer.

Since providers likely don't coordinate their namespaces, what stops them from picking the same namespace accidentally?

I wonder if one way to solve this could be a simple mapping setting, where you could remap a name to a new one, if you need to?

EmilStenstrom commented 1 year ago

I really like the idea of component packages! I can see a future when someone bundles django-components for a whole front-end library as a django app, that you can simply pip install and add to INSTALLED_APPS to get access to a big library of components!

I'm not that fond of the auto flags though. It breaks the possibility to look at the code and understand how it works, by adding indirection. Does saving a few keystrokes when you write a component (which you likely only do once?) really require adding a whole auto system?

timothyis commented 1 year ago

I definitely like the idea of component packages too!

What would the component path in your example look like if your component file were located further down in a sub-package?

I'm not overly sure, as I'm still not 100% used to the Django way of things. But currently, I'm using something like:

components/
    dropdown/
        dropdown-root/
            dropdown-root.py
            dropdown-root.html
            dropdown-root.js
        dropdown-item/
            dropdown-item.py
            dropdown-item.html

But I think it would be better if it were somehow combined, but as above, the code would control how it's "exported" for use

components/
    dropdown/
        dropdown.py
        templates/
            root.html
            item.html
        scripts/
            dropdown.js

Not sure if I'm making it too complex here, but I really like the granular filesystems that a lot of JS frameworks have these days and have been trying to replicate that in Django a bit recently. E.g., trying to have a components directory in each Django app directory if they're used solely in that app.

I guess with the example I just gave, you could do some cool automatic inference of what to use for a component. Like, dropdown.js would load with any usage of any component in the dropdown component package (I think that's correct?). Or if you used root.js, it would load with the Dropdown.Root component usage.

Maybe this is stupid. It is late and I'm letting my thoughts run out as they come 😂

Basically, I think some filesystem based magic would be amazing in some way.

lemontheme commented 1 year ago

What stops them from picking the same namespace accidentally.

That's the neat part. Nothing.

Which sounds terrible until you realize that that's how all installed apps work. AppConfig's name attribute is the only thing that links a Django app to a string in INSTALLED_APPS. Similarly, without the templates/\<appname>/\<template>.html pattern, an app would have a terrible time locating its 'base.html' among tens of other similarly named template files. The same goes for static files.

So the only thing standing in the way of chaos is convention and due diligence. But it would appear that the Django community has accepted this as common practice.

a simple mapping setting

Provided the components are registered with an explicit namespace – meaning info about the namespace is available in the global component registry – then, yes, such a mapping would absolutely work. Where to specify the mapping is a different question, however. The reflex might be to shout "settings.py!", but that's a property of projects, not applications – and so that might again impact individual app reusability.

lemontheme commented 1 year ago

[...] a future when someone bundles django-components for a whole front-end library as a django app, that you can simply pip install and add to INSTALLED_APPS to get access to a big library of components!

Yes! That's exactly what I'd love us to be able to facilitate.

I'm also torn about the 'auto' feature. I know the pain of reasoning about code that's too magical (and, yes, the 'auto' design would rely on a fair bit of metaclass-enabled indirection, so... case in point.). Then again, I think saving keystrokes is something people care about – witness the popularity of projects like attrs (and, relatedly, the introduction of dataclasses), pydantic, and fastapi.

I should add by the way that all 'auto'-found properties can be overriden, as shown below. I'll admit though that it might still be strange having to learn a bunch of 'auto' flags.

class _(BaseComponent): 

    name = "actual_component_name"

    def get_context_data(self, date):
        return {
            "date": date,
        }

    class Config:  # This is new.
        auto = (  
            "name",  # does nothing because cls.name is specified
            "namespace",  
            ...
        )

An alternative design I've just thought up could be to put the 'auto' mechanism in a type hint or something... or a special type of class – kind of like auto() in enum.Enum – e.g.:

class _(BaseComponent): 

    name = component.auto()
    namespace = component.auto()

    def get_context_data(self, date):
        return {
            "date": date,
        }

The whole thing's a balancing act, really. On the one hand, users shouldn't have to compromise on the level of control they already have today. On the other, I think for many this is madness:

@component.register("calendar")  # okay: 'calendar'
class Calendar(component.Component):  # yes, i heard you the first time. this is 'calendar'
    template_name = "calendar/calendar.html"  # I get it already. It's 'calendar'.
    class Media:
        css = "calendar/calendar.css"  # fuck's sake, why am i still typing 'calendar'
        js = "calendar/calendar.js"  # arghhhhhhh

The above is intentionally tongue-in-cheek. I mean no disrespect. The current design is explicit and easy-to-read, but refactoring is a PITA. Compare with:

# Shared by all components in lazy developer's app.
class MagicalBaseComponent(Component):
    name = auto()
    template_name = auto()
    namespace = auto()
    class Media:
        js = auto()
        css = auto()

# in calendar/__init__.py
from . import MagicalBaseComponent

class _(MagicalBaseComponent):
    def get_context_data(self, date):
        return {
            "date": date,
        }

That's still pretty explicit. To see which properties are magical, just read the parent class. Want to change the name of the component? Just change the name of the package. However, all attributes can still be overriden, at which point auto() is disabled for that field.

timothyis commented 1 year ago
# Shared by all components in lazy developer's app.
class MagicalBaseComponent(Component):
    name = auto()
    template_name = auto()
    namespace = auto()
    class Media:
        js = auto()
        css = auto()

I think this is less preferable than having some filesystem naming conventions that allow automatic setting up of components in this way (personally.)

lemontheme commented 1 year ago

I get what you're saying. It's something I really like about the few JS frameworks I've studied.

But I'm just not sure how well such an approach would be received in Django land, where explicitness ranks highly on the desiderata list and a common pattern is 'subclass this generic class to get your specific desired functionality'.

Hence this compromise. Users have complete control over which parts of their components behave automagically. By default, they are just plain old components, 100% compatible with what we have today. Components with 'magical powers' can live alongside these standard components without any problems – except only the 'magical' class will get special treatment by the component definition and the component registration machinery.

I believe this should jive better with the overall Django ethos – although I'm personally still so new to Django that I may be very wrong!

In any case, what I want to avoid is a design that doesn't resemble regular Django code at all anymore. As an example of such design, I'd point to https://www.tetraframework.com/. From the examples it seems so different from regular Django that I lack the imagination to see how I'd incorporate it into an existing project. (That isn't to say it's not a cool project. The docs look really nice and they seem to have a lot of cool ideas.)

VojtechPetru commented 1 year ago

Just quickly jumping in to say that I really like the idea of component packages. On the other hand, I'm not a big fan of (2) (namespace argument) because it would allow to have only single namespace, which is IMHO not general enough.

EmilStenstrom commented 1 year ago

I'm also torn about the 'auto' feature. I know the pain of reasoning about code that's too magical (and, yes, the 'auto' design would rely on a fair bit of metaclass-enabled indirection, so... case in point.). Then again, I think saving keystrokes is something people care about – witness the popularity of projects like attrs (and, relatedly, the introduction of dataclasses), pydantic, and fastapi.

A test I try to put my code through: "Would a developer that know nothing about django-components understand what this code is doing?". Let's call it the "First day on the job-test". Stuff that's too magical usually fail this test, not because they are magic, but because the magic usually hides what's going on to the developer seeing this for the first time.

Looking at the three examples you've given:

class MagicalBaseComponent(Component):
    name = auto()
    template_name = auto()
    namespace = auto()
    class Media:
        js = auto()
        css = auto()

# in calendar/__init__.py
from . import MagicalBaseComponent

class _(MagicalBaseComponent):
    def get_context_data(self, date):
        return {
            "date": date,
        }

I don't think this setup passes the "First day on the job-test". I think it might go like this:

The auto() changes proposed break the link between the template and python files, and the python file and the js file. Instead you need to look at where the template file is, and infer the js file from there instead. While I agree that this isn't terrible, it's not how the rest of Django works. Usually you can go from the urls.py -> view -> template -> component, all with explicit paths to grep for when you jump between the steps.

@component.register("calendar")  # okay: 'calendar'
class Calendar(component.Component):  # yes, i heard you the first time. this is 'calendar'
    template_name = "calendar/calendar.html"  # I get it already. It's 'calendar'.
    class Media:
        css = "calendar/calendar.css"  # fuck's sake, why am i still typing 'calendar'
        js = "calendar/calendar.js"  # arghhhhhhh

    def get_context_data(self, date):
        return {
            "date": date,
        }

The above is intentionally tongue-in-cheek. I mean no disrespect. The current design is explicit and easy-to-read, but refactoring is a PITA. Compare with:

No offence taken! :) There's been two discussions related to decreasing the typing, while keeping the same explicitness:

Finally, about refactoring: Compare the task "adding another js file to a component" in the two examples:

js = "calendar/calendar.js" -> js = ["calendar/calendar.js", "calendar/helper.js"]

In the auto() example:

Overall, I think would be a decrease in developer productivity. Happy to hear you thoughts.

EmilStenstrom commented 1 year ago

About namespaces: I still think relying on providers to pick names that don't crash with anyone else is putting a lot of responsibility on them. Especially when they should also avoid clashing with local components which could use any name possible.

Good point that installed apps use the same system. That system seems to work so-so though, looking at stackoverflow where people try to use two apps with the same "reviews" name. The recommended way is to just rename your local app to avoid clashing with the installed one. Which is SUPER annoying, since database tables use the app name by default, so you're into a world of hurt trying to rename them... but I digress...

Being able to specify a namespace when using a component pack sidesteps this very nicely. How to do this is not as easy to give examples of...

Today components are discovered through installed_apps. So if you would pip install a component package, it would be autodiscover:ed. Maybe there could be a setting in COMPONENTS, maybe by reusing the autodiscover key?

COMPONENTS = {
    "autodiscover": {
        "calendar": "tailwind-calendar",
    }
}

You would only do this if there was an actual clash, so rarely.

lemontheme commented 1 year ago

Thanks for all the feedback, guys! I have a good feeling we're going to be able to converge on a design that's both ergonomic and sensible.

@VojtechPetru, I see what you're saying. Do either of the two variations on (2) below address your concerns?

@component.register("form", namespace=("calendarapp", "forms"))   # `namespace` also accepts a tuple
class BasicForm(component.Component):
    ...

or

@component.register("form", namespace="calendarapp.forms")   # use separators in string (`.` or `/` etc.)
class BasicForm(component.Component):
    ...

Sticking to the theme of namespaces (I'll come back to component packages in the next comment) –

@EmilStenstrom, I think such a mapping is an excellent last-resort type of measure. Keep in mind thought that the recommended best practice will be for the namespace root to mirror the name of the application. In that case, if there's a namespace clash between two installed apps, then so too will there be a clash between the names of the apps themselves.

The fact remains that it's a more general problem, for which creative solutions exist (SO post). I haven't checked if they work, but an alternative could be to ship a shorthand function that solves this more general problem.

Either way, in order for any of this to work, there must exist some mechanism that associates items in the global registry with individual applications. That obviously doesn't happen at the moment – everything ends up in the same 'bucket' – so this is still something we need to figure out.

Repeating some of the points in OP, we can:

Any personal preference? :)

lemontheme commented 1 year ago

Returning to the discussion about 'component packages', I think you've touched on a really good heuristic for evaluating design decisions. Related is the 'zero hours of sleep' test, which is particularly relevant for me personally.

Now, I think it's fair to say that all libraries have some learning curve. My experience is that it might sometimes take a while to get up to speed, but improved ergonomics tends to pay off in the long run. Getting to grips with the abstractions in any of the big JS frameworks definitely takes more than a few days. That makes sense if you're certain it's worth the time investment. Conversely, for smaller projects like ours, it's perhaps best to get new users off the ground as fast as possible – i.e. with self-explanatory abstractions and minimal doc diving. Also, it's easier to learn new things if they behave the same way as what you're used to.

In short: I agree with you. ;)

I can offer a number of rebuttals for your arguments against auto(), but they don't address your central point that components should preserve some clear path to the template/media files they depend on.

Let's build on that idea then.

What we want:

Ideas:

  1. Introduce some kind of shorthand function that takes a filename and resolves it relative to the directory of the component module #[1]
  2. Just use relative file paths #[2] (This can be easily made to work, if it doesn't already.)
  3. Add special attribute flag that changes how file paths are resolved (I don't like this one personally). #[3]
@component.register("calendar")
class Calendar(component.Component):
    find_resources_in_pkg = True  #[3]

    template_name = component.pkg_file("calendar.html")  #[1]  # or find_in_pkg
    class Media:
        css = "./calendar.css"  #[2]
        js = "calendar/calendar.js"

Then, if I wanted to follow my own file dependency naming scheme (e.g. template.html, style.css, index.js, etc.), I could do the following (using #[2]):

class MyComponent(component.Component):

    template_name = component.pkg_file("template.html")  #[1] 
    class Media:
        css = component.pkg_file("style.css")
        js = component.pkg_file("index.js")

# ---- Later -----

@component.register("component_a")
class Component(MyComponent):
    ...

The registered component subclass would know to look for template/media files in the same subdirectory/subpackage that holds the component module and would check for files named according to my own preferences. (In other words: no implicit magic.)

To save on a little more typing, I'd additionally make it optional to pass a name arg to component.register(). Instead, the registered name is taken from the class name. This is shown below.

@component.register
class Calendar(component.Component):
    ...

Side note/nit pick: Today, one usually does from django_component import component. Nearly all functionality is accessed via the component module. Personally, I find this a bit strange, particularly when using component.register(). I find it hard to explain exactly why. In part it's because it reads like an obj.method() call, but the obj 'component' is singular instead of plural (which is what you'd expect if you're registering something with a collection). Of course, I know this interpretation is incorrect, but that's how my brain reads it. Then there's the fact that the decorated class inherits from Component. If Component were ever instantiated (which it rarely is), convention dictates an instance of it would be assigned to a variable such as (lowercase) component. So here 'component' would have a different meaning. I know, this is some serious bike shedding – but might it not be nicer if the main functionality were made a top-level import, allowing either of the following:

import django_components as comps

@comps.register()
class ComponentA(comps.Component):
     ...

or

from django_components import register, Component

@register()
class ComponentA(comps.Component):
    ...

So, pretending it's our first day on the job, would the ideas in this comment help us get up to speed slower or about as fast as before? And then, switching to a long-term perspective, would we be happier maintaining code written in the present style or incorporating the design ideas discussed here?

EmilStenstrom commented 1 year ago

Would this be an option?

from django_components import component

@component.register("calendar")
class Calendar(component.Component):
    template_name = "template.html"
    class Media:
        css = "style.css"
        js = "index.js"

That is: We automatically look for files in the component directory we're in. We're only saying "calendar" twice here, but I think that makes sense since the class name is uppercase and the component name is not. When you look at the version with implicit name, to you map {% component "calendar" %} to class Calendar? I think that's slightly unintuitive. Also what about MyCell, should that me "mycell"? That's not at all as readable. So should we do my_cell when we convert to lowercase?

About the import path:

Slight negative of the last version: When you've scrolled down (say) 1000 lines in a long compnoent file. Will you know when register does? component.register makes it clear that the decorator is registering a component. Just register does not, and you have to guess from the inherited class name on the line below.

lemontheme commented 1 year ago

I think we need to be careful about resolving paths relative to the component sub-directory by default. I'd personally advise against it.

First, it breaks with Django convention, which assumes that file paths resolve to one of several possible global root directories – e.g. static paths are resolved relative to the first valid static dir; template paths, to a DIR in the TEMPLATES setting; user-uploaded media, to some media root, etc.

Okay, but let's suppose blind adherence to convention doesn't count. We still end up with two problems:

@component.register("calendar")
class Calendar(component.Component):
    template_name = "template.html"
    class Media:
        css = "style.css"
        js = "../other_component/index.js"

As for repeating the name, this is the rule I had in mind for auto-infering component names:

Does the class have a name or is it a single _? Has name -> Use the class name as-is (using the original PascalCase). Is _-named -> use the name of the immediate package.

We can drop the _ name thing as being too magical, though.

All in all, I'd be fine with this –

@register
class ComponentA(component.Component):
    ...

– and including it as –

{% component "ComponentA" %} {% endcomponent %}

Basically, the name parameter of register() would become optional.


Yeah, the last way of importing the Python API is my favorite.

Your point about not easily being able to infer the meaning of 'register' is very valid – although, my first reaction to anyone facing this problem would be "y u not split up your components module lol?"

Anyway, I suppose one of these two variants would be recommended for larger codebases:

from django_components import register as register_component

or

import django_components as components
@components.register()
class ...

The second is probably what I'd start using by default.

EmilStenstrom commented 1 year ago

About relative paths: The idea for an algorithm was: Check if the path has no slashes, if so resolve against the current directory. If not found fall back to the current algorithm. This would be fully backwards compatible, except you could drop "calendar/" in "calendar/calendar.html" if you wanted. Concerning your three points:

Making name param optional for register: I agree with you here. I think we can skip the "_" case, and use the lowercased version of the class name. The advantage from using the lowercased version is that we don't break the current conversion, that component names are all lowercase.

About imports: After thinking about it a bit more, and think from django_components import register is the best way forward, like you first suggested. Sure, long files mean you don't understand what register means, but that's not a big deal. So let's go with saving a few characters in that case. We can easily make this change in a backwards compatible way.

lemontheme commented 1 year ago

Alright, we're at about 50% agreement. I call that progress. =p (Although, we've gone slightly off-topic wrt the OP, since we still need figure out how we feel about implicit/explicit namespaces.)

The imports discussion is resolved. Indeed, easily backward-comptabile. ✔️

As for the other two points, my impression is that they introduce precisely the kind of magical/potentially surprising behavior I thought we'd agreed we want to avoid. So I'm a bit surprised tbh.

Take the relative paths, for instance. Your design definitely has its strengths, but I can't see how it accounts for these scenarios:

Regarding the first problem case, I suppose a simple solution would be to search inside the current component dir first, regardless of whether the path contains slahes. Only if it can't be matched to an existing file, do we default to the conventional file finding approach. I don't like this personally, since it will inevitably lead to hard-to-debug cases of the wrong file being matched and the user not understanding why.

A more explicit solution might be to define a new user-overridable (class)method that clearly expresses how paths will be resolved. Example:


DepsDict = Dict[Literal['template', 'css', 'js'], Union[str,Path]]

@register
class Calendar(Component):

    @classmethod
    def build_dependency_paths(cls, dependencies: DepsDict) -> DepsDict:
        root_dir = Path(cls.__file__).parent
        return {dep_type: [root_dir / f for f in files] for dep_type, files in dependencies}

    template_name = "template.html"
    class Media:
        css = "style.css"
        js = "index.js"

Users can override that method when they require specific behavior. I've made it a classmethod here because I see paths as a property of the component class, not of instances.

Just thinking out loud here. For me a priority is to make sure the component code contains some trace of how things are working, so that this doesn't become too magical.

I'm afraid we similarly risk sinning against explicitness if we opt to auto-lowercase class names. Couple of things to consider:

Thanks for sticking with me through the discussion here. I realize I'm giving a bit more push-back than I usually do. Also pinging @VojtechPetru and @timothyis – feel free to chime if there's anything that catches your eye here (and if you have time, obviously) 👋

EmilStenstrom commented 1 year ago

Well explained. What I didn't like is now different components might have an uppercase name, and some a lowercase one. No template tags use uppercase ever in Django, so it's a bit non-standard. But I agree that this being searchable is more important, so let's go with your "just take the class name straight off".

About allowing relative paths: I agree that this is introducing magic while cutting down on characters. Let's look at some code:

Version A:

@component.register("calendar")
class Calendar(component.Component):
    template_name = "calendar/template.html"
    class Media:
        css = "calendar/style.css"
        js = "calendar/index.js"

Version B:

@component.register("calendar")
class Calendar(component.Component):
    template_name = "template.html"
    class Media:
        css = "style.css"
        js = "index.js"

Version C:

@component.register("calendar")
class Calendar(component.Component):
    template_name = component.pkg_file("template.html")  #[1] 
    class Media:
        css = component.pkg_file("style.css")
        js = component.pkg_file("index.js")

Are you saying that your prefer version C here? In my min it adds complexity for very little gain. Even though Version B does not explicitly say where it looks for the files, a good guess would be that it uses the ones in the same directory. It just makes sense, and fits well with the purpose of the library: to bundle python, html, css, and js together.

About your corner cases:

A lazy developer who keeps e.g. all their template files in a single flat directory under some template/ dir. Bad practice, obviously, but that's their choice. The paths of these template files do not contain any slashes. How would we resolve such paths?

We would look for the file in the current directory, not find it, and then if not found, look where we look today. Fully backwards compatible unless there's local files with the same name in the same directory (unlikely). Yes, this adds another file access, but I think this is fair to simplify components for everyone else.

What if it makes more sense for a component to use a mix of 'absolute' (i.e. relative to some global dir) and 'relative' (i.e. relative to the file of a component module) – how would a developer convey that unambiguously to other colleagues? Surely, the first-day-on-the-job dev is going to be very confused, no?

Is this the css = ["calendar.css", "util/util.css"] case? Looking at that, I think it's clear which is component relative, and which is from another component. Do you agree?

(Please continue speaking your mind, and I'll do the same! :) )

lemontheme commented 1 year ago

(Yes, please do!)

So, regarding the component names, there's one other approach I can see working that would both make things more explicit and give an indication of the recommended naming scheme. As before, we'd add a user-overridable class method to the Component class.

from django_components.utils import convert_to_snake_case

class Component:  # base component cls

    @classmethod
    def get_default_name(cls) -> str:  # or get_default_registered_name()
        return convert_to_snake_case(cls.__name__)

    ...

That would give

@register #  -> calls Calendar.get_default_name, which is inherited from Component, resulting in name = 'calendar' (lower)
class Calendar(Component): ...

The docstring of the register() function will explain that when name=None, the name is taken from the specific component subclass's get_default_name() method. That leaves a clear trace in the code of how things are working. If users want different naming behavior they are 100% free to create their own subclass and use that as the base for their own component classes.

I think this might be a good marriage of convenience and explicitness. What do you think?

Turning to the file path resolution, I can understand how Version C might seem needlessly complex you had to repeat it in every single component definition. That's not how I see it being used, however. The way I picture it is that we and/or users subclass the base component class once, add standard dependency paths (e.g. index.js), and then re-use the smarter base class for all their components.

For those who prefer a different dependency naming approach, i.e. re-using the component name for all dependencies, e.g. calendar.js, calendar.css, etc. (as opposed to index.js, style.css, etc.), we can support this as well by allowing the pkg_file function (which is not a great name btw) to take a callback, e.g.

class MyBaseComponent(Component):
    template_name = pkg_file(lambda cls: f"{cls.get_default_name()}.html")

Any child subclass would not need to specify template_name anymore.

For cases where there are multiple media dependencies, we could consider allowing glob expressions, e.g.:

class MyBaseComponent(Component):
    class Media:
        js = pkg_files("*.js")  # notice plural name

Downside: a slight increase in the API surface. Upside: it's pretty explicit IMO.

But if we were to keep things simpler, as you propose:

Yes, this adds another file access, but I think this is fair to simplify components for everyone else.

Agree.

I think it's clear which is component relative, and which is from another component.

Is it thought? Take the second path in your example: util/util.css. We'd interpret that as an absolute path, correct? Well... What you say to the user who's keeping util.css in a child directory of the current component package/(sub)directory, e.g. like this:

.
└── calendar
    ├── calendar.css
    ├── calendar.html
    ├── calendar.js
    ├── calendar.py
    └── utils
        └── utils.css

Obviously, this doesn't pose a problem if we try to resolve the file path wrt to the current component dir first, but still – I just wanted to point out the assumption that path separators = absolute path might break in certain circumstances.

In sum, it's true I prefer Version C for its explicitness. But we could make Version B work as well. If we opt for B, my only concern would be making it debuggable. Introducing machinery to inspect the actual path used is perfectly do-able though.

Additionally, in case of B, I would suggest making the path resolution behavior user-overridable – with the same advantages wrt explicitness as before.

Our updated component would look something like this:

class Component:  # base component cls

    @classmethod
    def get_default_name(cls) -> str:  # or get_default_registered_name()
        return convert_to_snake_case(cls.__name__)

    @classmethod
    def resolve_dependency(cls, fname: str | Path, dep_type: Literal['template', 'media']) -> Path:
        import inspect
        root_dir = Path(inspect.getmodule(cls).__file__)  # will raise error in repl
        rel_path = root_dir /  fname
        return rel_path if rel_path.exists() else Path(fname)

Aside from all this, where do you feel namespaces should fit in? Do we also make them infer-able from the Component, e.g. with a similar get_default_namespace() method? Or do we consider it the sole responsibility of the registration machinery?

EmilStenstrom commented 1 year ago

It's worth considering two perspectives here:

  1. What does the default, recommended, component look like?
  2. Which parts is possible to customize when you want something that's different from the default?

I think your changes to the default component satisfies both: We simplify the default, recommended component to contain less repetition. But we also add the possibility to customize this behavior by simply by overriding a method. Great solution!

Do we agree that this is the new recommended default component structure?

from django_components import register, Component

@register
class Calendar(Component):
    template_name = "template.html"
    class Meta:
        css = "style.css"
        js = "index.js"

About file resolution, I wonder if we by default could use COMPONENTS_DIR / get_default_name() as the default path, instead of infering it from the module?

About namespaces: I think adding a get_default_namespace method is a great thing. Maybe that also changes the name, so you use {% component "tailwind_megapack:calendar" %} when importing something from a component package with get_namespace set?

That would mean that you could override it to avoid conflicts if you want.

lemontheme commented 1 year ago

Exactly! Sensible defaults but easy customization with standard OOP. Nothing fancy. I think that aligns pretty well with the Django design philosophy.

I'm definitely still making up my mind about what a good default structure should look like. If we start by looking at what we have today...

.
└── calendar
    ├── calendar.css
    ├── calendar.html
    ├── calendar.js
    └── calendar.py

Pros:

Cons:

If we compare this to a possible new naming scheme:

.
└── calendar
    ├── component.py  # or __init__.py [^1]
    ├── template.html  
    ├── index.js  # OR script.js
    └── style.css

Pros:

Cons:

We should also decide whether we want to treat individual component directories as plain directories or as packages. We're at a point where we don't strictly even need to introduce the concept of 'component packages'. It's a question I'd resolve by looking at how we separate namespaces in the namespace hierarchy.

Question: what counts as a 'namespace' to you? I don't believe we have a clear definition yet. So far, my interpretation is that a namespace is directory on the path between a recognized top-level COMPONENT_DIR and the directory containing the component module. A sequence of namespaces yields a namespace hierarchy.

For example, re-using the example above, given the following file hierarchy...

. # inside a django app OR at the project root level
└── components
    └── calendarapp  # app name (similar to template directory naming conventions)
        └── todos
            └── task_title
               ├── component.py  # or __init__.py
               ├── script.js  # or index.js
               ├── style.css
               └── template.html

... I'd conclude that the namespace hierarchy is calendarapp > todos and the component name is task_title.

But maybe your working definition is different? For instance, maybe what I see as namespaces are in fact just pieces of the component name, giving the complete name calendarapp/todos/task_title, whereas the namespace is explicitly user-defined and is a sort of meta-key that can be used to overcome naming conflicts (e.g. through some project setting), giving: namespace:calendarapp/todos/task_title. Or maybe the namespace should only be the first subdirectory under COMPONENTS_DIR, giving e.g. calendarapp:todos/task_title.

Quite a few possibilities here.

Moving on, letting file resolution depend on an overridable get_default_name() method might be somewhat problematic. Should an overridden get_default_name() return a name that's different than the name of the module's immediate parent directory, the file wouldn't be found. By contrast, the module 'knows' precisely where it's located in the file system.

Thennnn again, we're actually touching on an important question here – and it sort of brings us back to the conversation about infering a component's name from its parent directory/package.

We probably don't want this:

Mind, this scenario can occur today as well (by passing a different name to register()). In fact, we register components under different names all the time in our tests. But in a real-life code base it's probably not ideal.

[...] Maybe that also changes the name, so you use {% component "tailwind_megapack:calendar" %} [...]

Precisely. And the namespace could be remapped as well in settings. By default I was thinking of infering the namespace (hierarchy) by starting at the component directory and traversing up the tree until we hit the top-level COMPONENT_DIR.


Apologies – my thoughts are a bit more scrambled today than usual. Feel free to respond in an equally rambling style =p

timothyis commented 1 year ago

Sorry, I haven't had time to read through all of this, but I just wanted to note that we don't only use one JS file per component. E.g., I am loading a vendor file and a custom JS file for our dialog/modal component. I do believe that a js folder in the component directory would be helpful.

I still haven't figure out how to minify our JS files yet.

Also, it would be tremendously helpful we could choose how script tags render on a page, per script. E.g., some scripts could be deferred.

EmilStenstrom commented 1 year ago

Quick replies to @lemontheme:

EmilStenstrom commented 1 year ago

@timothyis You can modify how a specific component is rendered by overriding Component.get_js_dependencies method on a component. Go wild! :)

lemontheme commented 1 year ago

@timothyis Thanks for weighing in. No worries, we've accumulated a fair bit of text here.

Multiple files per dependency type seems like something the new design should support. I've got a couple of ideas. Are multiple files something you think should be auto-discovered or specified explicitly? The reason I ask is because – my knowledge of JS being less deep – I'm concerned the order in which scripts are loaded might be important in certain cases?

Minification is something I'd like to look at in the long run. I can imagine a command that autogenerates a webpack index.js or something like that. But definitely still early days there.

lemontheme commented 1 year ago

@EmilStenstrom

timothyis commented 1 year ago

Are multiple files something you think should be auto-discovered or specified explicitly? The reason I ask is because – my knowledge of JS being less deep – I'm concerned the order in which scripts are loaded might be important in certain cases?

Indeed, defining the order is important. Technically you can solve this with JS itself, but I would definitely require load ordering.

EmilStenstrom commented 1 year ago

@lemontheme Yes, that was the idea! Not sure if your need both calendarapp and todos there, are you thinking that the todos level is optional?

About js files: We support multiple js files today with js = ["1.js", "2.js"]. They should be rendered in that order too I think?

About minifiation: This is supported today with other packages as I've understood it. The fact that we use Django's Media class below makes things work automatically. But needs documentation: #39

timothyis commented 1 year ago

We support multiple js files today

Yes indeed, but with the context of this thread (from what I've read), I wanted to make sure that this is maintained.

VojtechPetru commented 1 year ago

Hello 👋 quite a rich discussion going on here. I don't think I have much to add, so just my 2¢.

timothyis commented 1 year ago

Out of interest, will this enable components of the same namespace (if you like) to access the same context? I'd quite like to share context within a set of components that are part of the same thing (e.g., we have a table, table-row, table-column component set). It would be amazing to access componentScope.responsive within the table-row component if responsive=True is passed through the table component that wraps the row component, but without relying on page context to handle this so we can use only where we want without affecting component scope.

EmilStenstrom commented 1 year ago

@timothyis I think this would be a separate feature. The idea here is mainly to allow using two components with the same names.

rtr1 commented 10 months ago

Can I ask a couple questions:

  1. What is the purpose or need to register components at all? What problem does it solve?
  2. How does this need compare to other major concepts in Django, on the spectrum of explicit-to-implicit?

For instance, some parts require explicit, manual registration such as adding a model to the admin portal. While others are implicitly and automatically found and used in the app, such as templates, or model migrations happening without having to specify where any models are located.

EmilStenstrom commented 10 months ago

@rtr1: Just to clarify: Do you have another system in mind that does not rely on registration of components? What advantages does that system have?

rtr1 commented 10 months ago

@EmilStenstrom I’m saying, in software development, in general, the default seems to be no registration. I don’t register Django models, or React components, or pydantic models, or Tkinter widgets, or class objects in general.

So why is there registration instead of no registration? Is it so templates can map the component names to registered components?

EmilStenstrom commented 10 months ago

@rtr1 Yes, you are right. I added registrations as a way to bind a component name to a specific class, just like you do with custom template tags. It's a way to decouple the name of the component from the actual component, and make that easily changeable.

rtr1 commented 10 months ago

Okay interesting.

I recall when I was experimenting with single file components and adding a component’s as_view() method (to return partials to something like HTMX), I ended up having to ditch the register decorator and instead have a separate file that calls the register function on each of the components. Otherwise the register function was getting called more than once.

So I am essentially building out something like the urls.py model of doing things.

Something like:

# ./components/registration.py
from components.title import Title
from components.table import Cell
from django_components import register

register(“cell”)(Cell)
refister(“title”)(Title)
# …

Then in urls.py:

from components.title import Title

urlpatterns = [
    path(“title”, Title.as_view())
    # …
]
Fingel commented 4 months ago

I may be late to the party here, but I am in favor of a more lax approach.

If a reusable app provides a component named table, and you want to register your own component named table, I think that's fine, as long as yours takes priority. Chances are if you are creating your own table component you don't care about the one installed by the library, you want to use your own.

I believe there is already a precedence for this in Django: template tags. There is nothing stopping you from creating a template tag with the same name as one provided by a reusable app. In practice, I believe most app authors namespace their tags by simply naming them something that won't conflict, for example django-bootstrap4 prefixes all template tags with bootsrap_: https://django-bootstrap4.readthedocs.io/en/latest/templatetags.html

Interestingly, there was a PR merged to Django a few years ago that implements a system check for duplicate template tag names: https://github.com/django/django/pull/15005

I think a system check on duplicate component names would be a great approach, and probably not too hard to implement (could be emitted during the autodiscovery step). App authors should be considerate to not make their component names to general but if they do conflict, I think it's fine to use whichever was registered "last" (just like some apps tell you to place their entry in INSTALLED_APPS near the end).

EmilStenstrom commented 4 months ago

With templatetags you explicitly import them at the top of the template. This means you have the option to either import a component library, or your own tag on a template to template basis.

in the Django-components case all components are registered in a global registry. If a duplicate name is encountered, we emit a AlreadyRegistered-exception.

The problem with this is if someone made a component library, there’s no way for you to decide how to decide what names to give the components. The library author sets them. And if you currently have a comp named table, you can’t use the library.

Fingel commented 4 months ago

With tempatetags you don't explicitly import them by name at the top of the template, typically you load a group of them, such as {% load bootstrap5 %} (or skip this entirely by adding the module to your TEMPLATE settings). This sort of acts like a namespace, only in that it allows you to decide to import all names or not, making all names from that app available in the template, like {% bootstrap_alert %}.

There is nothing stopping you from registering your own templatetags with names that conflict with those provided by reusable apps. Here is a repo demonstrating that.

In the template that only uses the 3rd party template tag, the output looks like this: https://github.com/Fingel/template-tag-nonsense/blob/012d8d4da3079a7185275ff7d6e8c08c304efbdd/myapp/templates/myapp/only_bootstrap.html#L12 image Now when we define our own template tag bootstap_alert and load the custom library after the bootstrap library you can see that it gets overwritten: https://github.com/Fingel/template-tag-nonsense/blob/012d8d4da3079a7185275ff7d6e8c08c304efbdd/myapp/templates/myapp/duplicate_filter.html#L12 image Furthermore, reversing the order of the load statements reverts to using the 3rd party app version, showing that the library always returns the last one registered.

in the Django-components case all components are registered in a global registry. If a duplicate name is encountered, we emit a AlreadyRegistered-exception.

This is an implementation detail though, which I spent some time staring at a couple days ago. It's not strictly necessary to throw an error, instead it could (maybe with a warning?) simply overwrite a registered component with the new one. Looking and Django's source code, we see they do something very similar when adding functions to their template library:

https://github.com/django/django/blob/2005530920e7c290bb1cf7d9ada155250faa81ad/django/template/library.py#L44

It may be nice to follow the behaviors that the Django project already has in place, especially considering that django-components in some ways can be thought as similar to super charged template tags.

EmilStenstrom commented 4 months ago

@Fingel What I was getting at was that there is an explicit {% load %} for each set of template tags at the top of the template. With django-components we just have a couple of generic tags ({% component %} and {% component_block %}), all loaded with a single {% load component_tags %}. Registering components, and indeed raising exceptions, is instead done when the module is loaded. So the issue here is that component registration is done at import time, and there is no way to control it.

Fingel commented 4 months ago

I see, so the issue here is there’s not a clear way to get an explicit import order as there is with load tags. I guess there is always the order in INSTALLED_APPS with later apps taking precedence. I have seen apps that actually rely on this, though it is not a very good experience.

I’ll have to think on this. FWIW I am interested in this because I’m considering generalizing a few ui elements written as django components as a reuse-able Django app, and I had the same thought as the original poster.

EmilStenstrom commented 4 months ago

Making component packages installed apps, and ordering them in INSTALLED_APPS is one way to do it. I'm sure there are others. One disadvantage of that I can think of: It's "all or nothing", you can't just install one component, and all components from the later app overrides all existing ones.

Generalizing a couple of components sounds like a great project! Just making a couple of components in the style you want them likely gives good inputs into what a practical solution should look like.

EmilStenstrom commented 1 month ago

This thread is too long for anyone to jump into. If there are suggestions on how to improve how someone would package and distribute a pre-made set of components I suggest you start a new issue thread.