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

Utility mechanism for plugins to render templates #577

Closed
simonw opened this issue Sep 23, 2019 · 7 comments
Closed

Utility mechanism for plugins to render templates #577

simonw opened this issue Sep 23, 2019 · 7 comments
Labels
Milestone

Comments

@simonw
Copy link
Owner

simonw commented Sep 23, 2019

Sometimes a plugin will need to render a template for some custom UI. We need a documented API for doing this, which ensures that everything will work correctly if you extend base.html etc.

See also #576. This could be a .render() method on the Datasette class, but that feels a bit weird - should that class also take responsibility for rendering?

@simonw simonw added the plugins label Sep 23, 2019
@simonw simonw added this to the Datasette 1.0 milestone Sep 23, 2019
@simonw
Copy link
Owner Author

simonw commented Oct 4, 2019

Here's the horrific (stack inspecting) workaround I came up with for a prototype of datasette-atom: simonw/datasette-atom@c0e3bd9#diff-acba9942430bc5e616410567296f92ffR87

@simonw
Copy link
Owner Author

simonw commented Nov 3, 2019

I think a .render_template() method on the Datasette class would be acceptable, since the purpose of that class will mainly be to provide a documented API for plugins to use: #576

@simonw
Copy link
Owner Author

simonw commented Nov 28, 2019

Better idea: take advantage of pluggy dependency injection. If a plugin takes a render argument we can send it a function that can be used to render a template.

The need to await render(...) might be difficult here though.

@simonw
Copy link
Owner Author

simonw commented Dec 9, 2019

I'm going to go with Datasette(...).render_template(...) - I need that for #648, and it makes sense to me that the Datasette class is the documented interface that plugins interact with.

@simonw
Copy link
Owner Author

simonw commented Dec 10, 2019

The reason I've been dragging my heels on adding template rendering to the Datasette class is that it feels messy - should that class be responsible for both data access AND template rendering?

I think I can come to terms with this thanks to plugins. The Datasette class can represent the family of features that plugins affect - which means that having it expose the template rendering API is reasonable.

@simonw
Copy link
Owner Author

simonw commented Dec 22, 2019

The code in question currently lives in BaseView.render():

async def render(self, templates, request, context):
template = self.ds.jinja_env.select_template(templates)
select_templates = [
"{}{}".format("*" if template_name == template.name else "", template_name)
for template_name in templates
]
body_scripts = []
# pylint: disable=no-member
for script in pm.hook.extra_body_script(
template=template.name,
database=context.get("database"),
table=context.get("table"),
view_name=self.name,
datasette=self.ds,
):
body_scripts.append(jinja2.Markup(script))
extra_template_vars = {}
# pylint: disable=no-member
for extra_vars in pm.hook.extra_template_vars(
template=template.name,
database=context.get("database"),
table=context.get("table"),
view_name=self.name,
request=request,
datasette=self.ds,
):
if callable(extra_vars):
extra_vars = extra_vars()
if asyncio.iscoroutine(extra_vars):
extra_vars = await extra_vars
assert isinstance(extra_vars, dict), "extra_vars is of type {}".format(
type(extra_vars)
)
extra_template_vars.update(extra_vars)
template_context = {
**context,
**{
"app_css_hash": self.ds.app_css_hash(),
"select_templates": select_templates,
"zip": zip,
"body_scripts": body_scripts,
"extra_css_urls": self._asset_urls("extra_css_urls", template, context),
"extra_js_urls": self._asset_urls("extra_js_urls", template, context),
"format_bytes": format_bytes,
"database_url": self.database_url,
"database_color": self.database_color,
},
**extra_template_vars,
}
if request.args.get("_context") and self.ds.config("template_debug"):
return Response.html(
"<pre>{}</pre>".format(
escape(json.dumps(template_context, default=repr, indent=4))
)
)
return Response.html(await template.render_async(template_context))

Should datasette.render_template() do exactly this, or should it be slightly different?

Plugins need the option to not pass a request object - so maybe that parameter becomes optional.

Perhaps plugins should be able to render templates without other plugins getting to inject their own variables?

Does it always make sense to dump in all of those extra template context variables?

@simonw
Copy link
Owner Author

simonw commented Feb 4, 2020

For the moment I'm going to move it to async def render_template() on datasette but otherwise keep the implementation the same.

The new signature will be:

async def render_template(self, template, context=None, request=None, view_name=None):

template can be a list of strings or a single string. If a list of strings a template will be selected from them.

I'll reconsider the large list of default context variables later on in a separate ticket.

simonw added a commit that referenced this issue Feb 4, 2020
simonw added a commit that referenced this issue Feb 4, 2020
@simonw simonw closed this as completed in 70b915f Feb 4, 2020
simonw added a commit that referenced this issue Feb 14, 2020
Also improved parameter documentation for other methods, refs #576
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant