-
-
Notifications
You must be signed in to change notification settings - Fork 83
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
fix: loader tags compatibility #468
Merged
JuroOravec
merged 8 commits into
EmilStenstrom:master
from
JuroOravec:134-fix-components-and-loader-tags-compat
May 2, 2024
Merged
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
312d9db
refactor: fix components and loader tags compat
JuroOravec 4eaf9b0
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] f3cd4cc
refactor: fix tests
JuroOravec 429f60f
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] 88f82d7
refactor: add comment and test for resolved fill names
JuroOravec 3061c41
refactor: fix typo
JuroOravec 44c87cc
Merge branch 'master' into 134-fix-components-and-loader-tags-compat
JuroOravec 4fd3df0
docs: add intuition on slots and blocks
JuroOravec File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,166 @@ | ||
# Using `slot` and `block` tags | ||
|
||
1. First let's clarify how `include` and `extends` tags work inside components. | ||
So when component template includes `include` or `extends` tags, it's as if the "included" | ||
template was inlined. So if the "included" template contains `slot` tags, then the component | ||
uses those slots. | ||
|
||
So if you have a template `abc.html`: | ||
```django | ||
<div> | ||
hello | ||
{% slot "body" %}{% endslot %} | ||
</div> | ||
``` | ||
|
||
And components that make use of `abc.html` via `include` or `extends`: | ||
```py | ||
@component.register("my_comp_extends") | ||
class MyCompWithExtends(component.Component): | ||
template = """{% extends "abc.html" %}""" | ||
|
||
@component.register("my_comp_include") | ||
class MyCompWithInclude(component.Component): | ||
template = """{% include "abc.html" %}""" | ||
``` | ||
|
||
Then you can set slot fill for the slot imported via `include/extends`: | ||
|
||
```django | ||
{% component "my_comp_extends" %} | ||
{% fill "body" %} | ||
123 | ||
{% endfill %} | ||
{% endcomponent %} | ||
``` | ||
|
||
And it will render: | ||
```html | ||
<div> | ||
hello | ||
123 | ||
</div> | ||
``` | ||
|
||
2. Slot and block | ||
|
||
So if you have a template `abc.html` like so: | ||
|
||
```django | ||
<div> | ||
hello | ||
{% block inner %} | ||
1 | ||
{% slot "body" %} | ||
2 | ||
{% endslot %} | ||
{% endblock %} | ||
</div> | ||
``` | ||
|
||
and component `my_comp`: | ||
|
||
```py | ||
@component.register("my_comp") | ||
class MyComp(component.Component): | ||
template_name = "abc.html" | ||
``` | ||
|
||
Then: | ||
|
||
1. Since the `block` wasn't overriden, you can use the `body` slot: | ||
|
||
```django | ||
{% component "my_comp" %} | ||
{% fill "body" %} | ||
XYZ | ||
{% endfill %} | ||
{% endcomponent %} | ||
``` | ||
|
||
And we get: | ||
|
||
```html | ||
<div>hello 1 XYZ</div> | ||
``` | ||
|
||
2. `blocks` CANNOT be overriden through the `component` tag, so something like this: | ||
|
||
```django | ||
{% component "my_comp" %} | ||
{% fill "body" %} | ||
XYZ | ||
{% endfill %} | ||
{% endcomponent %} | ||
{% block "inner" %} | ||
456 | ||
{% endblock %} | ||
``` | ||
|
||
Will still render the component content just the same: | ||
|
||
```html | ||
<div>hello 1 XYZ</div> | ||
``` | ||
|
||
3. You CAN override the `block` tags of `abc.html` if my component template | ||
uses `extends`. In that case, just as you would expect, the `block inner` inside | ||
`abc.html` will render `OVERRIDEN`: | ||
|
||
````py | ||
@component.register("my_comp") | ||
class MyComp(component.Component): | ||
template_name = """ | ||
{% extends "abc.html" %} | ||
|
||
{% block inner %} | ||
OVERRIDEN | ||
{% endblock %} | ||
""" | ||
``` | ||
|
||
```` | ||
|
||
4. This is where it gets interesting (but still intuitive). You can insert even | ||
new `slots` inside these "overriding" blocks: | ||
|
||
```py | ||
@component.register("my_comp") | ||
class MyComp(component.Component): | ||
template_name = """ | ||
{% extends "abc.html" %} | ||
|
||
{% load component_tags %} | ||
{% block "inner" %} | ||
OVERRIDEN | ||
{% slot "new_slot" %} | ||
hello | ||
{% endslot %} | ||
{% endblock %} | ||
""" | ||
``` | ||
|
||
And you can then pass fill for this `new_slot` when rendering the component: | ||
|
||
```django | ||
{% component "my_comp" %} | ||
{% fill "new_slot" %} | ||
XYZ | ||
{% endfill %} | ||
{% endcomponent %} | ||
``` | ||
|
||
NOTE: Currently you can supply fills for both `new_slot` and `body` slots, and you will | ||
not get an error for an invalid/unknown slot name. But since `body` slot is not rendered, | ||
it just won't do anything. So this renders the same as above: | ||
|
||
```django | ||
{% component "my_comp" %} | ||
{% fill "new_slot" %} | ||
XYZ | ||
{% endfill %} | ||
{% fill "body" %} | ||
www | ||
{% endfill %} | ||
{% endcomponent %} | ||
``` |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,9 @@ | ||
from typing import Callable, List, NamedTuple, Optional | ||
|
||
from django.template import Context, Template | ||
from django.template.base import Node, NodeList, TextNode | ||
from django.template.defaulttags import CommentNode | ||
from django.template.loader_tags import ExtendsNode, IncludeNode, construct_relative_path | ||
|
||
|
||
def nodelist_has_content(nodelist: NodeList) -> bool: | ||
|
@@ -20,26 +22,79 @@ class NodeTraverse(NamedTuple): | |
parent: Optional["NodeTraverse"] | ||
|
||
|
||
def walk_nodelist(nodes: NodeList, callback: Callable[[Node], Optional[str]]) -> None: | ||
def walk_nodelist( | ||
nodes: NodeList, | ||
callback: Callable[[Node], Optional[str]], | ||
context: Optional[Context] = None, | ||
) -> None: | ||
"""Recursively walk a NodeList, calling `callback` for each Node.""" | ||
node_queue: List[NodeTraverse] = [NodeTraverse(node=node, parent=None) for node in nodes] | ||
while len(node_queue): | ||
traverse = node_queue.pop() | ||
callback(traverse) | ||
child_nodes = get_node_children(traverse.node) | ||
child_nodes = get_node_children(traverse.node, context) | ||
child_traverses = [NodeTraverse(node=child_node, parent=traverse) for child_node in child_nodes] | ||
node_queue.extend(child_traverses) | ||
|
||
|
||
def get_node_children(node: Node) -> NodeList: | ||
def get_node_children(node: Node, context: Optional[Context] = None) -> NodeList: | ||
""" | ||
Get child Nodes from Node's nodelist atribute. | ||
|
||
This function is taken from `get_nodes_by_type` method of `django.template.base.Node`. | ||
""" | ||
# Special case - {% extends %} tag - Load the template and go deeper | ||
if isinstance(node, ExtendsNode): | ||
# NOTE: When {% extends %} node is being parsed, it collects all remaining template | ||
# under node.nodelist. | ||
# Hence, when we come across ExtendsNode in the template, we: | ||
# 1. Go over all nodes in the template using `node.nodelist` | ||
# 2. Go over all nodes in the "parent" template, via `node.get_parent` | ||
nodes = NodeList() | ||
nodes.extend(node.nodelist) | ||
template = node.get_parent(context) | ||
nodes.extend(template.nodelist) | ||
return nodes | ||
|
||
# Special case - {% include %} tag - Load the template and go deeper | ||
elif isinstance(node, IncludeNode): | ||
template = get_template_for_include_node(node, context) | ||
return template.nodelist | ||
|
||
nodes = NodeList() | ||
for attr in node.child_nodelists: | ||
nodelist = getattr(node, attr, []) | ||
if nodelist: | ||
nodes.extend(nodelist) | ||
return nodes | ||
|
||
|
||
def get_template_for_include_node(include_node: IncludeNode, context: Context) -> Template: | ||
""" | ||
This snippet is taken directly from `IncludeNode.render()`. Unfortunately the | ||
render logic doesn't separate out template loading logic from rendering, so we | ||
have to copy the method. | ||
""" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As it says above, this function is a copy from Django's codebase |
||
template = include_node.template.resolve(context) | ||
# Does this quack like a Template? | ||
if not callable(getattr(template, "render", None)): | ||
# If not, try the cache and select_template(). | ||
template_name = template or () | ||
if isinstance(template_name, str): | ||
template_name = ( | ||
construct_relative_path( | ||
include_node.origin.template_name, | ||
template_name, | ||
), | ||
) | ||
else: | ||
template_name = tuple(template_name) | ||
cache = context.render_context.dicts[0].setdefault(include_node, {}) | ||
template = cache.get(template_name) | ||
if template is None: | ||
template = context.template.engine.select_template(template_name) | ||
cache[template_name] = template | ||
# Use the base.Template of a backends.django.Template. | ||
elif hasattr(template, "template"): | ||
template = template.template | ||
return template |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
{% load component_tags %} | ||
<!DOCTYPE html> | ||
<html lang="en"> | ||
<body> | ||
{% component "slotted_component" %} | ||
{% fill "header" %}{% endfill %} | ||
{% fill "main" %} | ||
{% slot "body" %} | ||
Helloodiddoo | ||
{% block inner %} | ||
Default inner | ||
{% endblock %} | ||
{% endslot %} | ||
{% endfill %} | ||
{% endcomponent %} | ||
</body> | ||
</html> |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
{% load component_tags %} | ||
<!DOCTYPE html> | ||
<html lang="en"> | ||
<body> | ||
{% component "slotted_component" %} | ||
{% fill "header" %}{% endfill %} | ||
{% fill "main" %} | ||
{% block body %} | ||
{% endblock %} | ||
{% endfill %} | ||
{% endcomponent %} | ||
</body> | ||
</html> |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,12 +1,12 @@ | ||
{% load component_tags %} | ||
<!DOCTYPE html> | ||
<html lang="en"> | ||
<body> | ||
<main role="main"> | ||
<div class='container main-container'> | ||
{% block body %} | ||
{% endblock %} | ||
</div> | ||
</main> | ||
</body> | ||
</html> | ||
<body> | ||
<main role="main"> | ||
<div class='container main-container'> | ||
{% block body %} | ||
{% endblock %} | ||
</div> | ||
</main> | ||
</body> | ||
</html> |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To resolve the parent Template referenced via
{% extends %}
tag, we needed to pass it a Context. That's why much of the non-test changes is adding acontext
argument to functions