Skip to content

BUG: Jinja2Renderer's tag rendering is too broad in scopeΒ #547

@pydanny

Description

@pydanny

Describe the bug

@bluerosej discovered that as our tag rendering in air.templates.Jinja2Renderer relies on the str built-in rather than the .render() method only on Renderables it converts things like dicts and lists of dicts to strings. That makes using Jinja templates harder than it should be.

# https://github.com/feldroy/air/blob/main/src/air/templating.py#L86

context = {k: str(v) for k, v in context.items()}

Expected outcome

What we should do instead is something like this psuedocode:

context = {
    k: str(v) if hasattr(v, "render") or isinstance(v, Renderable) else v
    for k, v in context.items()
}

Caution

This might have unpleasant side effects, so we need to tread carefully. That means a focus on tests covering any edge case we can cook up.

Additional context

Many thanks to @bluerosej for identifying this bug. Their workaround is here: https://github.com/pyladiesmanila/VoicesOfPyLadiesManila/pull/18/files#diff-cca44dd7fc814d07de9290f341144cb5ef7a34bbfd8bb9181a098dd809b542e2R85-R87 and directly inspired the potential fix listed above. I'll make sure they get shared attribution for any commits I make on a bugfix PR.

@bluerosej, if you want to work on this ticket instead of me, we're open to pull requests. Just say the word and the ticket is yours. πŸ˜„

Metadata

Metadata

Assignees

Labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions