From 898d1483820778a05d7db9730b00d1702d241e6b Mon Sep 17 00:00:00 2001 From: lemontheme Date: Mon, 13 Mar 2023 10:22:47 +0100 Subject: [PATCH] Add required kwd to slot tag and add test Move required slot check to SlotNode.render(); clean up needed Remove unused code; drop caching Update docs Incorporate PR feedback --- README.md | 13 +- django_components/app_settings.py | 6 - django_components/component.py | 161 ++++++++---------- .../templatetags/component_tags.py | 70 ++++---- tests/django_test_setup.py | 3 +- ...ponent_nesting_template_pt2_dashboard.html | 3 +- .../slotted_template_with_required_slot.html | 5 + .../template_with_nonunique_slots.html | 2 +- tests/test_templatetags.py | 18 +- 9 files changed, 143 insertions(+), 138 deletions(-) create mode 100644 tests/templates/slotted_template_with_required_slot.html diff --git a/README.md b/README.md index f76acb8c..a07d38ef 100644 --- a/README.md +++ b/README.md @@ -323,16 +323,21 @@ This makes it possible to organize your front-end around reusable components. In # Using slots in templates -_New in version 0.26_ (__breaking change__): Defining slots and passing content to them used to be achieved by a single block tag `{% slot %}`. To make component nesting easier these functions have been split across two separate tags. Now, `{% slot %}` serves only to declare/define/open new slots inside the component template. The function of passing in content to a slot has been moved to a newly introduced `{% fill %}` tag. +_New in version 0.26_: -Components support something called 'slots'. +- The `slot` tag now serves only to declare new slots inside the component template. + - To override the content of a declared slot, use the newly introduced `fill` tag instead. +- Whereas unfilled slots used to raise a warning, filling a slot is now optional by default. + - To indicate that a slot must be filled, the new keyword `required` should be added at the end of the `slot` tag. + +Components support something called 'slots'. When a component is used inside another template, slots allow the parent template to override specific parts of the child component by passing in different content. This mechanism makes components more reusable and composable. In the example below we introduce two block tags that work hand in hand to make this work. These are... -- `{% slot %}`/`{% endslot %}`: Declare new slot on component template. -- `{% fill %}`/`{% endfill %}`: Used inside component block. The content of this block is injected into the slot with the same name. +- `{% slot %}`/`{% endslot %}`: Declares a new slot in the component template. +- `{% fill %}`/`{% endfill %}`: (Used inside a component block.) Fills a declared slot with the specified content. Let's update our calendar component to support more customization by updating our calendar.html template. diff --git a/django_components/app_settings.py b/django_components/app_settings.py index 9ca705db..11c26e83 100644 --- a/django_components/app_settings.py +++ b/django_components/app_settings.py @@ -17,11 +17,5 @@ class AppSettings: def TEMPLATE_CACHE_SIZE(self): return self.settings.setdefault("template_cache_size", 128) - @property - def STRICT_SLOTS(self): - """If True, component slots that are declared must be explicitly filled; else - a TemplateSyntaxError is raised.""" - return self.settings.setdefault("strict_slots", False) - app_settings = AppSettings() diff --git a/django_components/component.py b/django_components/component.py index 26068216..5aef8d59 100644 --- a/django_components/component.py +++ b/django_components/component.py @@ -1,29 +1,23 @@ from __future__ import annotations -import warnings -from collections import defaultdict -from contextlib import contextmanager -from functools import lru_cache +import copy from typing import ( TYPE_CHECKING, ClassVar, Dict, + Iterator, List, Optional, - Tuple, TypeVar, ) -from django.conf import settings from django.core.exceptions import ImproperlyConfigured from django.forms.widgets import MediaDefiningClass -from django.template import TemplateSyntaxError +from django.template import Context, TemplateSyntaxError from django.template.base import Node, NodeList, Template from django.template.loader import get_template from django.utils.safestring import mark_safe -from django_components.app_settings import app_settings - # Allow "component.AlreadyRegistered" instead of having to import these everywhere from django_components.component_registry import ( # noqa AlreadyRegistered, @@ -104,61 +98,26 @@ class Component(metaclass=SimplifiedInterfaceMediaDefiningClass): """Render only JS dependencies available in the media class.""" return mark_safe("\n".join(self.media.render_js())) - @classmethod - @lru_cache(maxsize=app_settings.TEMPLATE_CACHE_SIZE) - def fetch_and_analyze_template( - cls, template_name: str - ) -> Tuple[Template, Dict[str, SlotNode]]: - template: Template = get_template(template_name).template - slots = {} - for slot in iter_slots_in_nodelist(template.nodelist, template.name): - slot.component_cls = cls - slots[slot.name] = slot - return template, slots + def get_declared_slots( + self, context: Context, template: Optional[Template] = None + ) -> List[SlotNode]: + if template is None: + template = self.get_template(context) + return list( + dfs_iter_slots_in_nodelist(template.nodelist, template.name) + ) - def get_processed_template(self, context): - template_name = self.get_template_name(context) - # Note: return of method below is cached. - template, slots = self.fetch_and_analyze_template(template_name) - self._raise_if_fills_do_not_match_slots( - slots, self.instance_fills, self._component_name - ) - self._raise_if_declared_slots_are_unfilled( - slots, self.instance_fills, self._component_name - ) + def get_template(self, context, template_name: Optional[str] = None): + if template_name is None: + template_name = self.get_template_name(context) + template = get_template(template_name).template return template - @staticmethod - def _raise_if_declared_slots_are_unfilled( - slots: Dict[str, SlotNode], fills: Dict[str, FillNode], comp_name: str - ): - # 'unconditional_slots' are slots that were encountered within an 'if_filled' - # context. They are exempt from filling checks. - unconditional_slots = { - slot.name for slot in slots.values() if not slot.is_conditional - } - unused_slots = unconditional_slots - fills.keys() - if unused_slots: - msg = ( - f"Component '{comp_name}' declares slots that " - f"are not filled: '{unused_slots}'" - ) - if app_settings.STRICT_SLOTS: - raise TemplateSyntaxError(msg) - elif settings.DEBUG: - warnings.warn(msg) + def set_instance_fills(self, fills: Dict[str, FillNode]) -> None: + self._instance_fills = fills - @staticmethod - def _raise_if_fills_do_not_match_slots( - slots: Dict[str, SlotNode], fills: Dict[str, FillNode], comp_name: str - ): - unmatchable_fills = fills.keys() - slots.keys() - if unmatchable_fills: - msg = ( - f"Component '{comp_name}' passed fill(s) " - f"refering to undefined slot(s). Bad fills: {list(unmatchable_fills)}." - ) - raise TemplateSyntaxError(msg) + def set_outer_context(self, context): + self._outer_context = context @property def instance_fills(self): @@ -168,28 +127,56 @@ class Component(metaclass=SimplifiedInterfaceMediaDefiningClass): def outer_context(self): return self._outer_context or {} - @contextmanager - def assign( - self: T, - fills: Optional[Dict[str, FillNode]] = None, - outer_context: Optional[dict] = None, - ) -> T: - if fills is not None: - self._instance_fills = fills - if outer_context is not None: - self._outer_context = outer_context - yield self - self._instance_fills = None - self._outer_context = None - - def render(self, context): - template = self.get_processed_template(context) - current_fills_stack = context.get( - FILLED_SLOTS_CONTEXT_KEY, defaultdict(list) + def get_updated_fill_stacks(self, context): + current_fill_stacks = context.get(FILLED_SLOTS_CONTEXT_KEY, None) + updated_fill_stacks = ( + copy.deepcopy(current_fill_stacks) + if current_fill_stacks is not None + else {} ) for name, fill in self.instance_fills.items(): - current_fills_stack[name].append(fill) - with context.update({FILLED_SLOTS_CONTEXT_KEY: current_fills_stack}): + if name in updated_fill_stacks: + updated_fill_stacks[name].append(fill) + else: + updated_fill_stacks[name] = [fill] + return updated_fill_stacks + + def validate_fills_and_slots_( + self, + context, + template: Template, + fills: Optional[Dict[str, FillNode]] = None, + ) -> None: + if fills is None: + fills = self.instance_fills + all_slots: List[SlotNode] = self.get_declared_slots(context, template) + slots: Dict[str, SlotNode] = {} + # Each declared slot must have a unique name. + for slot in all_slots: + slot_name = slot.name + if slot_name in slots: + raise TemplateSyntaxError( + f"Encountered non-unique slot '{slot_name}' in template " + f"'{template.name}' of component '{self._component_name}'." + ) + slots[slot_name] = slot + # All fill nodes must correspond to a declared slot. + unmatchable_fills = fills.keys() - slots.keys() + if unmatchable_fills: + msg = ( + f"Component '{self._component_name}' passed fill(s) " + f"refering to undefined slot(s). Bad fills: {list(unmatchable_fills)}." + ) + raise TemplateSyntaxError(msg) + # Note: Requirement that 'required' slots be filled is enforced + # in SlotNode.render(). + + def render(self, context): + template_name = self.get_template_name(context) + template = self.get_template(context, template_name) + self.validate_fills_and_slots_(context, template) + updated_fill_stacks = self.get_updated_fill_stacks(context) + with context.update({FILLED_SLOTS_CONTEXT_KEY: updated_fill_stacks}): return template.render(context) class Media: @@ -197,23 +184,15 @@ class Component(metaclass=SimplifiedInterfaceMediaDefiningClass): js = [] -def iter_slots_in_nodelist(nodelist: NodeList, template_name: str = None): +def dfs_iter_slots_in_nodelist( + nodelist: NodeList, template_name: str = None +) -> Iterator[SlotNode]: from django_components.templatetags.component_tags import SlotNode nodes: List[Node] = list(nodelist) - slot_names = set() while nodes: node = nodes.pop() if isinstance(node, SlotNode): - slot_name = node.name - if slot_name in slot_names: - context = ( - f" in template '{template_name}'" if template_name else "" - ) - raise TemplateSyntaxError( - f"Encountered non-unique slot '{slot_name}'{context}" - ) - slot_names.add(slot_name) yield node for nodelist_name in node.child_nodelists: nodes.extend(reversed(getattr(node, nodelist_name, []))) diff --git a/django_components/templatetags/component_tags.py b/django_components/templatetags/component_tags.py index 7244e081..e11c3185 100644 --- a/django_components/templatetags/component_tags.py +++ b/django_components/templatetags/component_tags.py @@ -1,6 +1,6 @@ from __future__ import annotations -from typing import TYPE_CHECKING, DefaultDict, List, Optional, Tuple +from typing import TYPE_CHECKING, Dict, List, Optional, Tuple from django import template from django.conf import settings @@ -164,11 +164,13 @@ class UserSlotVar: class SlotNode(Node): - def __init__(self, name, nodelist): + def __init__( + self, name, nodelist, template_name: str = "", required=False + ): self.name = name self.nodelist = nodelist - self.component_cls = None - self.is_conditional: bool = False + self.template_name = template_name + self.is_required = required def __repr__(self): return f"" @@ -178,13 +180,19 @@ class SlotNode(Node): raise TemplateSyntaxError( f"Attempted to render SlotNode '{self.name}' outside a parent component." ) - filled_slots: DefaultDict[str, List[FillNode]] = context[ + filled_slots: Dict[str, List[FillNode]] = context[ FILLED_SLOTS_CONTEXT_KEY ] - fill_node_stack = filled_slots[self.name] + fill_node_stack = filled_slots.get(self.name, None) extra_context = {} - if not fill_node_stack: # if [] + if not fill_node_stack: # if None or [] nodelist = self.nodelist + # Raise if slot is 'required' + if self.is_required: + raise TemplateSyntaxError( + f"Slot '{self.name}' is marked as 'required' (i.e. non-optional), " + f"yet no fill is provided. Check template '{self.template_name}'" + ) else: fill_node = fill_node_stack.pop() nodelist = fill_node.nodelist @@ -204,6 +212,16 @@ def do_slot(parser, token): # e.g. {% slot %} if len(args) == 1: slot_name: str = args[0] + required = False + elif len(args) == 2: + slot_name: str = args[0] + required_keyword = args[1] + if required_keyword != "required": + raise TemplateSyntaxError( + f"'{bits[0]}' only accepts 'required' keyword as optional second argument" + ) + else: + required = True else: raise TemplateSyntaxError( f"{bits[0]}' tag takes only one argument (the slot name)" @@ -220,7 +238,8 @@ def do_slot(parser, token): nodelist = parser.parse(parse_until=["endslot"]) parser.delete_first_token() - return SlotNode(slot_name, nodelist) + template_name = parser.origin.template_name + return SlotNode(slot_name, nodelist, template_name, required) class FillNode(Node): @@ -328,17 +347,16 @@ class ComponentNode(Node): for fill_node in self.fill_nodes } - # Create a fresh isolated context if requested w 'only' keyword. - with component.assign( - fills=resolved_fills, outer_context=context.flatten() - ): - component_context = component.get_context_data( - *resolved_context_args, **resolved_context_kwargs - ) - if self.isolated_context: - context = context.new() - with context.update(component_context): - rendered_component = component.render(context) + component.set_instance_fills(resolved_fills) + component.set_outer_context(context) + + component_context = component.get_context_data( + *resolved_context_args, **resolved_context_kwargs + ) + if self.isolated_context: + context = context.new() + with context.update(component_context): + rendered_component = component.render(context) if is_dependency_middleware_active(): return ( @@ -432,7 +450,9 @@ def fill_tokens(parser): not is_whitespace(token) and token.token_type != TokenType.COMMENT ): raise TemplateSyntaxError( - f"Content tokens in component blocks must be placed inside 'fill' tags: {token}" + "Component block EITHER contains illegal tokens tag that are not " + "{{% fill ... %}} tags OR the proper closing tag -- " + "{{% endcomponent_block %}} -- is missing." ) @@ -536,7 +556,6 @@ class IfSlotFilledNode(Node): ): # [(, nodelist, )] self.branches = branches - self.visit_and_mark_slots_as_conditional_() def __iter__(self): for _, nodelist, _ in self.branches: @@ -546,15 +565,6 @@ class IfSlotFilledNode(Node): def __repr__(self): return f"<{self.__class__.__name__}>" - def visit_and_mark_slots_as_conditional_(self): - stack = list(self.nodelist) - while stack: - node = stack.pop() - if isinstance(node, SlotNode): - node.is_conditional = True - for nodelist_name in node.child_nodelists: - stack.extend(getattr(node, nodelist_name, ())) - @property def nodelist(self): return NodeList(self) diff --git a/tests/django_test_setup.py b/tests/django_test_setup.py index 39ee33d9..421e3d9e 100644 --- a/tests/django_test_setup.py +++ b/tests/django_test_setup.py @@ -10,12 +10,11 @@ if not settings.configured: "DIRS": ["tests/templates/"], } ], - COMPONENTS={"template_cache_size": 128, "strict_slots": False}, + COMPONENTS={"template_cache_size": 128}, MIDDLEWARE=[ "django_components.middleware.ComponentDependencyMiddleware" ], DATABASES={}, - # DEBUG=True ) django.setup() diff --git a/tests/templates/slotted_component_nesting_template_pt2_dashboard.html b/tests/templates/slotted_component_nesting_template_pt2_dashboard.html index 5dede4e6..5df9d845 100644 --- a/tests/templates/slotted_component_nesting_template_pt2_dashboard.html +++ b/tests/templates/slotted_component_nesting_template_pt2_dashboard.html @@ -8,6 +8,7 @@ {% endcomponent_block %}
    {% for item in items %} -
  1. {{ item }}
    2. {% endfor %} +
  2. {{ item }}
    3. + {% endfor %}
