Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: @template_tag and refactor how template tags are defined #910

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

JuroOravec
Copy link
Collaborator

No description provided.

@@ -0,0 +1,197 @@
---
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a documentation page on creating custom template tags that make use of django-component's features.

Here's a preview:

Screen.Recording.2025-01-16.at.10.15.25.mov

- If required parameters are missing, an error is raised
- If unexpected parameters are passed, an error is raised

To accept keys that are not valid Python identifiers (e.g. `data-id`), or would conflict with Python keywords (e.g. `is`), you can use the `**kwargs` syntax:
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This section describes what we outlined in #900 (reply in thread), that kwargs like data-id or class should still be accepted. But when doing so, these kwargs must be collected onto **kwargs.

@@ -585,29 +586,29 @@ def _list_urls(urlpatterns: Sequence[Union[URLPattern, URLResolver]], prefix="")
return urls


def _format_tag_signature(tag_spec: TagSpec) -> str:
def _format_tag_signature(node_cls: BaseNode) -> str:
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before, the API reference for template tags was created from the TagSpec objects that were atttached to the template tag definitions. Now the same information is held directly by the Node classes

from django_components.util.template_tag import TagParams

HTML_ATTRS_DEFAULTS_KEY = "defaults"
HTML_ATTRS_ATTRS_KEY = "attrs"


