Expose sql and params arguments to various plugin hooks

[simonw]

On Discord: https://discord.com/channels/823971286308356157/996877076982415491/1022784534363787305

Hi! I'm attempting to write a plugin that would provide some statistics on text fields (most common words, etc). I would want this information displayed in the table pages, and (ideally) also updated when users make custom queries from the table pages.

It seems one way to do this would be to use the extra_template_vars hook, and make the appropriate SQL query there. So extra_template_vars would create a variable that is a list of most common words, and this is displayed on the page, possibly above the regular table view.

Is there a way that the plugin code can access the SQL query (or even the data) that was used to produce the table view? I can see that TableView class constructs the SQL query, but I can't seem to find a way to access that information from the objects that are available to extra_template_vars.

[simonw]

I've wanted something like this in the past too. I think the thing to do here might be to add sql and params arguments to a bunch of the plugin hooks, such that they can see the main query that is being used on the page that they are helping to render.

While I'm working on this: https://docs.datasette.io/en/0.62/plugin_hooks.html#register-output-renderer-datasette output renderer functions take sql but do not currently take params - they should also take params.

[simonw]

Which plugin hooks should take sql and params?

• extra_template_vars(template, database, table, columns, view_name, request, datasette)
• extra_css_urls(template, database, table, columns, view_name, request, datasette)
• extra_js_urls(template, database, table, columns, view_name, request, datasette)
• extra_body_script(template, database, table, columns, view_name, request, datasette)

And maybe these:

• render_cell(row, value, column, table, database, datasette)
• table_actions(datasette, actor, database, table, request)

I'll start by implementing the first set, then I'll think further about those "maybes".

[simonw]

Implementation challenge: all four of those hooks are called inside the datasette.render_template() method, which has this signature:

datasette/datasette/app.py

Lines 945 to 947 in cb1e093

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

So I would have to pull the sql and params variables out of the context since they are not being passed to that method. OR I could teach that method to take those as optional arguments.

Might be an opportunity to clean up this hack:

datasette/datasette/app.py

Lines 959 to 964 in cb1e093

	for extra_script in pm.hook.extra_body_script(
	template=template.name,
	database=context.get("database"),
	table=context.get("table"),
	columns=context.get("columns"),
	view_name=view_name,

[simonw]

Maybe the signature for that method should be:

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

Where plugin_context is a special dictionary of values that can be passed through to plugin hooks that accept them - so database, table, columns, sql and params.

Those would then be passed when specific views call render_template() - which they currently do via calling BaseView.render(...), but actually the views that are used for tables and queries don't even call that directly due to the weird designed used with DataView subclasses that implement a .data() method.

So yet another change that's blocked on fixing that long-running weird piece of technical debt:

• Complete refactor of TableView and table.html template #1518

[jefftriplett]

While you are adding features, would you be future-proofing your APIs if you switched over some arguments over to keyword-only arguments or would that be too disruptive?

Thinking out loud:

async def render_template( 
     self, templates, *, context=None, plugin_context=None, request=None, view_name=None 
 ): 

[simonw]

This is a good idea - it's something I should do before Datasette 1.0.

I was a tiny bit worried about compatibility (Datasette is 3.7+) but it looks like they have been in Python since 3.0!

[simonw]

Made a start on this:

diff --git a/datasette/hookspecs.py b/datasette/hookspecs.py
index 34e19664..fe0971e5 100644
--- a/datasette/hookspecs.py
+++ b/datasette/hookspecs.py
@@ -31,25 +31,29 @@ def prepare_jinja2_environment(env, datasette):
 
 
 @hookspec
-def extra_css_urls(template, database, table, columns, view_name, request, datasette):
+def extra_css_urls(
+    template, database, table, columns, sql, params, view_name, request, datasette
+):
     """Extra CSS URLs added by this plugin"""
 
 
 @hookspec
-def extra_js_urls(template, database, table, columns, view_name, request, datasette):
+def extra_js_urls(
+    template, database, table, columns, sql, params, view_name, request, datasette
+):
     """Extra JavaScript URLs added by this plugin"""
 
 
 @hookspec
 def extra_body_script(
-    template, database, table, columns, view_name, request, datasette
+    template, database, table, columns, sql, params, view_name, request, datasette
 ):
     """Extra JavaScript code to be included in <script> at bottom of body"""
 
 
 @hookspec
 def extra_template_vars(
-    template, database, table, columns, view_name, request, datasette
+    template, database, table, columns, sql, params, view_name, request, datasette
 ):
     """Extra template variables to be made available to the template - can return dict or callable or awaitable"""

diff --git a/datasette/app.py b/datasette/app.py
index 03d1dacc..2f3a46fe 100644
--- a/datasette/app.py
+++ b/datasette/app.py
@@ -1036,7 +1036,9 @@ class Datasette:
 
         return await template.render_async(template_context)
 
-    async def _asset_urls(self, key, template, context, request, view_name):
+    async def _asset_urls(
+        self, key, template, context, request, view_name, sql, params
+    ):
         # Flatten list-of-lists from plugins:
         seen_urls = set()
         collected = []
@@ -1045,6 +1047,8 @@ class Datasette:
             database=context.get("database"),
             table=context.get("table"),
             columns=context.get("columns"),
+            sql=sql,
+            params=params,
             view_name=view_name,
             request=request,
             datasette=self,