diff --git a/tests/templates/slotted_template_with_required_slot.html b/tests/templates/slotted_template_with_required_slot.html new file mode 100644 index 00000000..f2b425cf --- /dev/null +++ b/tests/templates/slotted_template_with_required_slot.html @@ -0,0 +1,5 @@ +{% load component_tags %} +
+

{% slot "title" required %}{% endslot %}

+

{% slot "subtitle" %}{% endslot %}

+
\ No newline at end of file diff --git a/tests/templates/template_with_nonunique_slots.html b/tests/templates/template_with_nonunique_slots.html index f6d649f9..5cd7276f 100644 --- a/tests/templates/template_with_nonunique_slots.html +++ b/tests/templates/template_with_nonunique_slots.html @@ -1,4 +1,4 @@ {% load component_tags %}
{% slot "header" %}Default header{% endslot %}
-
{% slot "header" %}Default main header{% endslot %}
{# <- whoops! slot name 'header' used twice. +
{% slot "header" %}Default main header{% endslot %}
{# <- whoops! slot name 'header' used twice. #}
{% slot "footer" %}Default footer{% endslot %}
\ No newline at end of file diff --git a/tests/test_templatetags.py b/tests/test_templatetags.py index 32897648..38de7d50 100644 --- a/tests/test_templatetags.py +++ b/tests/test_templatetags.py @@ -334,6 +334,21 @@ class ComponentSlottedTemplateTagTest(SimpleTestCase): """ self.assertHTMLEqual(rendered, expected) + def test_missing_required_slot_raises_error(self): + class Component(component.Component): + template_name = "slotted_template_with_required_slot.html" + + component.registry.register("test", Component) + template = Template( + """ + {% load component_tags %} + {% component_block 'test' %} + {% endcomponent_block %} + """ + ) + with self.assertRaises(TemplateSyntaxError): + template.render(Context({})) + class SlottedTemplateRegressionTests(SimpleTestCase): def setUp(self): @@ -902,9 +917,6 @@ class ComponentNestingTests(SimpleTestCase): {% endcomponent_block %} """ ) - import sys - - sys.setrecursionlimit(100) rendered = template.render(Context({"items": [1, 2]})) expected = """