class HtmlAttrsNode(BaseNode):
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now, all these 4 are co-located:

  1. Tag definition (e.g. whether it's {% slot %} or {% provide %}),
  2. The Node class that the tag is converted to
  3. The render logic when said Node is encountered during render
  4. The source of truth for the API documentation of this template tag

So I refactored all our template tags, from templatetags/component_tags.py, so that the actual definitions are stored in the respective files. So now, templatetags/component_tags.py contains no logic, and it only declares which template tags we define.

Previously the documentation for individual tags was in templatetags/component_tags.py, so that's why you can see the docstrings here. The docstrings didn't change.

append_attrs: List[Tuple[str, Any]] = []

# Resolve all data
args, kwargs = self.params.resolve(context)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also the render() methods are simpler now, because they no longer have to contain logic for figuring out what inputs the template tag received. This is now done behind the scenes by BaseNode. And the individual template tag implementations only need to declare the signature on the render() method.


self.name = name
self.isolated_context = isolated_context
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Flags are not stored on the Node class directly, but instead can be accessed as self.flags["flag_name"]

self.registry = registry

def __repr__(self) -> str:
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Common methods like __repr__ were moved up to BaseNode

self.name,
getattr(self, "nodelist", None), # 'nodelist' attribute only assigned later.
@classmethod
def parse( # type: ignore[override]
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ComponentNode is used when we use {% component %} template tag.

Remember that we use ComponentRegistry:

  1. To allow components to be rendered either as {% component %} or {% comp_name %}
  2. To allow to share components as standalone packages.

Because of that, when we use the {% comp_name %} ("shorthand") syntax, then to play nice with the new definition of template tags, we dynamically create a subclass of BaseNode / ComponentNode, that defines the comp_name / endcomp_name as it's start / end tag.


Looking at the code below, I realize that this creates a subclass of ComponentNode on every {% component %} (or custom named tag). But in reality, we need to define only one subclass for component, etc.

So this could be improved by storing the created subclasses, and reusing them when the start_tag matches (and raise if the end_tag or registry don't match).

But this is already a big MR, so I'll leave that for later.


def render(self, context: Context) -> str:
trace_msg("RENDR", "COMP", self.name, self.node_id)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also the tracing logs were moved into BaseNode.

@@ -62,27 +62,18 @@ def trace(logger: logging.Logger, message: str, *args: Any, **kwargs: Any) -> No


def trace_msg(
action: Literal["PARSE", "RENDR", "GET", "SET"],
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed unused bits from this function

@@ -22,6 +22,11 @@ def setup_test_config(
"tests/templates/",
"tests/components/", # Required for template relative imports in tests
],
"OPTIONS": {
"builtins": [
"django_components.templatetags.component_tags",
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes it so that we don't have to call {% load component_tags %} in tests. See #900 (reply in thread)

# the component name and passing the rest to the actual tag function.
def tag_fn(parser: Parser, token: Token) -> ComponentNode:
# Let the TagFormatter pre-process the tokens
bits = token.split_contents()
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This couple of next lines previously lived in templatetags/component_tags.py (https://github.com/EmilStenstrom/django-components/pull/910/files#diff-7345171ed38e1dd2e4b8199b65a2fd082323bdb331ff0ef0dc0dd3bd43f111aeL498).

It calls the TagFormatter to figure out which start and end template tags should be used with a particular component (e.g. {% component "table" %} ... {% endcomponent %} vs {% table %} ... {% endtable %}.

I moved it up here, so that, instead, the ComponentNode only accepts the already-resolved start_tag and end_tag.

#########################################################


def _component_dependencies(type: Literal["js", "css"]) -> SafeString:
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The {% component_js_dependencies %} and {% component_css_dependencies %} were moved to this file for cosistency.

# NOTE: The documentation generation script in `docs/scripts/reference.py` actually
# searches this file for all `Node` classes and uses them to generate the documentation.
# The docstring on the Node classes is used as the tag's documentation.
ComponentNode.register(register)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file now only register the template tags onto the registry.


@property
def active_flags(self) -> List[str]:
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

active_flags was also moved to BaseNode, so now all our Node classes have this property


def resolve_kwargs(self, context: Context) -> "FillWithData":
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FillNode.resolve_kwargs and FIllNode._process_kwarg were removed. Now There is only FillNode.render()

)

# For details see https://github.com/EmilStenstrom/django-components/pull/902
def validate_params(self, params: List["TagParam"]) -> Tuple[List[Any], Dict[str, Any]]:
Copy link
Collaborator Author

@JuroOravec JuroOravec Jan 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TagSpec.validate_params is now a standalone function https://github.com/EmilStenstrom/django-components/pull/910/files#diff-56d60a8b1d11d8ff86b912ddde6d8ee539f71a1fae917bc39a1fd049ea22705cR21.

Also, the body got simpler as we don't need to sort inputs into args and kwargs.

@@ -11,152 +16,36 @@
from django_components.util.tag_parser import TagAttr, parse_tag


@dataclass
class TagSpec:
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TagSpec was removed. the tag, end_tag, flags and signature are now all defined by the BaseNode subclasses.

@@ -180,64 +69,54 @@ class TagParam:
value: Any


class TagParams(NamedTuple):
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TagParams can be removed as a class. Previously it was meant only to ensure that the params, and tag_spec were held in a joint object. But now that's done by BaseNode.

So, similarly to validation, I refactored TagParams.resolve() into a standalone function resolve_params

@@ -348,3 +227,99 @@ def merge_repeated_kwargs(params: List[TagParam]) -> List[TagParam]:
params_by_key[param.key].value += " " + str(param.value)

return resolved_params


def apply_params_in_original_order(
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This logic was previously part of validate_params.

The behaviour of this function relates to what we discussed in #900 (comment).

In the end it's implemented so that template tags can have also kwargs like data-id=123 or `class="pt-4".

{% mytag data-id=123 class="pt-4" %}

This would raise if these kwargs were passed like so:

fn(data-id=123, class="pt-4")

Because the first one is not a valid Python variable name, and the latter is a reserved keyword.

So instead, we pass those through the extra_kwargs, which makes these special kwargs to be applied as:

extra_kwargs = {"data-id": 123, "class": "pt-4"}
fn(**extra_kwargs)

Which IS allowed by Python.

return cls


class BaseNode(Node, metaclass=NodeMeta):
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this file is the core of this PR, which adds the BaseNode class and @template_tag decorator, for an easy creation of template tags that use the same features like other django-components' tags.

@JuroOravec JuroOravec marked this pull request as ready for review January 18, 2025 12:05
@JuroOravec
Copy link
Collaborator Author

Note to self: After this, there's a bit of cleanup remaining:

  1. I noticed that in the docs site, the page listing all Django signals is NOT shown. I think I need to push to source control the updated built reference pages in docs/reference.

  2. Avoid duplicitly creating ComponentNode subclasses, as described in feat: @template_tag and refactor how template tags are defined #910 (comment)

  3. I still need to update the documentation regarding the use of literal dicts and lists in template tags, and other changes introduced in feat: Literal dicts and lists part 2 #902. But after this PR things are clearer and I'll be able to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant